Linting Your Code08 Feb 2018
What is a linter?
Every modern mainstream language has a linter. The name comes from the C programming language, in which the first tool that did more with your source code than let you edit it, print it, or compile it was
Why follow a style guide?
- It’s easier to keep the code in a project consistent. This makes it easier to read and maintain.
- You and your teammates can avoid bikeshedding on what conventions to follow.
- When people contribute code to your project, it’s a low-effort way to keep it from looking like a hodge-podge.
- You can enforce this before you even see a pull request, and without seeming arbitrary. By running a linter on a CI server, you can annotate pull requests with whether they’ve conform to your style guides.
- You’ll get in the habit of writing professional-looking code.
- Your code will work with standard tooling. For example, the Sphinx documentation generator, used by ReadTheDocs, understands the PEP 257, Google, and Numpy docstring conventions linked above.
- Even if you’re working alone — you can avoid thinking about, remembering, or changing your mind about incidental details.
Also see ITech Girl’s What is Flake8 and why we should use it? on Python Pandemonium.
Installing a linter
pip3 install flake8 installs the
requirements.txt and the README. This ensures that a collaborator who follows the setup instructions will end up with the tool, and be instructed to use it. Like other easily-automated code quality tools, the linter is something that you run continuously. It is easier to keep issues out of your codebase than to allow them to build up, especially where those issues, even if they’re “only” stylistic, may lead to bugs or maintenance problems.
flake8 to the requirements file also ensures that it’s available on the CI.2 We’ll return to this later.
flake8 . runs the linter against the files in the current directory and its subdirectories. This reports a number (46) of violations; you can see the full list here. Here’s a sample from the top, edited to remove multiple instances of the same issue type:
./tests/mqtt_json_test.py:1:1: D100 Missing docstring in public module ./tests/mqtt_json_test.py:5:1: E402 module level import not at top of file ./tests/mqtt_json_test.py:10:1: D103 Missing docstring in public function ./mqtt_json/mqtt_config.py:9:80: E501 line too long (95 > 79 characters) ./mqtt_json/mqtt_config.py:32:1: E305 expected 2 blank lines after class or function definition, found 1
Each line consists of a source location, an error code (
D100), and an error description. The location and description allow you to find and fix the violation. The error code is useful in order to ignore it, as we’ll see next.
Configuring the linter
If we had to fix 47 violations before we could start running the linter on a continuous basis — or, if running it reported so many errors that new violations got lost in the noise — then adopting a linter wouldn’t provide much bang for the buck. We want to ignore some violations — some for now, some forever.
There’s two ways to ignore an violation (that we’ll cover), and three reasons to do so.
How to ignore:
- An whole class of violations can be completely ignored, from all files, by adding its code to a
setup.cfgfile. Commit #
- A particular violation can be ignored by placing a special comment, for example
#noqa: E402, on the line that causes the violation. Commit #
b3f1b3bhas examples of this.
Why to ignore:
- We aren’t ready to fix it. Running a code quality tool with some error checking turned off is still better than not running one at all. An example in
setup.cfgis the ignore for
D100“Missing docstring in public module”. I was already thinking I should at least add a module comment to each file;
flake8reminds me that I haven’t done so.
- We disagree with the recommendation. I personally prefer self-documenting code (well-designed code with well-chosen names) to comments and doc-strings; I don’t believe in documenting every class and function; hence,
ignore=D012“Missing docstring in public method”2.
- Somewhere in the middle. I’m also undecided about whether to stick to an 80-character line length. On the one hand, this limit is historically from when screens were smaller. On the other hand, using narrow line lengths makes it easier to view two or three files side-by-side. Rather than resolve this now, I’ve added
max-line-length = 120in the config file.
- Special circumstances. In this case, I’m using a kludge (manipulating Python’s module search path) to import a package from a file’s parent directory (the project root). The “right” thing to do is to publish the package to a private package directory, but this is too heavyweight for this job. The technique used in the current code base, of appending to
sys.pathand then continuing the
importsequence, is IMO appropriate for this special use. I don’t want to ignore
E402“module level import not at top of file” in general, because I agree that it’s generally a bad idea and want to be warned, so I’ll use a file-level ignore to ignore it only where I’ve deliberately used it. (There’s better examples of special circumstances below.)
313af92 fixes the remaining issues. In this case, these are stylistic. This isn’t always true. Often in a dynamic language I find misspelled variables names, that are therefore undefined variables. (This also means that code isn’t being tested.) Usually I also find unused imports, which can clutter the program and make it harder to read and maintain dependencies. (Unused imports are a compiler error in Go.) That didn’t happen here, but the corresponding application of flake8 to the Twilio → MQTT Gateway turned some of these up.
Flake8 itself implements a baseline of style checks. Flake8 is also a framework for running static code checkers, which are implemented as Python packages, and called plugins.
e57c7d7 adds a number of flake8 plugins, by adding them to the requirements file and configuring them. These plugins perform additional checks — for suspicious constructs, security violations, and properly-formatted docstrings:
|mccabe||report functions with high complexity|
|flake8-bugbear||likely bugs and design problems|
|flake8-builtins||using a Python built-in as a variable name|
|flake8-docstrings||doc string format|
|flake8-comprehensions||recommend list/dict/set comprehensions|
|flake8-mock||mock methods that don’t look like they’d assert but don’t|
If you’re following along in your own project, you’ll need to either:
- Install plugins via
pip3 installfrom the command line, and then add the ones you keep to
- Add plugins to
requirements.txt, and then run
pip3 install -r requirements.txtto install them in your local Python environment.
[Or, if you’re using Pipenv, it takes care of maintaining its requirements file (
Pipenv) and your set of locally installed packages together. None of the instructions in this class refers to Pipenv, because I haven’t had a chance to use it yet.]
PEP 8 defines an order for
import statements: first system modules, then third-party modules, then modules from the current project; groups separated by spaces; imports alphabetical within a group.
flake8-isort enforces this. Commit #
e57c7d7 adds these to our project, and updates the code to follow this convention. I’ve included this separately, for a couple of reasons:
- So that the preceding commits touch a smaller number of source file lines, and it’s easier to see what’s going on.
- It’s a pain to maintain compliance with
flake8-isortunless you use tooling to do so. The
isortcommand-line tool does this, and there’s also editor plugins that keep your imports sorted. If you don’t use tools to automatically keep your source code in compliance with this coding standard, you may not want a tool that checks for compliance. (The same is true of some of the more mechanical parts of the style guide, such as how many blank lines to leave between program elements, but I’ve found it particularly true of import order.)
Using a linter is more pleasant if you get instant feedback, in the form of squiggly lines or other markup in your editor, instead of delayed feedback, with lines of harsh criticism when you remember to run a terminal command, that you then have to correlate with you source code.
Flake8 plugins or instructions for popular editors:
- Atom’s flake8 plugin uses the framework provided by the linter plugin.
- The flake8 package for Sublime Text.
- Visual Studio Code. Note that flake8 is not the default linter; you’ll have to configure Visual Studio as described here in order to switch from pylint to flake8.
Fixes to many style suggestions can be applied automatically. From the command-line, autopep8 automatically applies many PEP 8 guidelines to Python sources. (Google’s yapf is an alternative.) isort, mentioned above, sorts
$ pip3 install autopep8 isort # fix a file $ autopep8 --in-place example.py $ isort example.py # fix all the files files in a directory $ autopep8 --in-place -rc . $ isort -rc .
Better than periodically running these tools from the command line, is running them in your editor. I have them set to run every time I save a file.
Many other languages have similar formatters and import managers, that run on the command line or with editor integration.
In addition to running your test suite, a Continuous Integration server can run your linter (and, later, other code quality tools).
This has the advantage that even if you forgot to do so, someone is keeping an eye on your code quality (at least the aspects that tooling can measure), and will alert you if it degrades.
If your CI server is integrated with Github, it can also automatically run against branches and pull requests, and label the pull request as to whether it maintains whatever coding standards are codified in your linter configuration.
730c1be configured Travis to run
flake8. This is almost as simple as adding
flake8 . to the
script section of the file, but…
When I tried this, I got an error (here). Searching for the error messages revealed this discussion. I disagree with
flake8-isort’s design here, but in interest of just getting it working: commit #
79194f3 fixes the issue, by adding an empty
[isort] section to the configuration file.
Here the same sequence of changes — adding a linter, fixing the revealed violations, integrating with CI — applied to the other model projects.
Twilio → MQTT Gateway
- Commit #
flake8and my preferred plugin set, configures them, and ignores or fixes revealed violations.
- Commit #
flake8-isortand updates the
importorder in the sources.
- Commit #
446327cupdates the CI server (Travis) to run
flake8as part of integration testing.
- Commit #
17e39daupdates the README to describe
flake8as part of the development flow.
- Finally, remember that issue with
flake8-isortand Travis? Commit #
49b646cfixes that issue here. It also modifies the Travis configuration file to require
requirements.txt. This wasn’t necessary before, because Travis automatically install
pytest. And it wasn’t necessary in Bear-as-a-Service, because that project had a single requirements file. It’s necessary here, in order to tell Travis to install
flake8and its plugins (and whatever development tools we add later, that it doesn’t automatically add).
Skillz front end:
- Commit #
eslintto the package requirements, and creates a configuration file. As with the
flake8configuration file, this disables warnings that don’t match my personal style (
no-use-before-define), as well as warnings that I plan to get to later (
- Commit #
9ea6daeupdates the source files to address the revealed issues. Most of this was done automatically via
- I’m using the AirBnb style guide. It requires that React files containing JSX end in
.js. Commit #
f2de67fupdates the source files to rename the files. It’s a separate commit from the commit that changes the file contents, to make forensics easier.
Skillz back end:
Finally, Commit #
7a1a9a4 configures Travis to apply
eslint to the sources.
Course web site
- Commit #
xo —fixto fix those issues that can be fixed automatically.
- Commit #
8e2fd68fixes remaining issues.
The web site uses the React framework, which has its own style guide and linter. (This is similar to what we saw earlier for testing, and is typical of large frameworks.)
- Commit #
- Commit #
036f55efixes issues that
Flake8 is one of many Python linters. I selected it because of its extensibility, and because I’ve had issues installing the leading Python linter, pylint.
The other main Python linters include:
- Pyflakes. Flake8 includes this.
- pycodestyle. This used to be called
pep8, and you will still see it referred to thus.
- Pylama. I have no experience with this one.
As well as a number of linters, there’s even a number of linter comparisons:
- Codacy: Review of Python Static Analysis Tools
- SideCI: About style guide of python and linter tool. pep8, pyflakes, flake8, haking, Pylint
- Reddit: Which Python Linters to Use
- Slant: Pylint vs. Flake8
Many of these comparisons also include mypy, which is a static type checker. I place this in a different category, and we’ll get to type checking later.
Python has a number of near-standard linters. Other languages some fewer; some have only one. More on that in the section “
The Twilio MQTT Gateway repo distinguishes between
requirements.txt, which lists only those packages necessary to run the code, and
requirements-dev.txt, which also lists those packages required to develop the code. If we used that distinction here, we’d put
requirements.txt. ↩ ↩2