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

indent rule unexpected result with for loop + semi-colon for the body #8882

Closed
Trott opened this issue Jul 5, 2017 · 1 comment · Fixed by homezen/eslint-config-homezen#43
Closed

indent rule unexpected result with for loop + semi-colon for the body #8882

Trott opened this issue Jul 5, 2017 · 1 comment · Fixed by homezen/eslint-config-homezen#43
Assignees
@not-an-aardvark
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

Comments

@Trott
Copy link
Contributor

Trott commented Jul 5, 2017

Tell us about your environment

  • ESLint Version: 4.1.1
  • Node Version: 8.1.3
  • npm Version: 5.0.4

What parser (default, Babel-ESLint, etc.) are you using?
default

Please show your full configuration:

Configuration 
root: true

env:
  node: true
  es6: true

parserOptions:
  ecmaVersion: 2017

rules:
  indent: [2, 2, {ArrayExpression: first,
                  CallExpression: {arguments: first},
                  FunctionDeclaration: {parameters: first},
                  FunctionExpression: {parameters: first},
                  MemberExpression: off,
                  ObjectExpression: first,
                  SwitchCase: 1}]

What did you do? Please include the actual source code causing the issue.

for (var handle; handle = this.handles.shift(); handle.close())
  ;

What did you expect to happen?
I expected indentation of the ; that represents the body of the for loop to be accepted at 2 spaces of indentation.

What actually happened? Please include the actual, raw output from ESLint.

2:1 error Expected indentation of 0 spaces but found 2 indent
@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jul 5, 2017
@not-an-aardvark
Copy link
Member

Thanks for reporting, I can reproduce this.

@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 and removed triage An ESLint team member will look at this issue soon labels Jul 5, 2017
@not-an-aardvark not-an-aardvark self-assigned this Jul 6, 2017
not-an-aardvark added a commit that referenced this issue Jul 6, 2017
@not-an-aardvark
Update: fix indentation of EmptyStatements (fixes #8882)
2153c58 
The `indent` rule contains some logic to ensure that semicolons at the start of a line are indented correctly when using semicolon-free style. For example, in the following code the semicolon on the second line is not indented, even though it's part of the `bar()` statement.

```js
if (foo) bar()
; [1, 2, 3].map(foo)
```

Due to a bug, this logic also applied to semicolons in `EmptyStatement` nodes, resulting in an incorrect indentation for those semicolons. This commit fixes that bug.
@not-an-aardvark not-an-aardvark mentioned this issue Jul 6, 2017
Merged
This was referenced Jul 9, 2017
Closed
Closed
Closed
Closed
Merged
@HomeZenBot HomeZenBot mentioned this issue Jul 10, 2017
Merged
Trott added a commit to Trott/io.js that referenced this issue Jul 10, 2017
@Trott
tools: update ESLint to 4.2.0
d71fd5f 
ESLint 4.2.0 contains a fix for a bug that is blocking us from moving to
the non-legacy stricter indentation linting. Update to 4.2.0 to remove
the blocking issue.

Refs: eslint/eslint#8882
Refs: eslint/eslint#8885
@Trott Trott mentioned this issue Jul 10, 2017
Closed
2 tasks
Trott added a commit to Trott/io.js that referenced this issue Jul 12, 2017
@Trott
tools: update ESLint to 4.2.0
1562fb9 
ESLint 4.2.0 contains a fix for a bug that is blocking us from moving to
the non-legacy stricter indentation linting. Update to 4.2.0 to remove
the blocking issue.

PR-URL: nodejs#14155
Ref: eslint/eslint#8882
Ref: eslint/eslint#8885
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
addaleax pushed a commit to nodejs/node that referenced this issue Jul 18, 2017
@Trott @addaleax
tools: update ESLint to 4.2.0
ef53149 
ESLint 4.2.0 contains a fix for a bug that is blocking us from moving to
the non-legacy stricter indentation linting. Update to 4.2.0 to remove
the blocking issue.

PR-URL: #14155
Ref: eslint/eslint#8882
Ref: eslint/eslint#8885
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Fishrock123 pushed a commit to nodejs/node that referenced this issue Jul 19, 2017
@Trott @Fishrock123
tools: update ESLint to 4.2.0
c8f2d26 
ESLint 4.2.0 contains a fix for a bug that is blocking us from moving to
the non-legacy stricter indentation linting. Update to 4.2.0 to remove
the blocking issue.

PR-URL: #14155
Ref: eslint/eslint#8882
Ref: eslint/eslint#8885
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Aug 16, 2017
@Trott @MylesBorins
tools: update ESLint to 4.2.0
80029b4 
ESLint 4.2.0 contains a fix for a bug that is blocking us from moving to
the non-legacy stricter indentation linting. Update to 4.2.0 to remove
the blocking issue.

PR-URL: #14155
Ref: eslint/eslint#8882
Ref: eslint/eslint#8885
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@Trott Trott mentioned this issue Aug 16, 2017
Merged
4 tasks
Trott added a commit to Trott/io.js that referenced this issue Aug 16, 2017
@Trott
tools: update ESLint to 4.2.0
54ec97f 
ESLint 4.2.0 contains a fix for a bug that is blocking us from moving to
the non-legacy stricter indentation linting. Update to 4.2.0 to remove
the blocking issue.

PR-URL: nodejs#14155
Ref: eslint/eslint#8882
Ref: eslint/eslint#8885
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Aug 16, 2017
@Trott @MylesBorins
tools: update ESLint to 4.2.0
ec68f07 
ESLint 4.2.0 contains a fix for a bug that is blocking us from moving to
the non-legacy stricter indentation linting. Update to 4.2.0 to remove
the blocking issue.

Backport-PR-URL: #14859
PR-URL: #14155
Ref: eslint/eslint#8882
Ref: eslint/eslint#8885
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Sep 5, 2017
@Trott @MylesBorins
tools: update ESLint to 4.2.0
c5adb5f 
ESLint 4.2.0 contains a fix for a bug that is blocking us from moving to
the non-legacy stricter indentation linting. Update to 4.2.0 to remove
the blocking issue.

Backport-PR-URL: #14859
PR-URL: #14155
Ref: eslint/eslint#8882
Ref: eslint/eslint#8885
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@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.
Assignees

@not-an-aardvark not-an-aardvark

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
Development

Successfully merging a pull request may close this issue.

⬆️ eslint @ 4.2.0 homezen/eslint-config-homezen
3 participants
@Trott @not-an-aardvark @eslintbot