Linting Your Code

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 lint. lint examines C code for programming constructs commonly associated with programming errors. Modern linters also examine your code for consistency and style, either relative to style guides that are part of the language definition (Python’s PEP 8 and PEP 257, Elm and Go’s built-in formatters), or to “house” style guides published by organizations that do significant work with the language (the Google and Numpy Google docstring guides, AirBnB’s React and JavaScript style guides).

This post is mostly about Python tools. The concepts apply to other languages, just the names of the tools are different. A couple of the example projects use JavaScript instead of Python.

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

We’ll configure a linter, flake81, to run against the Bear-as-a-Service repo. We’ll configure flake8, fix some of the issues that it reports, and mark some issues to revisit later.

pip3 install flake8 installs the flake8 tool.

Commit #f820de1 adds flake8 to 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.

Adding flake8 to the requirements file also ensures that it’s available on the CI.2 We’ll return to this later.

First run

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.cfg file. Commit #303c296 implements this.
  • A particular violation can be ignored by placing a special comment, for example #noqa: E402, on the line that causes the violation. Commit #b3f1b3b has 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.cfg is the ignore for D100 “Missing docstring in public module”. I was already thinking I should at least add a module comment to each file; flake8 reminds 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 = 120 in 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.path and then continuing the import sequence, 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.)

Fixing issues

Commit #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.

Adding plugins

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.

Commit #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:

Package Description
mccabe report functions with high complexity
flake8-bandit security testing
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 install from the command line, and then add the ones you keep to requirements.txt; or
  • Add plugins to requirements.txt, and then run pip3 install -r requirements.txt to 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.]

Sorting imports

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-isort unless you use tooling to do so. The isort command-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.)

Editor Integration

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:

Automatic Fixes

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 import statements.

$ 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.

Formatters:

Import sorters:

Many other languages have similar formatters and import managers, that run on the command line or with editor integration.

CI 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.

Commit #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.

Other projects

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 #54a4d03 adds flake8 and my preferred plugin set, configures them, and ignores or fixes revealed violations.
  • Commit #76c6e46 adds flake8-isort and updates the import order in the sources.
  • Commit #446327c updates the CI server (Travis) to run flake8 as part of integration testing.
  • Commit #17e39da updates the README to describe pytest and flake8 as part of the development flow.
  • Finally, remember that issue with flake8-isort and Travis? Commit #49b646c fixes that issue here. It also modifies the Travis configuration file to require requirements-dev.txt instead of 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 flake8 and its plugins (and whatever development tools we add later, that it doesn’t automatically add).

Skillz

The Skillz project is written in JavaScript, and uses the standard JavaScript linter, eslint. You’ll see clear parallels with Python.

Skillz front end:

  • Commit #25373be adds eslint to the package requirements, and creates a configuration file. As with the flake8 configuration 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 (jsx-a11y/click-events-have-key-events).
  • Commit #9ea6dae updates the source files to address the revealed issues. Most of this was done automatically via eslint —fix.
  • I’m using the AirBnb style guide. It requires that React files containing JSX end in .jsx, not .js. Commit #f2de67f updates 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

xo is a wrapper for eslint, that comes with its own style guide and an alternate mechanism for configuration. Commit #5d52a29 adds xo to the project and configures it.

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.)

flake8 alternatives

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:

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.

  1. Python has a number of near-standard linters. Other languages some fewer; some have only one. More on that in the section “ flake8 alternatives”. 

  2. 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 flake8 in requirements-dev.txt, not requirements.txt 2