Skip to content

[@typescript-eslint/indent] False positive on chained calls in multiline const declaration #119

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Closed
scottohara opened this issue Jan 22, 2019 · 2 comments · Fixed by #385
Closed

[@typescript-eslint/indent] False positive on chained calls in multiline const declaration #119

scottohara opened this issue Jan 22, 2019 · 2 comments · Fixed by #385
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@scottohara
Copy link
Contributor

scottohara commented Jan 22, 2019 •
edited
Loading

Repro

{
  "rules": {
    "@typescript-eslint/indent": ["error", 2, { "VariableDeclarator": { "const": 3 } }]
  }
}
const div: JQuery<HTMLElement> = $('<div>')
        .addClass('some-class')
        .appendTo($('body')),

      button: JQuery<HTMLElement> = $('<button>')
        .text('Cancel')
        .appendTo(div);

Expected Result
No errors

Actual Result

2:1  error  Expected indentation of 2 spaces but found 8  @typescript-eslint/indent
3:1  error  Expected indentation of 2 spaces but found 8  @typescript-eslint/indent

Additional Info

  1. Only the indentation on lines 2-3 seems to trigger the error. Similar indentation used on lines 6-7 does not trigger the error.
  2. The error only seems to trigger when generic type annotations (e.g. JQuery<HTMLElement>) are used.

An example without generics does not error:

const name: string = '  Typescript  '
        .toUpperCase()
        .trim(),

      greeting: string = ` Hello ${name} `
        .toUpperCase()
        .trim();

Versions

package version
@typescript-eslint/eslint-plugin 1.0.0
@typescript-eslint/parser 1.0.0
TypeScript 3.2.1
ESLint 5.11.1
node 11.5.0
npm 6.5.0
@scottohara scottohara added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jan 22, 2019
@bradzacher bradzacher added the bug Something isn't working label Jan 22, 2019
@bradzacher
Copy link
Member

the indenter does some seemingly weird things sometimes in regards to how it calculates indentation.
Something as simple as a type annotation can cause it to reset the indentation level for the entire statement.

confirmed against master.

@bradzacher bradzacher removed the triage Waiting for team members to take a look label Jan 22, 2019
armanio123 pushed a commit to armanio123/typescript-eslint that referenced this issue Jan 24, 2019
@macklinu @JamesHenry
Chore: Update eslint-plugin-node to v6.0.x (typescript-eslint#119)
42462c7
@scottohara
Copy link
Contributor Author

Just to add a bit more detail here, I temporarily added the code samples above into the valid array of the indent tests.

(My intention was to attempt debugging the problem myself and submit a PR, but I got lost trying to understand how the rule actually works).

When the test suite runs, the error reported includes nodeType: Punctuator.

I'm not sure if this helps at all.

@typescript-eslint/eslint-plugin: Summary of all failing tests
@typescript-eslint/eslint-plugin: FAIL tests/rules/indent.test.ts (8.105s)
@typescript-eslint/eslint-plugin:   ● indent › valid › 
@typescript-eslint/eslint-plugin: const container: JQuery<HTMLElement> = $('<div>')
@typescript-eslint/eslint-plugin:         .addClass('some-class')
@typescript-eslint/eslint-plugin:         .appendTo($('body')),
@typescript-eslint/eslint-plugin:       button: JQuery<HTMLElement> = $('<button>')
@typescript-eslint/eslint-plugin:         .text('Cancel')
@typescript-eslint/eslint-plugin:         .appendTo(container);
@typescript-eslint/eslint-plugin:             
@typescript-eslint/eslint-plugin:     assert.strictEqual(received, expected)
@typescript-eslint/eslint-plugin:     Expected value to strictly be equal to:
@typescript-eslint/eslint-plugin:       0
@typescript-eslint/eslint-plugin:     Received:
@typescript-eslint/eslint-plugin:       2
@typescript-eslint/eslint-plugin:     Message:
@typescript-eslint/eslint-plugin:       Should have no errors but had 2: [ { ruleId: 'indent',
@typescript-eslint/eslint-plugin:         severity: 1,
@typescript-eslint/eslint-plugin:         message: 'Expected indentation of 2 spaces but found 8.',
@typescript-eslint/eslint-plugin:         line: 3,
@typescript-eslint/eslint-plugin:         column: 1,
@typescript-eslint/eslint-plugin:         nodeType: 'Punctuator',
@typescript-eslint/eslint-plugin:         messageId: 'wrongIndentation',
@typescript-eslint/eslint-plugin:         endLine: 3,
@typescript-eslint/eslint-plugin:         endColumn: 9,
@typescript-eslint/eslint-plugin:         fix: { range: [Array], text: '  ' } },
@typescript-eslint/eslint-plugin:       { ruleId: 'indent',
@typescript-eslint/eslint-plugin:         severity: 1,
@typescript-eslint/eslint-plugin:         message: 'Expected indentation of 2 spaces but found 8.',
@typescript-eslint/eslint-plugin:         line: 4,
@typescript-eslint/eslint-plugin:         column: 1,
@typescript-eslint/eslint-plugin:         nodeType: 'Punctuator',
@typescript-eslint/eslint-plugin:         messageId: 'wrongIndentation',
@typescript-eslint/eslint-plugin:         endLine: 4,
@typescript-eslint/eslint-plugin:         endColumn: 9,
@typescript-eslint/eslint-plugin:         fix: { range: [Array], text: '  ' } } ]
@typescript-eslint/eslint-plugin:       at testValidTemplate (../../node_modules/eslint/lib/testers/rule-tester.js:415:20)
@typescript-eslint/eslint-plugin:       at Object.RuleTester.it (../../node_modules/eslint/lib/testers/rule-tester.js:584:25)

@scottohara scottohara mentioned this issue Mar 28, 2019
Merged
aprilrd added a commit to aprilrd/typescript-eslint that referenced this issue Apr 4, 2019
@aprilrd
fix(eslint-plugin): prevented a regression for a fixed issue (typescr…
9bfdbae 
…ipt-eslint#119)
@aprilrd aprilrd mentioned this issue Apr 4, 2019
Closed
aprilrd added a commit to aprilrd/typescript-eslint that referenced this issue Apr 4, 2019
@aprilrd
fix(eslint-plugin): fix an indentation following generic type annotat…
6aab650 
…ion (typescript-eslint#119)
@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Feb 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(eslint-plugin) indent: fix false positive on type parameters scottohara/typescript-eslint
2 participants
@scottohara @bradzacher