Automatically running linters on commit

By:

on

July 16, 2015

This post is a follow-up to last week's post, Conforming to coding standards with linters.

As we learned last week, linters are tools that you can use to check if a file contains any syntax errors, and/or whether it conforms to coding standards. This blog post discusses how to ensure that linters get run automatically when you make a commit.

If you find a better process, please blog about it and post a link in the comments!

This tutorial assumes:

  • You write or modify code,
  • You have linters to run on your code, and,
  • You use Git to manage your code.

Git hooks

Git has a way to fire off custom scripts when certain events happen (e.g.: when you commit) called Git Hooks. Note that, while conceptually similar, Git hooks are unrelated to Drupal hooks. A git hook is simply a shell script with a certain name that sits in a repository's .git/hooks/ folder.

The hook named pre-commit (i.e.: .git/hooks/pre-commit) is run when you try to run git commit, but before you enter a commit message (or, if you run git commit -n -m "Commit message.", before the commit is written to the repository). If the pre-commit script exits with an error code (i.e.: non-zero), then the commit will be aborted. This means that you can use a pre-commit hook to enforce that a certain set of linters pass before the code can be committed.

icefox/git-hooks

Managing a git hook script can be difficult, however, because:

  • You have to manually set it up on every clone of the repository (i.e.: each person's machine),
  • There's no way to share it, except by version-controlling it separately, and,
  • It gets complicated if you want to run more than one check (i.e.: linter), because:
    • There can be only one pre-commit script per repository, and therefore,
    • The script ends up becoming a mess of chained-if statements that gets more complex with each check you add.

Fortunately, there's a tool called icefox/git-hooks which is designed to make it easy to version-control and manage Git hooks in simple, bite-sized scripts (i.e.: one script per linter that you want to run).

Installing it is easy: simply clone it and add it to your PATH (for full installation instructions, check out it's README).

Once icefox/git-hooks is installed, you must connect it to each repository that you want to use it on by running git hooks --install. Don't forget to run git hooks --install after cloning a repository too! git hooks --install replaces each one of the hooks in .git/hooks/ with a stub that searches for hooks managed by icefox/git-hooks, and runs them if it finds them.

Version-controlled, per-project hooks

Besides making it easy to run more than one check without convoluted scripts, icefox/git-hooks also makes it easy to set up and share a set of git hooks on a per-project basis. It does so by storing the scripts inside the repository they are intended to run on. For example, to add pre-commit hooks to a project, add a folder named git_hooks to the root of the repository, add a pre-commit folder inside it, add your shell scripts in that folder, and commit them:

    cd /path/to/repositoryroot
    mkdir -p git_hooks/pre-commit
    edit git_hooks/pre-commit/my_hook.sh
    git add git_hooks/pre-commit/my_hook.sh
    git commit

If you need to change the order that hooks run in, add a number to the beginning of the script's filename (i.e.: a git hook named 1-foo will run before 2-bar which will run before baz).

Other hook scopes

icefox/git-hooks also lets you set up hooks to run globally (i.e.: all repositories initialized with git hooks --install on a machine), and per-user (i.e.: all properly-initialized repositories that you interact with). You might want this if, say, you don't want to commit OS cache files or resource forks (e.g.: Thumbs.db or .DS_Store), or you want to make sure you don't commit files with CRLF line-endings (although, in the latter case, you could use a .gitattributes file to fix them automatically instead of throwing errors).

Since you need super-user privileges to set up global hooks, I never use them: I use per-user hooks instead. But on a machine with a shared login, global hooks could be useful.

For example, to add per-user pre-commit hooks to your user account, add a .git_hooks folder to your home directory, add a pre-commit folder inside it, and add your shell scripts in that folder:

    mkdir -p $HOME/.git_hooks/pre-commit
    edit $HOME/.git_hooks/pre-commit/my_hook.sh

Choosing pre-commit hooks

To this point, we've (intentionally) only talked about icefox/git-hooks' hooks in the abstract. What do they actually look like, and what can you do with them?

icefox/git-hooks has both it's own git hooks and some contributed hooks which you can use as examples, but they're not all that useful for Drupal developers.

I've put together a collection of hooks that I run on my own (Drupal) projects. Note, however, that my collection of linters includes both linters that check for syntax errors (whose filenames are prefixed with 1-), and linters that check for coding standards violations (whose filenames are unprefixed).

The general structure of a hook looks like this:

#!/usr/bin/env bash

function test_file {
    # Test (run a linter on) a single file.
}

case "${1}" in
  # If icefox/git-hooks asks this hook to give more information about itself...
  --about )
    echo "Information about this hook."
    ;;

  # Otherwise, lint the file.
  * )
    # For each file which is about to be committed whose name matches certain pattern, test that file.
    for file in $(git diff-index --cached --name-only HEAD | grep -E '\.(php|inc|module|install|profile|test)') ; do
      test_file "${file}"
    done
    ;;
