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.

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

Indentation fix is misbehaving and ignoring comments #6571

Closed
adamreisnz opened this issue Jun 30, 2016 · 13 comments · Fixed by #7618
Closed

Indentation fix is misbehaving and ignoring comments #6571

adamreisnz opened this issue Jun 30, 2016 · 13 comments · Fixed by #7618
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion indent Relates to the `indent` rule rule Relates to ESLint's core rules

Comments

@adamreisnz
Copy link

adamreisnz commented Jun 30, 2016

What version of ESLint are you using?
I am using Atom Linter's ESLint package, which specified ESLint ^2.11.1 in it's package.json.

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

Please show your full configuration:

root: true
extends: eslint:recommended
parserOptions:
  ecmaVersion: 6
  ecmaFeatures:
    impliedStrict: true
  sourceType: module
env:
  es6: true
  browser: true
  node: true
  mocha: true
  jasmine: true
rules:
  linebreak-style:
    - error
    - unix
  max-len:
    - error
    - code: 100
      tabWidth: 2
      ignoreComments: true
      ignoreUrls: true
  indent:
    - error
    - 2
    - SwitchCase: 1
      VariableDeclarator:
        let: 2
        const: 3
  semi:
    - error
    - always
  consistent-this:
    - error
    - self
    - $ctrl
  quotes:
    - error
    - single
    - allowTemplateLiterals: true
  curly:
    - error
    - all
  comma-dangle:
    - error
    - only-multiline
  new-cap:
    - error
    - newIsCap: true
      capIsNew: true
      properties: true
  camelcase:
    - error
    - properties: never
  array-bracket-spacing:
    - error
    - never
  arrow-spacing:
    - error
    - before: true
      after: true
  block-spacing:
    - error
    - always
  comma-spacing:
    - error
    - before: false
      after: true
  computed-property-spacing:
    - error
    - never
  generator-star-spacing:
    - error
    - before: true
      after: false
  key-spacing:
    - error
    - beforeColon: false
      afterColon: true
      mode: minimum
  keyword-spacing:
    - error
    - before: true
  semi-spacing:
    - error
    - before: false
      after: true
  space-in-parens:
    - error
    - never
  space-unary-ops:
    - error
    - words: true
      nonwords: false
  space-before-function-paren:
    - error
    - never
  space-before-blocks:
    - error
    - always
  yoda:
    - error
    - never
  wrap-iife:
    - error
    - outside
  eqeqeq:
    - error
    - always
  newline-per-chained-call:
    - error
    - ignoreChainWithDepth: 2
  one-var-declaration-per-line:
    - error
    - initializations
  brace-style:
    - error
    - stroustrup
  eol-last: error
  dot-notation: error
  space-infix-ops: error
  no-with: error
  no-unreachable: error
  no-redeclare: error
  no-unexpected-multiline: error
  no-multi-spaces: error
  no-multi-str: error
  no-trailing-spaces: error
  no-mixed-spaces-and-tabs: error
  no-spaced-func: error
  no-whitespace-before-property: error
  no-lonely-if: error
  no-var: error
  no-implicit-coercion: error

What did you do? Please include the actual source code causing the issue.
This is a portion of the source code that is affected. Note that to prevent errors about max line length, I have moved the mixins onto a separate line, but afterwards I open the curly bracket on the first indentation level to prevent the whole file from being indented too much:

export default FlowController.extend(
  BookAppointment, ProcessOutcome, PreventNavigation,
{

  //Nav model
  navModel: Nav.create({
    pageTitle: '...'
  }),

  //Model aliases
  client: Ember.computed.reads('model.client'),
  cards: Ember.computed.reads('model.cards'),

  /**
   * Ok to navigate away check
   */
  isOkToNavigateAway(targetRoute) {

    //Still in first screen
    if (this.get('step') === 'choose-card') {
      return true;
    }

    //Going to appointment
    if (targetRoute === 'appointments.book') {
      return true;
    }

    //Have submitted
    if (this.get('hasSubmitted')) {
      return true;
    }

    //Not ok
    return false;
  }

  //...
});

What did you expect to happen?

  1. That my indentation remains as is, on account of my curly bracket starting at the beginning of the line.
  2. That if ESLInt fixes indentation, it actually also fixes the comments and not leave a garbled mess :)

What actually happened? Please include the actual, raw output from ESLint.
Everything has been indented, except for all the comments, leaving a bit of a mess:

export default FlowController.extend(
  BookAppointment, ProcessOutcome, PreventNavigation,
  {

  //Nav model
    navModel: Nav.create({
      pageTitle: '...'
    }),

  //Model aliases
    client: Ember.computed.reads('model.client'),
    cards: Ember.computed.reads('model.cards'),

  /**
   * Ok to navigate away check
   */
    isOkToNavigateAway(targetRoute) {

    //Still in first screen
      if (this.get('step') === 'choose-card') {
        return true;
      }

    //Going to appointment
      if (targetRoute === 'appointments.book') {
        return true;
      }

    //Have submitted
      if (this.get('hasSubmitted')) {
        return true;
      }

    //Not ok
      return false;
    }

  //...
  });

I don't know how issue 1 can be resolved. It's an annoying Ember pattern that you specify the object you're extending with as the last parameter, and I'd like to be able to have the indentation for that object start at the first position without ESLint trying to fix it.

Issue 2 seems like a genuine bug. It should include comments when fixing indentation or it's just useless.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jun 30, 2016
@adamreisnz
Copy link
Author

It seems the only way to prevent issue 1 from happening is by putting both first two lines up against the gutter, which is a bit ugly:

export default FlowController.extend(
BookAppointment, ProcessOutcome, PreventNavigation, {

  //Now this level of indentation is respected

@nzakas
Copy link
Member

nzakas commented Jul 2, 2016

cc @gyandeeps

@nzakas nzakas added bug ESLint is working incorrectly rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jul 2, 2016
@gyandeeps
Copy link
Member

@adambuczynski Can you please simply the example and provide the actual eslint output for your code. For comment indentation, we do not do comment indentation yet. There is an issue open already.

@adamreisnz
Copy link
Author

@gyandeeps what do you mean? I have provided a full example. How should I obtain the raw output from within Atom? I'm using eslint as an Atom linter.

@lukeapage
Copy link
Contributor

Thats #3845 to be specific

@adamreisnz
Copy link
Author

This issue is not only related to comments. There's another bug with the level of indentation not being respected correctly, see my example above.

@platinumazure
Copy link
Member

@adambuczynski The indent rule still doesn't report or fix comments, but I'm wondering if some of the other issues you've experienced have improved at all? Can you please update us with what you experience in your given examples with latest ESLint? Thanks!

@adamreisnz
Copy link
Author

@platinumazure I'll have a look and see what comes up

@adamreisnz
Copy link
Author

@platinumazure still the exact same behaviour with eslint 3.8.0

@platinumazure
Copy link
Member

Thanks! We'll keep digging.

On Oct 16, 2016 3:27 PM, "Adam Buczynski" notifications@github.com wrote:

@platinumazure https://github.com/platinumazure still the exact same
behaviour with eslint 3.8.0


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#6571 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AARWeliEyPAe9CjrTNJJNB73A2Ec5Aarks5q0oiugaJpZM4JCkRO
.

not-an-aardvark added a commit that referenced this issue Nov 18, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Dec 12, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Dec 12, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Dec 14, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Dec 14, 2016
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Jan 2, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Jan 10, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
@gyandeeps gyandeeps added the indent Relates to the `indent` rule label Jan 14, 2017
not-an-aardvark added a commit that referenced this issue Jan 14, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Jan 28, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Feb 10, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Feb 13, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Feb 18, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Feb 23, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. If this gets released in a major version, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

---

As mentioned above, the new implementation checks a lot of cases that the old implementation did not check. Based on past experience with fixing bugs in the `indent` rule, I anticipate that this will break quite a lot of builds. We might want to consider holding this off until we release a major version.
not-an-aardvark added a commit that referenced this issue Feb 27, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144)

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
not-an-aardvark added a commit that referenced this issue Feb 27, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
not-an-aardvark added a commit that referenced this issue Mar 5, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
not-an-aardvark added a commit that referenced this issue Mar 12, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
not-an-aardvark added a commit that referenced this issue Mar 12, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
not-an-aardvark added a commit that referenced this issue Mar 12, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
@jameswomack
Copy link

@platinumazure Has a workaround been discovered since October?

@platinumazure
Copy link
Member

@jameswomack Hi, sorry, for which issue? There are a few mentioned here. I believe the indent rule is now correctly fixing comments but I'm not 100% sure.

@not-an-aardvark
Copy link
Member

@platinumazure No, it doesn't fix comments in its current form. (It does in the rewrite.)

not-an-aardvark added a commit that referenced this issue Mar 18, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
not-an-aardvark added a commit that referenced this issue Mar 18, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
not-an-aardvark added a commit that referenced this issue Mar 18, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
not-an-aardvark added a commit that referenced this issue Apr 4, 2017
Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself
ilyavolodin pushed a commit that referenced this issue Apr 7, 2017
…e) (#7618)

* Update: rewrite `indent` (fixes #1801, #3737, #3845, #6007, ...16 more)

Fixes #1801, fixes #3737, fixes #3845, fixes #6007, fixes #6571, fixes #6670, fixes #6813, fixes #7242, fixes #7274, fixes #7320, fixes #7420, fixes #7522, fixes #7616, fixes #7641, fixes #7662, fixes #7771, fixes #7892, fixes #8011, fixes #8038, fixes #8144

The existing implementation of `indent` had a lot of bugs (see above list). It worked by detecting a node type (e.g. `ObjectExpression`), and then ensuring that the indentation around the object satisfies certain constraints (e.g. the properties of the `ObjectExpression` are offset by 4 spaces from the opening bracket). This approach had a number of disadvantages:

- Since it only checked indentation according to an explicit list of patterns, there were a lot of cases where it accidentally didn't check the indentation at all. For example, there was no check for the indentation of a closing `)` in a `CallExpression`, so the rule just silently ignored incorrect indentation in these cases. (#7522)
- there were a lot of nodes where indentation wasn't checked at all. For example, it didn't check indentation for ternary expressions (#7420) or destructuring assignments (#6813).
- Since it could only check indent patterns on nodes, it couldn't check the indentation of comments (#3845, #6571) or optional tokens such as parentheses around an expression (#7522)

This commit rewrites the `indent` rule. The new strategy is based on tokens rather than nodes:

1. Create a hashmap (`OffsetStorage#desiredOffsets`). The keys are all the tokens and comments in the file, and the values are objects containing information for a specific offset, measured in indent levels, from a either a specific token or the first column. For example, an element in an array will have `{offset: 1, from: openingCurly}` to indicate that it is offset by one indentation level from the opening curly brace. All the offsets are initialized to 0 at the start.
1. As the AST is traversed, modify the offsets of tokens accordingly. For example, when entering a `BlockStatement`, offset all of the tokens in the `BlockStatement` by 1 from the opening curly brace of the `BlockStatement`.
1. After traversing the AST, calculate the expected indentation levels of every token in the file (according to the `desiredOffsets` map).
1. For each token, compare the expected indentation to the actual indentation in the file, and report the token if the two values are not equal.

This has the following advantages:

- It is guaranteed to check the indentation of every single token in the file, with the exception of some tokens that are explicitly ignored*. This ensures that no tokens end up unexpectedly being ignored.
- Since tokens/comments are used instead of nodes, there are no unchecked "stray tokens".
- All nodes are evaluated in a context-free manner. In other words, each node only has to set an offset for its own children, without worrying about what how much indentation the node itself has or what the node's parents are.
- The rule ends up with an expected indentation map for the entire file at once, and so it can fix the entire file in one pass. (The previous implementation often required multiple passes. For example, if a node was misaligned with its parent in the previous implementation, the node would get fixed, even if the node's position was actually correct and the parent was off.)

*There are a few cases where the new implementation explicitly ignores lines. I decided to do this because there is a huge amount of inconsistency in what people seem to prefer for these cases. In the future, we might want to stop ignoring these cases so that the indentation of all lines is checked. One such case is:

```js
({
  foo:
  bar
});

// versus

({
  foo:
    bar
});
```

Comments are treated a bit differently from tokens in that they can have several different indentations. This is because it can be difficult to tell what the comment is referring to. For example:

```js
if (foo) {
  doSomething();
  // comment about the doSomething() call
} else if (bar.baz()) {
  doSomethingElse();
}

// versus

if (foo) {
  doSomething();
// comment about the bar.baz() call
} else if (bar.baz()) {
  doSomethingElse();
}
```

Specifically, a comment is allowed to have one of three indentations:

1. The same indentation as the token right before it
1. The same indentation as the token right after it
1. The computed indentation for the comment itself

* Ensure reported range has endLine and endColumn

* Use objects instead of WeakMaps to improve performance

* Update the big explanation comment at the top of the file

* Fix variable capitalization

* Remove unneeded IfStatement logic

* Remove unused equality check

* Add test for else without block

* Fix single-line statements with semicolon-first style
@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.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion indent Relates to the `indent` rule rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants