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: ignoreNodes not working as expected in "tab" mode #9392

Closed
robinhouston opened this issue Oct 5, 2017 · 1 comment
Closed

indent rule: ignoreNodes not working as expected in "tab" mode #9392

robinhouston opened this issue Oct 5, 2017 · 1 comment
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

@robinhouston
Copy link
Contributor

I have a proposed fix for this, which I’ll send as a pull request. But I don’t think the bug itself has been reported yet, hence the below.

Tell us about your environment

  • ESLint Version: 4.8.0
  • Node Version: 8.1.2
  • npm Version: 5.4.2

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

Please show your full configuration:

Configuration 
{
    "rules": {
        "indent": ["error", "tab", {
            "ignoredNodes": [ "VariableDeclarator" ]
        }]
    }
}

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

/* eslint indent: [ "error", "tab", { "ignoredNodes": [ "VariableDeclarator" ] } ] */
var x = 1,
    y = 2;

What did you expect to happen?
No error, since VariableDeclarator nodes are ignored.

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

$ eslint --no-eslintrc dent.js 

/Users/robin/Desktop/dent.js
  3:1  error  Expected indentation of 4 tabs but found 4 spaces  indent

✖ 1 problem (1 error, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.
@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Oct 5, 2017
robinhouston added a commit to robinhouston/eslint that referenced this issue Oct 5, 2017
@robinhouston
indent: ignored nodes should accept any indentation
a61a7dd 
When a node is ignored by the indent rule, it ought not to matter
how it’s indented. But the ignoring of nodes was implemented in
such a way that the *type* of indentation (tabs vs spaces) was
being checked. For example in "tab" mode, an ignored line indented
by four spaces would cause the error “Expected indentation of 4 tabs
but found 4 spaces”.

In particular, this is a problem with “tabs for indentation, spaces
for alignment” styles, where we want to allow code like:

var x = 1,
    y = 2;

where the second line is aligned using four spaces.

This commit marks ignored indents by making them instances of
the IgnoredTokenIndent class, and explicitly ignoring the indentation
of such lines.

All tests pass.

Fixes eslint#9392.
@robinhouston robinhouston mentioned this issue Oct 5, 2017
Merged
robinhouston added a commit to robinhouston/eslint that referenced this issue Oct 5, 2017
@robinhouston
Fix: ignored nodes in indent rule (fixes eslint#9392)
90ae854 
When a node is ignored by the indent rule, it ought not to matter
how it’s indented. But the ignoring of nodes was implemented in
such a way that the *type* of indentation (tabs vs spaces) was
being checked. For example in "tab" mode, an ignored line indented
by four spaces would cause the error “Expected indentation of 4 tabs
but found 4 spaces”.

In particular, this is a problem with “tabs for indentation, spaces
for alignment” styles, where we want to allow code like:

var x = 1,
    y = 2;

where the second line is aligned using four spaces.

This commit marks ignored indents by making them instances of
the IgnoredTokenIndent class, and explicitly ignoring the indentation
of such lines.

All tests pass.

Fixes eslint#9392.
robinhouston added a commit to robinhouston/eslint that referenced this issue Oct 5, 2017
@robinhouston
Alternative fix for eslint#9392
aafb937
@not-an-aardvark
Copy link
Member

Thanks for the report. I can reproduce this issue.

@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 Oct 5, 2017
robinhouston added a commit to robinhouston/eslint that referenced this issue Oct 5, 2017
@robinhouston
Fix: ignored nodes in indent rule (fixes eslint#9392)
8af1f44 
When a node is ignored by the indent rule, it ought not to matter
how it’s indented. But the ignoring of nodes was implemented in
such a way that the *type* of indentation (tabs vs spaces) was
being checked. For example in "tab" mode, an ignored line indented
by four spaces would cause the error “Expected indentation of 4 tabs
but found 4 spaces”.

In particular, this is a problem with “tabs for indentation, spaces
for alignment” styles, where we want to allow code like:

var x = 1,
    y = 2;

where the second line is aligned using four spaces.

This commit marks ignored indents by making them instances of
the IgnoredTokenIndent class, and explicitly ignoring the indentation
of such lines.

All tests pass.

Fixes eslint#9392.
robinhouston added a commit to robinhouston/eslint that referenced this issue Oct 5, 2017
@robinhouston
Fix: ignored nodes in indent rule (fixes eslint#9392)
3b9daa7 
When a node is ignored by the indent rule, it ought not to matter
how it’s indented. But the ignoring of nodes was implemented in
such a way that the *type* of indentation (tabs vs spaces) was
being checked. For example in "tab" mode, an ignored line indented
by four spaces would cause the error “Expected indentation of 4 tabs
but found 4 spaces”.

In particular, this is a problem with “tabs for indentation, spaces
for alignment” styles, where we want to allow code like:

var x = 1,
    y = 2;

where the second line is aligned using four spaces.

The implementation is taken from @not-an-aardvark’s comment
eslint#9393 (review)

All tests pass.

Fixes eslint#9392.
robinhouston added a commit to robinhouston/eslint that referenced this issue Oct 7, 2017
@robinhouston
Fix: ignored nodes in indent rule (fixes eslint#9392)
bf7a3ed 
When a node is ignored by the indent rule, it ought not to matter
how it’s indented. But the ignoring of nodes was implemented in
such a way that the *type* of indentation (tabs vs spaces) was
being checked. For example in "tab" mode, an ignored line indented
by four spaces would cause the error “Expected indentation of 4 tabs
but found 4 spaces”.

In particular, this is a problem with “tabs for indentation, spaces
for alignment” styles, where we want to allow code like:

var x = 1,
    y = 2;

where the second line is aligned using four spaces.

The implementation is taken from @not-an-aardvark’s comment
eslint#9393 (review)

All tests pass.

Fixes eslint#9392.
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Apr 10, 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 Apr 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

No branches or pull requests
3 participants
@robinhouston @not-an-aardvark @eslintbot