Rewrite prefer-const

[ajafff]

PR checklist

• Fixes: Support destructuring parameter of prefer-const #2068, prefer-const false positive in for loop #2195
• New feature, bugfix, or enhancement
  • Includes tests
• Documentation update

What changes did you make?

[enhancement] show warnings for var
[bugfix] correctly handle variables shadowed by parameters and catched exceptions
[bugfix] don't warn if one variable in a for loop initializer can not be const
[bugfix] handle more cases in destructuring
[bugfix] only fix initialized variables
[new-rule-option] {destructuring: "all"} to only warn if all variables in a destructuring can be const

Is there anything you'd like reviewers to focus on?

Tests fail with typescript@2.0 because one test contains syntax (spread) which is not supported in that version of the language. In real life this will not result in a problem.
How to handle the ci failure?

[nchen63]

I don't think we should special case variable declaration in for loops to disable flagging for const. This makes sense:

for (const x of y) { ... }

The issue with #2195 actually didn't really have to do with for loops so much as declaring 2 things with the same let.

[ajafff]

@nchen63 I also thought about adding an option for multiple variables declared in the same let. But that can simply be fixed by using separate VaraibleStatements.

The special handling is only for for-loops, not for ... of or for ... in;
The reason is that for loops are the only place where you cannot split a VariableDeclarationList into multiple VariableStatements without moving a variable to another scope.

for (let foo of bar); // still fails
for (let foo in bar); // still fails
for (let foo=bar; someCondition;); // still fails because foo is not reassigned
let i = 0;
for (let len=foo.length; i<len;++i); // still fails because len is not reassigned

// this is the only case that does not fail anymore
// Otherwise you would need to move variable len out of the loop initializer to the parent scope, which could leed to unexpected behaviour if it shadows another variable
for (let i=0, len=foo.length; i<len; ++i); // no fail for len because i is reassigned

Btw: eslint also works that way.

[nchen63]

none of the options for fixing the broken test seems appealing:

1. remove the cases that break pre-ts 2.0
2. change infrastructure to detect when not to run a test (make a different set of tests run for pre-ts 2.0)
3. somehow hack the logic to handle the unknown/newer ts code (not sure if possible)

[nchen63]

"any" should be a const

[ajafff]

I made some further changes to correctly handle reassigned variables in parameter initializer.
Additionally fixes will now only be applied if all variables of a VariableDeclarationList are initialized to avoid compiler errors.

[ajafff]

Regarding the failing tests with older versions of typescirpt:

#2253 will eventually have the same problem and so will all future rules based on new language features or syntax.
I think we need a way to tell the testRunner to ignore certain tests for certain versions of typescript.

[ajafff]

The failing tests are now fixed. Should be ready to merge.

[nchen63]

@ajafff thanks!