eslint / typescript-eslint-parser

An ESLint custom parser which leverages TypeScript ESTree to allow for ESLint to lint TypeScript source code.
Other
915 stars 92 forks source link

Indent rule regression starting with v20.1.0 #586

Closed scottohara closed 5 years ago

scottohara commented 5 years ago

What version of ESLint are you using? 5.11.1

What version of TypeScript are you using? 3.1.1

What version of typescript-eslint-parser are you using? Any version from 20.1.0 onwards

What code were you trying to parse?

{
  "parser": "typescript-eslint-parser",
  "rules": {
    "indent": ["error", "tab", {"VariableDeclarator": {"const": 3}}]
  }
}

(in the code below, ^ represents a tab character)

interface Foo {
  a: number;
  b: number;
}

const foo: Foo = {
^ ^ ^ ^ a: 1,
^ ^ ^ ^ b: 2
^ ^ ^ },
^ ^ ^ bar = 1;

What did you expect to happen? No errors

What happened? In typescript-eslint-parser@20.0.0, code passes with no errors.

With typescript-eslint-parser@20.1.0 onwards, the errors below occur:

7:1  error  Expected indentation of 1 tab but found 4   indent
8:1  error  Expected indentation of 1 tab but found 4   indent
9:1  error  Expected indentation of 0 tabs but found 3  indent

The code can be modified like below to pass, but clearly this is not what the config intends:

const foo: Foo = {
  a: 1,
  b: 2
},
      bar = 1

Vanilla JS code without a type annotation passes as expected:

const foo = {
        a: 1,
        b: 2
      },
      bar = 1;
scottohara commented 5 years ago

Same issue appears when using spaces instead of tabs:

{
  "parser": "typescript-eslint-parser",
  "rules": {
    "indent": ["error", 2, {"VariableDeclarator": {"const": 3}}]
  }
}
// No type annotation..OK
const foo = {
        a: 1,
        b: 2
      },
      bar = 1;

// With type annotation..OK
const foo: Foo = {
  a: 1,
  b: 2
},
      bar = 1;

// With type annotation..not OK
const foo: Foo = {
        a: 1,
        b: 2
      },
      bar = 1;
7:1  error  Expected indentation of 2 spaces but found 8  indent
8:1  error  Expected indentation of 2 spaces but found 8  indent
9:1  error  Expected indentation of 0 spaces but found 6  indent
armano2 commented 5 years ago

you should consider using https://github.com/bradzacher/eslint-plugin-typescript/blob/master/docs/rules/indent.md

scottohara commented 5 years ago

Thanks @armano2.

It looks like this is a new rule in 1.0.0-rc0, and isn't available in the current 0.14.0 release.

Generally I prefer not to use dependencies labelled as rc or beta, so I guess I may need to disable the base indent rule for now until 1.0.0 is officially released. Hopefully that isn't too far away now.

armano2 commented 5 years ago

@scottohara i'm not using unstable versions to, but i will suggest you to test if this will solve your issue (if not you can report issue) :)

scottohara commented 5 years ago

Unfortunately, the same problem occurs when using typescript/indent (in v1.0.0-rc.0) instead of the main indent rule from eslint:

{
    "parser": "typescript-eslint-parser",
    "plugins": ["typescript"],
    "rules": {
        "indent": "off",
        "typescript/indent": ["error", "tab", {"VariableDeclarator": {"const": 3}}]
    }
}
interface Foo {
  a: number;
  b: number;
}

const foo: Foo = {
        a: 1,
        b: 2
      },
      bar = 1;
7:1  error  Expected indentation of 1 tab but found 4   typescript/indent
8:1  error  Expected indentation of 1 tab but found 4   typescript/indent
9:1  error  Expected indentation of 0 tabs but found 3  typescript/indent
armano2 commented 5 years ago

can you open this issue in eslint-plugin-typescript?

scottohara commented 5 years ago

Sure, no problem. I'll create an issue over there refer back to this thread.

It is worth reiterating that the problem only affects typescript-eslint-parser@20.1.0+, regardless of which plugin version or indent rule is used.

So from an outsider's perspective, it sure does feel like the problem is in the parser, not the plugin/rules.

eslint-plugin-typescript typescript-eslint-parser indent typescript/indent
0.14.0 20.0.0 N/A
1.0.0-rc.0 20.0.0
0.14.0 20.1.0+ N/A
1.0.0-rc.0 20.1.0+
armano2 commented 5 years ago

issue is with new nodes, and with fact that rules can affect them now. additionally you should consider checking v21.0.2 as a recent version.

v20.1.0 introduced visitor keys #516 v21.0.0 introduced proper scope analysis #540

if you are using version before 20.1.0 those nodes are just ignored, and formatting for them is not working at all.

JamesHenry commented 5 years ago

I think this issue was resolved, but feel free to open a new issue on the new project https://github.com/typescript-eslint/typescript-eslint at any time. It will contain the parser and the plugin, so we can write the strongest possible integration tests for issues like this.

Many thanks!