esac

Hopefully, the comments in the sample will make it clear how it works, but it is important to note that linters are run on the whole file, not just the parts you changed in a commit. This can be unexpected and annoying at first, especially if you run coding-standards linters in a pre-commit hook (because, if any part of the file violates coding standards, the commit will fail).

Should you automatically check for coding standards violations?

The last two sections have (sort-of) ended on the question, "Should you check for coding standards violations in your pre-commit hooks?" Unfortunately, there isn't an easy answer:

  • Running git pre-commit hooks is all-or-nothing: if any one of the linters fails then the commit will fail; put another way, all linters must pass for the commit to succeed,
  • That being said, in BASH, there is a way to make a linter fail without causing the pre-commit hook to fail (by running set +e before calling the linter and set -e after the linter) — see the example repository of hooks from earlier,
  • Skipping git pre-commit hooks is all-or-nothing: if you choose to skip the linters, you skip all of them, even the required syntax linters (more about this below), and,
  • As mentioned in the previous article, you never want to commit code with syntax errors, but you might want to (or rather, have no choice but to) commit code that violates coding standards.

Ultimately, I think this should be up to the development team to discuss and decide whether they want to enforce coding standards this rigorously. Some arguments that you can use to advocate for putting coding-standards linters in your pre-commit hooks:

  • All new code must conform to coding standards in order to be committed
  • If your team only wants to enforce the highest-severity problems, most linters allow you to ignore violations below a certain severity
    • However, your team might disagree with the linter maintainers about the severity of certain coding standards, and there isn't always an easy way around this.

      For example, at time-of-writing, the Drupal coding standard lints consider the standard "Doc comment short description must be on a single line, further text should be a separate paragraph" to be the same severity as "Using the e [i.e.: evaluate PHP] flag in [a regular expression] is a possible security risk." (both are error-level severity).

  • It is possible to skip the linters for certain commits (more about this below)
    • It would be worth discussing exactly which circumstances would make it appropriate to skip the linters, and whether certain linters should still be run manually.

Finally, if the team doesn't want to enforce coding standards this rigorously, but you do want to discipline yourself, remember that you can always add the coding-standards lints to your $HOME/.git_hooks/ folder.

Skipping pre-commit hooks (be careful!)

In some cases (e.g.: if you must commit contributed code to your repository), you might want to skip the pre-commit hooks (i.e.: skip running the linters). If you are considering this, keep in mind that all checks on all files will be skipped: especially if you are linting for coding standards violations, there might be other files staged for commit which you will need to run linters (especially: syntax linters) on manually.

Once you've done your due dilligence on the other files staged for commit, you can pass the -n flag to git-commit to skip all pre-commit hooks (e.g.: git commit -n -m "Commit message.").

Add new comment

Plain text

  • No HTML tags allowed.
  • Lines and paragraphs break automatically.