Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: fix multiline binary operator/parentheses indentation #8719

Merged
merged 1 commit into from Jun 15, 2017
Merged

Update: fix multiline binary operator/parentheses indentation #8719

merged 1 commit into from Jun 15, 2017

Conversation

not-an-aardvark
Copy link
Member

@not-an-aardvark not-an-aardvark commented Jun 13, 2017 •
edited

What is the purpose of this pull request? (put an "X" next to item)

[x] Bug fix (#8666, #8717, #8710)

What changes did you make? (Give an overview)

Fixes #8666, fixes #8717, fixes #8710

Previously, the logic for indenting multiline parenthesized expressions assumed that the indentation of every token in the expression other than the first was dependent on the first token. However, this assumption is not always correct. This led to bugs with multiline parenthesized expressions (#8710). Additionally, the BinaryExpression listener attempted to account for this assumption by always linking its tokens' indentation to the first token's indentation, even when it didn't make sense to do so. This led to other bugs (#8666, #8717). This commit updates the parenthesis logic to avoid making that assumption and check the indentation of all the tokens properly.

Is there anything you'd like reviewers to focus on?

Nothing in particular

@not-an-aardvark not-an-aardvark added accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly indent Relates to the `indent` rule rule Relates to ESLint's core rules labels Jun 13, 2017
@not-an-aardvark not-an-aardvark changed the title Update: fix multiline binary operator indent (fixes #8666, fixes #8717) WIP: Update: fix multiline binary operator indent (fixes #8666, fixes #8717) Jun 13, 2017
@not-an-aardvark
Update: fix multiline binary operator/parentheses indentation
1a171dc 
Fixes #8666, fixes #8717, fixes #8710

Previously, the logic for indenting multiline parenthesized expressions assumed that the indentation of every token in the expression other than the first was dependent on the first token. However, this assumption is not always correct. This led to bugs with multiline parenthesized expressions (#8710). Additionally, the BinaryExpression listener attempted to account for this assumption by always linking its tokens' indentation to the first token's indentation, even when it didn't make sense to do so. This led to other bugs (#8666, #8717). This commit updates the parenthesis logic to avoid making that assumption and check the indentation of all the tokens properly.
@not-an-aardvark not-an-aardvark force-pushed the indent-multiline-binary-expressions-8717 branch from 67fdfcf to 1a171dc Compare June 13, 2017 03:16
@eslintbot
Copy link

LGTM

@not-an-aardvark not-an-aardvark changed the title WIP: Update: fix multiline binary operator indent (fixes #8666, fixes #8717) Update: fix multiline binary operator indent (fixes #8666, fixes #8717) Jun 13, 2017
@not-an-aardvark not-an-aardvark changed the title Update: fix multiline binary operator indent (fixes #8666, fixes #8717) Update: fix multiline binary operator/parentheses indentation Jun 13, 2017
@not-an-aardvark not-an-aardvark mentioned this pull request Jun 13, 2017
Closed
@gyandeeps gyandeeps merged commit b5a70b4 into master Jun 15, 2017
@gyandeeps gyandeeps deleted the indent-multiline-binary-expressions-8717 branch June 15, 2017 01:20
@renovate renovate bot mentioned this pull request Jun 24, 2017
Merged
@renovate-bot renovate-bot mentioned this pull request Jun 24, 2017
Merged
This was referenced Jun 24, 2017
Merged
Closed
Merged
This was referenced Jul 7, 2017
Closed
Closed
Closed
bors bot added a commit to IMA-WorldHealth/bhima that referenced this pull request Jul 9, 2017
@bors
Merge #1817
67a6f86 
1817: Update eslint to the latest version 🚀 r=jniles


## Version **4.1.0** of [eslint](https://github.com/eslint/eslint) just got published.

<table>
  <tr>
    <th align=left>
      Dependency
    </td>
    <td>
      eslint
    </td>
  </tr>
  <tr>
    <th align=left>
      Current Version
    </td>
    <td>
      3.19.0
    </td>
  </tr>
  <tr>
    <th align=left>
      Type
    </td>
    <td>
      devDependency
    </td>
  </tr>
</table>

The version **4.1.0** is **not covered** by your **current version range**.

Without accepting this pull request your project will work just like it did before. There might be a bunch of new features, fixes and perf improvements that the maintainers worked on for you though.

I recommend you look into these changes and try to get onto the latest version of eslint.
Given that you have a decent test suite, a passing build is a strong indicator that you can take advantage of these changes by merging the proposed change into your project. Otherwise this branch is a great starting point for you to work on the update.


---


<details>
<summary>Release Notes</summary>
<strong>v4.1.0</strong>

<ul>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/e8f1362ab640c883a5d296e951308fab22073e7f" class="commit-link"><tt>e8f1362</tt></a> Docs: Remove wrong descriptions in <code>padded-block</code> rule (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8783" class="issue-link js-issue-link" data-url="eslint/eslint#8783" data-id="237771100" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8783</a>) (Plusb Preco)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/291a78302c1d5d402c6582b3f4cc836e61fde787" class="commit-link"><tt>291a783</tt></a> Update: <code>enforceForArrowConditionals</code> to <code>no-extra-parens</code> (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/6196" class="issue-link js-issue-link" data-url="eslint/eslint#6196" data-id="155067290" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#6196</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8439" class="issue-link js-issue-link" data-url="eslint/eslint#8439" data-id="220697521" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8439</a>) (Evilebot Tnawi)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/a21dd32c46f95bc232a67929c224824692f94b70" class="commit-link"><tt>a21dd32</tt></a> New: Add <code>overrides</code>/<code>files</code> options for glob-based config (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/3611" class="issue-link js-issue-link" data-url="eslint/eslint#3611" data-id="104053558" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#3611</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8081" class="issue-link js-issue-link" data-url="eslint/eslint#8081" data-id="207675247" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8081</a>) (Sylvan Mably)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/879688ce96f80aa0692f732759c6f67a0c36c4c3" class="commit-link"><tt>879688c</tt></a> Update: Add ignoreComments option to no-trailing-spaces (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8061" class="issue-link js-issue-link" data-url="eslint/eslint#8061" data-id="206865549" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8061</a>) (Jake Roussel)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/b58ae2e6d6bd4662b549ca5c0472943055a74df8" class="commit-link"><tt>b58ae2e</tt></a> Chore: Only instantiate fileEntryCache when cache flage set (perf) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8763" class="issue-link js-issue-link" data-url="eslint/eslint#8763" data-id="236756225" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8763</a>) (Gyandeep Singh)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/98512881f1fc2417011247931fa089d987ee8cc6" class="commit-link"><tt>9851288</tt></a> Update: fix indent errors on multiline destructure (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/8729" class="issue-link js-issue-link" data-url="eslint/eslint#8729" data-id="235733166" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8729</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8756" class="issue-link js-issue-link" data-url="eslint/eslint#8756" data-id="236673913" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8756</a>) (Victor Hom)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/3608f06c2a412587c2d05dec0297803b25f3e630" class="commit-link"><tt>3608f06</tt></a> Docs: Increase visibility of code of conduct (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/8758" class="issue-link js-issue-link" data-url="eslint/eslint#8758" data-id="236687424" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8758</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8764" class="issue-link js-issue-link" data-url="eslint/eslint#8764" data-id="236758243" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8764</a>) (Kai Cataldo)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/673a58bc8420075ba698cee6762e17322a5263c3" class="commit-link"><tt>673a58b</tt></a> Update: support multiple fixes in a report (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/7348" class="issue-link js-issue-link" data-url="eslint/eslint#7348" data-id="182620143" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#7348</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8101" class="issue-link js-issue-link" data-url="eslint/eslint#8101" data-id="208681921" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8101</a>) (Toru Nagashima)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/7a1bc3893ab55d0ab16ccf4b7a62c85329ab4007" class="commit-link"><tt>7a1bc38</tt></a> Fix: don't pass default parserOptions to custom parsers (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/8744" class="issue-link js-issue-link" data-url="eslint/eslint#8744" data-id="236336414" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8744</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8745" class="issue-link js-issue-link" data-url="eslint/eslint#8745" data-id="236373829" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8745</a>) (Teddy Katz)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/c5b405280409698d14b62cbf3c87b7cf6cf71391" class="commit-link"><tt>c5b4052</tt></a> Chore: enable computed-property-spacing on ESLint codebase (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8760" class="issue-link js-issue-link" data-url="eslint/eslint#8760" data-id="236699991" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8760</a>) (Teddy Katz)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/3419f6446e205d79d9db77f6c176b9167d1fd8a7" class="commit-link"><tt>3419f64</tt></a> Docs: describe how to use formatters on the formatter demo page (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8754" class="issue-link js-issue-link" data-url="eslint/eslint#8754" data-id="236645523" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8754</a>) (Teddy Katz)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/a3ff8f21106cf8eca55978d4b3e053973f5e1bf2" class="commit-link"><tt>a3ff8f2</tt></a> Chore: combine tests in tests/lib/eslint.js and tests/lib/linter.js (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8746" class="issue-link js-issue-link" data-url="eslint/eslint#8746" data-id="236375849" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8746</a>) (Teddy Katz)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/b7cc1e6fe995d52e581fcb2b1a44e37a18680e90" class="commit-link"><tt>b7cc1e6</tt></a> Fix: Space-infix-ops should ignore type annotations in TypeScript (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8341" class="issue-link js-issue-link" data-url="eslint/eslint#8341" data-id="217102387" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8341</a>) (Reyad Attiyat)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/46e73eea69abc2ba80bb1397c6779b8789dbd385" class="commit-link"><tt>46e73ee</tt></a> Fix: eslint --init installs wrong dependencies of popular styles (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/7338" class="issue-link js-issue-link" data-url="eslint/eslint#7338" data-id="182134634" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#7338</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8713" class="issue-link js-issue-link" data-url="eslint/eslint#8713" data-id="235217725" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8713</a>) (Toru Nagashima)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/a82361b65699653761436a2e9acc7f485c827ca0" class="commit-link"><tt>a82361b</tt></a> Chore: Prevent package-lock.json files from being created (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/8742" class="issue-link js-issue-link" data-url="eslint/eslint#8742" data-id="236292937" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8742</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8747" class="issue-link js-issue-link" data-url="eslint/eslint#8747" data-id="236397701" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8747</a>) (Teddy Katz)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/5f81a68a3904a559764872e3f0c7453865a6c6dc" class="commit-link"><tt>5f81a68</tt></a> New: Add eslintIgnore support to package.json (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/8458" class="issue-link js-issue-link" data-url="eslint/eslint#8458" data-id="221689525" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8458</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8690" class="issue-link js-issue-link" data-url="eslint/eslint#8690" data-id="233757916" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8690</a>) (Victor Hom)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/b5a70b4e8c20dc1ea3e31137706fc20da339f379" class="commit-link"><tt>b5a70b4</tt></a> Update: fix multiline binary operator/parentheses indentation (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8719" class="issue-link js-issue-link" data-url="eslint/eslint#8719" data-id="235421314" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8719</a>) (Teddy Katz)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/ab8b0167bdaf3b8851eab3fbc2769f2bdd71677b" class="commit-link"><tt>ab8b016</tt></a> Update: fix MemberExpression indentation with "off" option (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/8721" class="issue-link js-issue-link" data-url="eslint/eslint#8721" data-id="235434741" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8721</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8724" class="issue-link js-issue-link" data-url="eslint/eslint#8724" data-id="235459105" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8724</a>) (Teddy Katz)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/eb5d12c15a32084907f1c58bcbec721b5008495d" class="commit-link"><tt>eb5d12c</tt></a> Update: Add Fixer method to Linter API (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8631" class="issue-link js-issue-link" data-url="eslint/eslint#8631" data-id="230242473" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8631</a>) (Gyandeep Singh)</li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/26a2daab311c8c59942c52f436d380a195db2bd4" class="commit-link"><tt>26a2daa</tt></a> Chore: Cache fs reads in ignored-paths (fixes <a href="https://urls.greenkeeper.io/eslint/eslint/issues/8363" class="issue-link js-issue-link" data-url="eslint/eslint#8363" data-id="218136776" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8363</a>) (<a href="https://urls.greenkeeper.io/eslint/eslint/pull/8706" class="issue-link js-issue-link" data-url="eslint/eslint#8706" data-id="235004396" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#8706</a>) (Victor Hom)</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 141 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/7d9e3beeb58c1ee71d53dfcfd3e3b0721dd79b46"><code>7d9e3be</code></a> <code>4.1.0</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/e727b7bdfcfb0564aabd713b32364e6f4afcfeec"><code>e727b7b</code></a> <code>Build: changelog update for 4.1.0</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/e8f1362ab640c883a5d296e951308fab22073e7f"><code>e8f1362</code></a> <code>Docs: Remove wrong descriptions in <code>padded-block</code> rule (#8783)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/291a78302c1d5d402c6582b3f4cc836e61fde787"><code>291a783</code></a> <code>Update: <code>enforceForArrowConditionals</code> to <code>no-extra-parens</code> (fixes #6196) (#8439)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/a21dd32c46f95bc232a67929c224824692f94b70"><code>a21dd32</code></a> <code>New: Add <code>overrides</code>/<code>files</code> options for glob-based config (fixes #3611) (#8081)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/879688ce96f80aa0692f732759c6f67a0c36c4c3"><code>879688c</code></a> <code>Update: Add ignoreComments option to no-trailing-spaces (#8061)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/b58ae2e6d6bd4662b549ca5c0472943055a74df8"><code>b58ae2e</code></a> <code>Chore: Only instantiate fileEntryCache when cache flage set (perf) (#8763)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/98512881f1fc2417011247931fa089d987ee8cc6"><code>9851288</code></a> <code>Update: fix indent errors on multiline destructure (fixes #8729) (#8756)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/3608f06c2a412587c2d05dec0297803b25f3e630"><code>3608f06</code></a> <code>Docs: Increase visibility of code of conduct (fixes #8758) (#8764)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/673a58bc8420075ba698cee6762e17322a5263c3"><code>673a58b</code></a> <code>Update: support multiple fixes in a report (fixes #7348) (#8101)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/7a1bc3893ab55d0ab16ccf4b7a62c85329ab4007"><code>7a1bc38</code></a> <code>Fix: don't pass default parserOptions to custom parsers (fixes #8744) (#8745)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/c5b405280409698d14b62cbf3c87b7cf6cf71391"><code>c5b4052</code></a> <code>Chore: enable computed-property-spacing on ESLint codebase (#8760)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/3419f6446e205d79d9db77f6c176b9167d1fd8a7"><code>3419f64</code></a> <code>Docs: describe how to use formatters on the formatter demo page (#8754)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/a3ff8f21106cf8eca55978d4b3e053973f5e1bf2"><code>a3ff8f2</code></a> <code>Chore: combine tests in tests/lib/eslint.js and tests/lib/linter.js (#8746)</code></li>
<li><a href="https://urls.greenkeeper.io/eslint/eslint/commit/b7cc1e6fe995d52e581fcb2b1a44e37a18680e90"><code>b7cc1e6</code></a> <code>Fix: Space-infix-ops should ignore type annotations in TypeScript (#8341)</code></li>
</ul>
<p>There are 141 commits in total.</p>
<p>See the <a href="https://urls.greenkeeper.io/eslint/eslint/compare/421aab44a9c167c82210bed52f68cf990b7edbea...7d9e3beeb58c1ee71d53dfcfd3e3b0721dd79b46">full diff</a></p>
</details>

<details>
  <summary>Not sure how things should work exactly?</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html) and of course you may always [ask my humans](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>


---


Your [Greenkeeper](https://greenkeeper.io) Bot 🌴
@not-an-aardvark not-an-aardvark mentioned this pull request Jul 10, 2017
Closed
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gyandeeps gyandeeps gyandeeps approved these changes

@kaicataldo kaicataldo kaicataldo approved these changes

Assignees
No one assigned
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly indent Relates to the `indent` rule rule Relates to ESLint's core rules
Projects
None yet
Milestone
No milestone
4 participants
@not-an-aardvark @eslintbot @gyandeeps @kaicataldo