Include suggested fixes along with diagnostics

[alexeagle]

Context: Angular team is starting to use tslint more, including for TypeScript code in Google's codebase, and @mgechev is building Angular 2-specific rules that we'll want to apply (https://github.com/mgechev/ng2lint)

Internally, we have a big codebase with shared ownership (http://www.wired.com/2015/09/google-2-billion-lines-codeand-one-place/). This means that when we want to improve the codebase, we'll make something an error (if it has very few false negatives), and fix all the existing occurrences as part of turning on that error ("they can readily remove bugs" as the article says). Things that must be warnings show up in code review.

In either case, any of our static analysis tools need produce one or more suggested fixes along with diagnostic messages. We use these to apply fixes before making something an error in the compiler, which we do globally for all developers.

I don't see anything in tslint that currently produces deltas against the input sources. Have you thought about producing fixes from tslint rules? Have you followed @rbuckton's work in https://github.com/Microsoft/TypeScript/tree/transforms which allows modifying TypeScript's AST? Any design thoughts on how this could work?

[jkillian]

Thanks for the info @alexeagle. We'd like to have this feature: #561 is this same idea. Of course for many of the rules, there's no good way to suggest a fix, but for simple stylistic rules it's doable enough.

I haven't looked deeply into the best way of doing this given the current TypeScript APIs, but I have been following microsoft/TypeScript#5595 somewhat and imagine it'd provide a good way of doing this. Funny enough, someone just asked about AST transformations over in the TypeScript issues.

On the other hand, it might be possible to do this a more basic way - rules could simply figure out how the text should change. For example, for a rule that requires semicolons, it could easily return some sort of object saying what text should be inserted where.

[alexeagle]

hey @jkillian

Thanks for the links. @DanielRosenwasser's suggestion on microsoft/TypeScript#7580 is similar to what we do in http://ErrorProne.info.
We would actually prefer to construct small ASTs and serialize them to code, rather than do string concatenation, but @cushon reminds me that we found in javac that pretty-printing AST nodes normalizes some bits of syntax, and drops comments. He adds
"I wish we could construct fixes as operations on the AST, but it'd work better with a different AST."

So I think the ideal answer is something that uses TypeScript to construct the string replacements by building AST chunks, and I'd like to look through microsoft/TypeScript#5595 to see how it could be done.

As for simple stylistic rules, we prefer to use a formatter (clang-format for Angular 2 and google-internal) to simply re-write our sources to comply with a style guide. This is a much better user experience for developers than manually addressing comments from a linter (even if the comments came with a suggested fix).

I am much more interested in the tricky cases, like rolling out --noImplicitFallthrough to all the codebases we work with.

[myitcv]

As for simple stylistic rules, we prefer to use a formatter

@alexeagle - for that exact reason we use tsfmt

I am much more interested in the tricky cases

As are we (hence #561). So would be very interested to either discuss further or ride on your coat tails if you've made some progress.

One other important consideration here is #680. I suspect without full type information you're going to find it very hard (impossible in places) to accurately rewrite the AST.

On a related note therefore, we are in the process of writing an initial version of something we are calling tsvet. tsvet is a "skunk works"-esque program that builds on the ideas explored here, and of course tslint itself. The ultimate goal will be to incorporate findings from tsvet into tslint via #680 if possible...

[alexeagle]

Thanks for the notes. Can I have a peek at any design or early implementation of tsvet?

[myitcv]

@alexeagle sure thing, will ping you when we have a first cut ready to share.

[jkillian]

Subscribe me to the tsvet first-look group too 😄

[myitcv]

@jkillian will do

[alexeagle]

@myitcv how is tsvet going? I've started an extensible compiler for Angular 2's needs, and I included an extension point that I hope works for your needs, see https://docs.google.com/document/d/17Ag4HLfSCYpuo154S6dhzMlT1oTV5E2dT4Ve2XM8eLQ/edit?usp=sharing

[jkillian]

Also @myitcv, are you making heavy use of the language services for tsvet? Wondering with regards to #1131 and perf issues, as I'm looking into ways to use them more efficiently

[myitcv]

@alexeagle thanks for the prompt. I'm putting something together, will share tomorrow most likely. Would be good to then take a view on how these pieces/tools fit together. I'll take a look through your doc.

@jkillian nothing concrete on that front as yet... but you're right, worth considering/looking into.

[myitcv]

@jkillian - can you email me ([email protected]) to let me know your email address?

[gvkhna]

@myitcv Please subscribe me to tsvet.