If you like it, you should have put some lint on it...
February 15, 2020
I once overheard a manager talking to a developer about the speed of development. He asked why the developer lints his code, that only slows him down. Another story comes from another company where a lead developer made a statement, that linters make developers lazy.
My opinion is that if you spend several hours of your day doing code reviews, and you only point out space vs tab
errors or argue about where to put curly braces, or where does the question mark go in a ternary expression… Well, then you really like to slack off.
Of course, a team should always have a coding style. Even guidelines defined as early as possible. In my experience, these guidelines are set by lead developers or architects who have their reasons. My problem is that too often these coding styles and guidelines get lost in a lonely confluence/wiki page after an unhealthy amount of arguing and bikeshedding.
Some of the rules are memorised by the development team, and they work on the codebase for some time, but developers come and go. You can even make that confluence (or wiki) page part of the new-hire process, but you must accept the fact, that certain people come from different places with different preferences which won’t change by reading a wiki page. So when somebody says that linters make developers lazy he is essentially not wrong, but not right either.
Let the linter deal with it
First and foremost, some things are essential to remember. Coding style rules are not in that category, because most of them can be perfectly automated. I’d rather remember my wife’s birthday than the proper place of curly braces preferred by the project’s architect (or whoever made the decision). So if you want to enforce it, automate it and when developers stop fighting the linter they become more productive.
One fine example id leaving multiple empty lines between methods or function calls. I personally didn’t care about multiple empty lines, but I had a colleague who pointed out every unnecessary empty line in the codebase.
Eventually, I got tired, because that colleague always put a needs work
flag on my pull-requests and often because of these empty lines. While the changes were good, I had to fix the empty lines and wait for another two approver to be able to finally merge it.
Tslint came to the rescue with the no-consecutive-empty-lines
rule and suddenly we were shipping features faster. Nobody got hurt in the process.
Now, the linter setup should not be something that blocks you. For example, it would be crazy to block typescript compilation because the code you write is not properly formatted (and cannot be fixed automatically). Linting should not be part of hot module replacement, however, there are 3 stages where you can lint your code.
1. Your IDE
2. Pre-commit hooks
3. CI/CD pipeline
I always recommend putting a lint check at the beginning of a CI/CD pipeline, that way no change could go in, which does not meet the preferred coding style. Running the linter manually every time before pushing is still something that you have to remember, that is why I prefer using pre-commit hooks. You can easily set them up. I am working with JavaScript/TypeScript most of the time and recently I have been using NX Workspace for my projects which comes with some out-of-the-box setup.
npm install husky lint-staged --save-dev
I usually start with installing Husky and Lint-Staged, then I set it up in my package.json file:
"husky": { "hooks": { "pre-commit": "lint-staged" } }
I used to set up the lint-staged commands to run the nx formatter and the linter on the whole project before commit, but that took a long time.
Luckily, my friend Tamás helped me out with a lint-staged.config.js
file:
const path = require("path")
module.exports = {
"*.{html,js,ts,json,md,css,scss}": (files) => {
if (
files.length > 0 &&
files[0] !== "[filename]" &&
files[0] !== "[file]"
) {
const cwd = process.cwd()
const relativePaths = files.map((f) => path.relative(cwd, f))
return [
`nx format:write --files="${relativePaths.join(",")}"`, //
`nx affected:lint --files="${relativePaths.join(
","
)}" --fix --parallel`, //
`git add ${relativePaths.join(" ")}`,
]
} else {
return []
}
},
}
This way only the staged and changed files are linted and formatted. As you can see, NX uses prettier for its format
script.
Here is my prettier config which gets used:
{
"arrowParens": "always",
"singleQuote": true,
"trailingComma": "es5",
"endOfLine": "lf",
"printWidth": 140
}
Setting the end of line
with prettier can prevent issues during code reviews, namely that somebody joins the team on a windows machine, and they have autoclrf = true
set. That makes pretty hard to find the actual changes.
A .editorconfig
file is important as well. Most IDEs accept it by default, some of them need plugins, but it is a worthy cause.
With a .editorconfig
file you can set the line endings, the indent style, the indent size and most basic code styling problems that can come up in a code review.
The most wonderful thing about aesthetic linting is that it saves so much time, and helps to focus the developers’ attention towards code readability and architecture instead of arguing about 2 spaces or 4 spaces or tabs.
Why are some of the stricter code formatting rules useful?
Diverging from an agreed upon coding style is usually autofixable using linters. This means that the new-hire can write whatever he/she wants, however, he/she wants, without interruption. Then the pre-commit hook will format his/her code and everybody is happy.
The developer now can focus on what to write instead of how to write it.
Of course, there are stricter rules for linters as well. I for one like SonarQube, but that is not always accessible for budget reasons.
However, we have sonar-js
and sonar-ts
lint rulesets installed as packages, which can help tremendously. One of my favourites is the cognitive-complexity linting rule.
That has certainly made my life easier because extremely complex methods stopped appearing in the codebase. Readable code is more maintainable,
and when functions are separated in small understandable chunks that is a benefit for everybody.
Cognitive complexity is an extremely important measurement. If you use VSCode you can find a very useful plugin here.
These conventions and configurations should live near the code itself. That is why you have .eslintrc
and tsconfig.json
and .stylelintrc
in your project’s root folder.
That way if somebody new joins the team they don’t have to fiddle around with setting up everything the way it is declared on a confluence or wiki page somewhere. That is why I don’t like to put these rules in .vscode
or .idea
or whatever folders. In my opinion, enforcing IDE
settings on developers is a serious intrusion into their personal spaces.
Can you control your developer’s IDE?
I once worked with somebody who insisted that everybody should use the same IDE settings that he/she used and the .vscode folder should be committed to the repository.
Whenever I use VSCode for certain projects, I love to use peacock so I can differentiate between windows.
However, peacock colour settings saved into the .vscode folder and pushed to the remote repository would force other developers to use the same colours that I like. Not a friendly way to work in a team and it would cause unnecessary merge conflicts as well.
Of course, you and your team should decide upon the ruleset which you want to apply. However, I recommend setting rules for brackets, member-ordering and everything that is autofixable. For example, setting up the arrowparens
rule would make it easier to search for arrow functions.
For example, you might vaguely remember using an arrow function with a specific variable. Searching for it using specificVariable) => {
will find it for you.
I’d like to end this post with a story of how linting could have prevented huge errors in production. I was working with a good friend of mine, who was a rookie developer under my hands at the time. He was excellent! He was eager to learn, and
he immediately accepted the fact that writing unit tests is really important. So he did, but one time he encountered an issue where he needed to fix something in one breaking unit test.
He focused that test (fit
in Jasmine) while tackling the problem, and forgot to un-focus it. The change was pushed, and another branch was rebased to his changes.
That other branch, however, broke lots of unit tests, but since in the CI pipeline only one focused test ran, we didn’t notice the issue first. It was a lucky catch, because one developer noticed, that the CI/CD pipeline was faster.
We’ve looked at the test reports and found only the focused test. That saved us that time, but obviously one little fit
is very easy to miss when the whole file is marked as a change.
After that incident we integrated the tslint-jasmine-rules ruleset to our linting process. The no-focused-tests
and no-disabled-tests
rules help a lot.
There are packages for jest and mocha, and other frameworks as well.
What are your favourite lint rules? Have you ever had situations where linting saved you? Or situations, where linting could have saved you? Tweet your stories at me if you’d like to. :)