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

[@typescript-eslint/typedef] variableDeclaration broken with for..of loops #783

Closed
octogonz opened this issue Jul 31, 2019 · 2 comments · Fixed by #787
Closed

[@typescript-eslint/typedef] variableDeclaration broken with for..of loops #783

octogonz opened this issue Jul 31, 2019 · 2 comments · Fixed by #787
Labels
bug Something isn't working good first issue Good for newcomers has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@octogonz
Copy link
Contributor

Repro

{
  "rules": {
    "@typescript-eslint/typedef": [
      "error",
      {
        "variableDeclaration": true
      },
    ],
  }
}
// error @typescript-eslint/typedef : expected item to have a type annotation
for (const item of ['a', 'b']) {
}

Expected Result

The error should not be reported for a for..of loop variable.

Actual Result

The error is reported. The error is impossible to fix, because TypeScript does not allow type annotations here.

Additional Info

TSLint encountered this same problem as palantir/tslint#743.

Their fix is here and might be relevant: palantir/tslint#745

Versions

package version
@typescript-eslint/eslint-plugin 2.0.0-alpha.4
@typescript-eslint/parser 1.13.0
TypeScript 3.5.3
ESLint 6.1.0
@octogonz octogonz added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Jul 31, 2019
@octogonz
Copy link
Contributor Author

octogonz commented Jul 31, 2019

@JoshuaKGoldberg TSLint does it like this:

    public visitVariableDeclaration(node: ts.VariableDeclaration) {
        // AFAIK, variable declarations will always have a grandparent, 
        // but check that to be on the safe side.
        // catch statements will be the parent of the variable declaration
        // for-in/for-of loops will be the gradparent of the variable declaration
        if (node.parent != null && node.parent.parent != null
                && node.parent.kind !== ts.SyntaxKind.CatchClause
                && node.parent.parent.kind !== ts.SyntaxKind.ForInStatement
                && node.parent.parent.kind !== ts.SyntaxKind.ForOfStatement) {
            this.checkTypeAnnotation("variable-declaration", node.name.getEnd(), node.type, node.name);
        }
        super.visitVariableDeclaration(node);
    }

That same idea should work for @typescript-eslint/typedef, right?

@bradzacher bradzacher added bug Something isn't working and removed triage Waiting for maintainers to take a look labels Jul 31, 2019
@octogonz
Copy link
Contributor Author

octogonz commented Aug 1, 2019

Here's a PR: #787

@bradzacher bradzacher added good first issue Good for newcomers has pr there is a PR raised to close this labels Aug 1, 2019
@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
bug Something isn't working good first issue Good for newcomers has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants