Why linting makes me yawn
I’ve always had some bias against linters. I want to figure out why.
First of all, lint reminds me of my dryer, and some particles of fabric that come loose and collect together and are captured in a screen. Cleaning this out between loads seems like the single least interesting, least impactful chore I could choose to do.
Whoever decided to name the first versions of software that detects likely defects a “linter” is the worst marketer in software engineering. Linting sounds optional. It sounds like mindless work you do when you’re bored and unmotivated. It sounds like it gives you the same shirt you put into the dryer, but with a microgram of fabric removed.
On the other hand, I believe in letting the computer do part of the coding, especially in the age of AI doing all the work instead of people. Think of the type checker in the compiler of a typed programming language. (I happen to work on TypeScript a lot.) It finds likely bugs in your program, and fixing type errors doesn’t feel boring or optional, it feels like engineering for maintainability and scale.
On the other end of the spectrum, linters do mundane things like warn you that you could have put that curly brace on a different line. This is exactly like cleaning out the dryer trap. I don’t care where the curly brace goes! If it’s supposed to go somewhere else, just do that. I once wrote a programming language (rookie mistake, but I’ve grown) and eventually decided not to have required syntax at all. Just store a mutable AST as a shared-editable data structure with audited modifications, and each engineer can choose whatever visualization of the AST they find most readable.
My conclusion for formatting nits: use a formatter over your code. We use clang-format for Angular 2; it’s great for TypeScript and my teammate @martinprobst is always making it better. You can still have style preferences, just make a decision as a team, and let the computer handle it from there. Build it into your toolchain so everyone runs it (this could still be better). I see no value in using a linter for this instead, and making a human do the fixing.
I brought up the type-checker. We all seem to agree that when a red-underline appears in the editor, or the compiler fails the build, it needs to be fixed, because the problem is likely a serious correctness bug. Let’s say I write another check, which is much like the type-checker: find method calls that have no side-effect and the return value was lost. The canonical example is
let a = " string "
a.trim()A lot of programming mistakes are due to misunderstanding an API, and assuming it mutates the value, just as this programmer seems to think that the value of `a` will be changed here.
If this is a linter check, then wow, linters are way more important than I thought, sign me up! We used FindBugs on Google’s Java code, it found real bugs, and we said, sign us up! But the linter doesn’t run in every place the compiler runs. The only satisfactory solution we found is, always run this kind of “100% incorrect” lint check along with the compiler. That means red underlines in the editor, command-line build fails, continuous integration fails, and the engineer who insists on using a totally different dev environment sees it too.
This is where I question having a separate linter tool. When should I run the linter? Should it force me to move curly braces around while I’m hacking away? Should I be stopped before running the build and test suite because of some stray whitespace? Will everyone on my team run it, or will Bob say “oh, yeah, that curly brace is on the end of line 34 because I didn’t run the linter on my change.” (Then I fix it and line 34 is forever blamed to the wrong person.) Because of the useless formatting checks, we want to defer getting a stack of warnings until we “clean up the code someday.” But for the 100% incorrectness checks, we want them right there next to the type checker, and enforced for everyone.
My conclusion for 100% incorrectness issues: add them to the compiler. Since some of these are particular to API usage and not the language spec, they need to be contributed into an extensible compiler with some plugin system. This conserves resources: the compiler already parsed and type-checked and symbol-tabled your code, and has that in memory, so avoid another tool having to do that again, possibly with a different version of the compiler or different options enabled.
So formatting is done by a formatter, and correctness is enforced by a compiler. What’s left? The “grey area” of issues that require some human judgement. These are “warnings”: maybe you should do something about this, but it’s not obvious what you should do, and maybe leaving it alone is a reasonable action.
I could write a lot about the grey area. But not today, there’s something more urgent. The tl;dr is: only show warnings during code review when you’ve touched the code in question. Let the code reviewer see it too. Build this up into a fantastic expert system for machine-assisted code review.
Then you’re done, grey area solved!
My last problem with linters, and for that matter with compiler checks that get added over time, is that few of them are accompanied by a suggested fix.
The suggested fix is kind of maybe useful to an engineer when encountering the issue. You probably meant to re-assign the string.trim value to “a” above, we can just suggest that, and modify your sources. Some editors do this with some static analysis plugins. Kind of cool, I guess.
But I’m much more interested in the fix for a different use case. When I want to change a compiler option for the Angular 2 codebase, and it restricts the semantics in some way, it will break our build. I could say “hey, I broke the build on purpose, now everyone else go fix it.” But I am rightfully thrown out a window. It’s my job to fix the broken build before I commit the compiler option change. And it can be a big job! Turning on the TypeScript “noImplicitAny” check, which requires all types to be explicit (or inferred), means putting a lot of type annotations in Angular 2 codebase where they are currently missing. I added some custom tslint checks to require types in some places, and fixed those issues as I went, but it was tough, manual work.
What I really need, as the “code janitor” for a big team, is for all new compiler checks to come with a suggested fix, that I can apply and send as a pull request to the inferred author of the code. Let’s take an example, TypeScript 1.8 adds “noImplicitFallthrough” to avoid missing “break” statements. What if I wanted to turn this on for a giant TypeScript codebase (like Visual Studio Code for example)?
First I turn on the option and see what breaks. We did this for noImplicitFallthrough and an issue: it would not allow the final case (or default block) to omit break, which is not actually a bug. Sure, we could put break in those places as well. But that’s in the grey area, not really a correctness issue. Iterate on the check until it catches only real positive cases. Maybe the TypeScript team at Microsoft will find a way to run new checks over all their internal codebases to refine in this way.
Next, research carefully what a good suggested fix would be. It might seem like there are a lot of different fixes. In practice, there are typically two:
- Preserve program behavior. Delete the “a.trim()” call.
- Try to re-establish the programmer’s likely intent
If you have good test coverage and care about the long-term viability of the program, do the second option, even though there are risks.
Finally, use the suggested fix to roll out the new compiler check. As the one expert on noImplicitFallthrough, you probably know more about the nuance of ways to fix it, so you should do it yourself, not leave a trail of warnings for others to find and possibly act on. Now everyone is guaranteed to have correct code, and you don’t need a wildly diverging set of compiler or linter options for different parts of your software.
My conclusion: linters, compilers, whatever need to respect programmer’s time, by establishing correctness in the compiler, not suggesting nitpicks that could be automated, and fixing the code instead of just finding problems in it. And we need a different name because dryer lint is super duper boring.
