Disclosure: I may earn affiliate revenue or commissions if you purchase products from links on my website. The prospect of compensation does not influence what I write about or how my posts are structured. The vast majority of articles on my website do not contain any affiliate links.
Static program analysis is a code quality inspection that is performed without the codebase needing to be executed. It is most commonly used to check code’s conformance to standards and detect errors. Because the program under test does not need to be executed, static analysis is relatively lightweight and can often be performed in real-time via integration with an IDE or text editor. Though there are some well-established standards for programming languages, static analyzers can be loaded with custom rules to meet an individual’s or organization’s guidelines. In this way, analyzers are often used to guide major refactorings.
Spending a lot of time with Python, I have grown fond of Pylint. It enforces PEP8 guidelines and is really useful in helping to catch errors when they are cheapest to fix before I build my Docker images. It is integrated with most major IDEs and can also be bolted on to vim using Syntastic (pylint.vim being deprecated). I often use vim, and Pylint has saved me an awful lot of time in preventing me from submitting what would have been bad merge requests. Yes, though I’ve gotten away from the practice, I spent more than a full year using a fairly standard vim setup for all my coding–Pylint was my lifeline and when I forgot to use it, I often paid the price. Pylint also helps enforce familiarity with (if not outright conformance to) PEP8 style guidelines, which are helpful for any newbie to learn. It provides a numerical rating of your program.
There is a darker side of Pylint, though. To the uninitiated, it is easy to confuse style guidelines with rules. While it may be fulfilling to manually go through a program and watch the Pylint score go up, whoever has to deal with your merge request is going to hate you. For one thing, you’re probably not actually doing much good and, two, even a stylistic refactor needs comprehensive integration testing. If you don’t have confidence in your testing pipeline, it’s a non-starter.
There are automated tools such as autopep8 which propose taking the manual guesswork out of stylistic refactoring. However, I never had a perfect experience using it, and wrong indentation was notoriously hard to detect and fix. So, I took it upon myself to manually refactor large codebases using PEP8 conformance as a baseline. I wasted days of my time. While there is no doubting that I became more intimately familiar with the codebase, the standards, and whatever audiobook I was able to listen to while mindlessly going about my business, most of the code never was merged into production.
If time travel was possible, I would open up a wormhole to the precise moment in my career where I decided that universal PEP8 conformance was necessary and taze myself. What in the world was I thinking? There was an issue raised in the requests library that explains a modern pragmatic approach to Pylint quite kindly. Reading through that thread was a real learning experience for me and partially inspired this blog post.
In summary, a new user raised an issue where he requested that the library conform more strongly to Pylint’s standards. He posted a dump of all of the errors highlighted by the static analysis.
The maintainer–who I have a lot of respect for–‘s response was kind but authoritative. The default Pylint standards are somewhat arbitrary and, more to the point, all of the errors raised by the tool for the Requests library were objectively not errors. For one thing, there was nothing constructive about the original post, though a naive user (read: me) would be tempted to see this as an easy way to contribute to a major project (though, again, there was no contribution aside from the error dump which was completely tone-deaf). The fact of the matter is that automated tools, especially in the realm of static analysis, can’t possibly be one hundred percent correct.
However, the original poster doubled down (citing books and style guidelines) and the maintainer responded again. I admire what he said because he could have just told the guy to buzz off. Instead, he posted some advice which has wide applications that I will share verbatim:
I agree that pylint can catch errors. However, doing a drive-by dump of every line reported by pylint is not a helpful approach. Two notes for the future:
- Authors often have different views on Python style, particularly PEP8. Requests does not follow all of PEP8, and this is quite deliberate. You should always consult a project’s author before raising a bug report about their code failing a linter.
- You should validate the concerns of the linter before raising them. Open source maintainers are often very busy people, and asking them to validate each line of the linter’s output for false positives is a rude way to behave. If you’d looked at these ‘errors’ first you’d have seen that they aren’t valid.
- It’s likely that you’ll say we should add a .pylintrc file to address these concerns. That’s not a good idea. We should not have to add a dotfile to the repository root for every person who writes a linter.
These commandments should guide each person’s usage of linting tools. I eventually gave up on major refactors and more pragmatically added a Pylint check (with many custom rules defined) to our build system. It is often through mistakes that we learn the most and I feel fortunate that the maintainer of this particular library was willing to write such a detailed response to a nuanced complaint.
