Automating Style In Clojure

We do everything we can to improve code quality. Our process includes rigorous code reviews focused on getting the correct level of abstraction, modularity, and reusability. We quickly realized that nitpicking code format and line length was distracting us from our goals. It isn’t that those aspects aren’t important, but that they should be standardized so we can spend our time on more interesting questions and problems.

To keep ourselves focused on the meaningful aspects of the code, we run a suite of automated analysis tools in our CI. This gives us a higher level of consistency and predictability before a branch gets to code review.

The Checks We Run

To get a green build, all code must pass both static analysis and code style checks. Due to plugin availability the checks that we run depend on the code’s target platform, i.e. whether it is written in Clojure, ClojureScript, or uses CLJC. Currently, we use the following tools to run checks:

• bikeshed checks for lines longer than 80 characters, extra whitespace, and files ending with blank lines. It has additional checks, but these are parameterized and disabled by default in our fork. We will make a pull request for our fork after changing it to match the upstream defaults.
  • Supports Clojure, ClojureScript, and CLJC.
  • lein bikeshed runs these checks locally.
• cljfmt is a formatter that checks for whitespace, indentation, and extra newlines. It will rewrite to the correct format with lein cljfmt fix. If using fix, be sure to run lein bikeshed after to check if the new format exceeded the maximum line length.
  • Supports Clojure and ClojureScript.
  • Unfortunately, cljfmt does not read CLJC files properly. Because of this we cannot use it for CLJC and have configured it to ignore CLJC files. After a bit of investigation it appears the problem here is with the rewrite-clj library incorrectly parsing the #? and #@? directives in CLJC files.
  • lein cljfmt check runs these checks locally.
• eastwood is a linter that checks for unused vars, namespaces, locals, args, etc. We have configured it to exclude some files (see our project.clj snippet below) that these requirements were too strict for.
  • Supports Clojure.
  • lein eastwood runs these checks locally.
• kibit is a static analyzer that checks for idiomatic code.
  • Supports Clojure and ClojureScript.
  • lein kibit runs these checks locally.

lein Project Configuration

The below snippet shows the part of our project.clj that configures the linters for our use case. To avoid problems parsing .cljc files, the first section restricts cljfmt to only evaluate .clj and .cljs files. The second section removes the constant-test linter from eastwood (because it leads to spurious warnings after macro-expansion for our code base) and adds a number of variable checking linters. We also instruct it to only check our source files (by default it will also look at test files) and ignore some namespaces.

  ...
  :cljfmt {:file-pattern #"[^\.#]*\.clj[s]?$"}
  :eastwood {:exclude-linters [:constant-test]
             :add-linters [:unused-fn-args :unused-locals :unused-namespaces
                           :unused-private-vars]
             :namespaces [:source-paths]
             :exclude-namespaces [...]}
  ...

Configuration for Continuous Integration

To run these checks in our CI system, we first had a Midje test that executed them as shell commands. This meant that all the tests would run regardless of whether the style checks passed — which was good because you could see all the results at once, but bad because it slowed down the Midje tests.

We realized the problems — the slowness and the inflexibility of grouping style checks with our other Midje tests — outweighed the benefits. We removed the Midje test and added the appropriate lein commands to our Drone configuration.

  - lein bikeshed
  - lein cljfmt check
  - lein eastwood
  - lein kibit

The CI systems calls these before running the Midje and then ClojureScript tests. If any of them fail, the build fails immediately.

Running with git hooks

To help avoid the frustration of pushing up a build only to see the style checks fail Okal wrote some git hooks. Developers can configure these hooks to automatically run bikeshed, kibit, and cljfmt before pushing.

#!/usr/bin/env sh

RED='\033[0;31m'
STOP_FORMATTING='\033[0m'
GREEN='\033[0;32m'
BOLD='\033[0;1m'

if [ "$DO_STYLE_CHECKS" == "true" ]; then
    echo 'Style checks are enabled. Unset $DO_STYLE_CHECKS to disable them'
    echo "${BOLD}Running lein bikeshed${STOP_FORMATTING}"

    lein bikeshed ||
    { echo "${RED}lein bikeshed failed${STOP_FORMATTING}\n"; exit 1; }
    echo "${BOLD}Running lein kibit${STOP_FORMATTING}"

    lein kibit ||
    { echo "${RED}lein kibit failed${STOP_FORMATTING}"; exit 1; }

    echo "${BOLD}Running lein cljfmt check${STOP_FORMATTING}"
    lein cljfmt check ||
    { echo "${RED}lein cljfmt check failed${STOP_FORMATTING}"; exit 1; }

    echo "\n${GREEN}Style checks passed${STOP_FORMATTING}\n"
    exit 0
else
    echo 'Skipping style checks. Set $DO_STYLE_CHECKS to true to enable them'
    exit 0
fi

Putting the above in .git/hooks/pre-push and enabling with export DO_STYLE_CHECKS=true will prevent you from pushing if your code has style issues. To push while temporarily bypassing style checks (assuming you have them enabled using the environment variable) pass the --no-verify flag when running git push.