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

fix(eslint-plugin) indent: fix false positive on type parameters #385

Merged
merged 5 commits into from Apr 7, 2019

Conversation

scottohara
Copy link
Contributor

Fixes #119

As noted in #119, the following code was triggering the indent rule:

/*eslint @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);

...but the following, almost identical, code was not:

/*eslint @typescript-eslint/indent: ["error", 2, { "VariableDeclarator": { "const": 3} }]*/

const div: JQuery = $('<div>')
        .addClass('some-class')
        .appendTo($('body')),

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

The difference, it seems, is the type parameter: <HTMLElement>.

Comparing the two ASTs, I notice a node type that was not included in the KNOWN_NODES set: TSTypeParameterInstantiation.

Without fully understanding the inner workings of the indent rule, I simply added this missing node type to the set; and sure enough it seems to have fixed the false positive.

I've added test cases for the valid code samples and confirmed that the tests still pass; but I'm unsure of any other impact from adding this additional node type to the set of known nodes.

Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bradzacher bradzacher merged commit d476f15 into typescript-eslint:master Apr 7, 2019
@scottohara scottohara deleted the indent branch April 7, 2019 22:38
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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