add auto-fix for no-var-keyword

[alexeagle]

Note, we just replace with let which can cause errors if the identifier is referenced before the declaration or outside the scope. TypeScript will catch these at type-check time

[palantirtech]

Thanks for your interest in palantir/tslint, @alexeagle! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

[jkillian]

My only concern here is that this can change the behavior of user code without triggering a TS error. Take the below (contrived) example:

var a = 0;
{ var a = 1; }
console.log(a);

It'll print "1" before the fix and "0" after the fix.

[ChrisMBarr]

FYI, the tslint-microsoft-contrib project has this as a formatter. It kinda works, but I ran into some issue with it.
Description: https://github.com/Microsoft/tslint-microsoft-contrib#supported-formatters
Code: https://github.com/Microsoft/tslint-microsoft-contrib/blob/master/src/fixNoVarKeywordFormatter.ts

[alexeagle]

@ChrisMBarr can you elaborate what issue you ran into?
@jkillian we don't require type-checker for this rule, so I suppose the best we can do is a syntactic check whether the declared token appears before the var statement?

[ChrisMBarr]

@alexeagle Check this issue I had open with them about the formatter: microsoft/tslint-microsoft-contrib#173

I had 2 problems with it, they fixed one of them as it was just a simple bug in the formatter, but the other one about file paths was not something they were able to resolve. perhaps they went about doing this in a way that wasn't ideal, but if the file path issue could be resolved that would be fantastic.

in the end though, it wasn't to big of a deal. i was able to make a complicated glob pattern that worked and I ran the formatter to replace all the var with let in my project in one pass. That worked great, then I deleted this complex pattern and enabled the TSLint rule to enforce this for any future var keywords.

IMO, running a fix like this should be a one-time thing so get an application up to speed with the TSLint rules. I've spoken with people on my team about automated code "fixes" like this and in general we don't want them to happen frequently for the possibility of potential bugs, etc.

[jkillian]

@alexeagle I'd guess that if you had code like my example above, you actually were expecting let semantics and not var semantics. In which case, TSLint would be fixing a bug.

Still though, a little risky to do that, so if it's easily possible to look for previous tokens of the same name in previous var declarations, let's do that as you suggested.

[ChrisMBarr]

Risky, but this seems like the kind of thing (hopefully) that gets checked over by a human after it's run, and it would only happen once. Are the situations where you'd want to keep running this auto-fixer?

Seems like you'd want to run this because the no-var-keyword rule produces a lot of violations and this would be a quick & easy way to fix them all at once. Do that, and then afterwards just enable that rule to catch any new/future violations.

[adidahiya]

Gonna merge this -- we categorize no-var-keyword as a Functionality related rule, and autofixers for these kinds of rules will likely require manual review.