Running Drupal code reviews within Vim and other editors

If you're developing at all in Drupal, you should adhere to Drupal coding standards, because coding standards make your code more readable and hence easier for you to maintain and others to debug:

  1. All code hosted on drupal.org should follow the coding standards, and it's rightly considered a bug to be fixed when it doesn't.
  2. Even if you don't have projects on drupal.org, you might have to hand over code to other developers, or even ask help about your own code on Drupal forums: following the drupal.org standards makes all of that much easier.
  3. And even if you never show your code to anyone else, you should pick a set of standards, if only for your own sanity: using Drupal's own means your entire codebase will follow it, not just what you've written.

But: coding standards are hard to follow. I completely appreciate that: my personal blind spot is that I never put spaces after foreach or if. Worse than that, enforcing coding standards is also hard: they often include quite subtle requirements, many of them specific to Drupal's codebase and community. Enforcing can often go far beyond mere syntax highlighting, and they're difficult to re-encode afresh, every time a new editor becomes the flavour of the month. So what can you do instead? I certainly didn't want to work out how to get Vim to highlight and/or review my Drupal code itself, so instead I looked at integrating with some existing solution, to do the heavy lifting.

The coder module is a boon to developers, in that it provides a Drush command to review your code for you: at the command prompt, on a build that has coder installed, you can run drush coder-review and it will report back on some or all of the improvements you need to make, depending on the precise options you give it.

I realised that, in coder, I had the perfect tool to integrate with our editor. So here's how I configured my editor to run drush coder-review with suitable arguments, every time I saved a Drupal-looking file.

Code examples below available at Github

Here I detail exactly how my local environment works. That means I do have a running copy of this code! You can find it on my Github account: github:jpstacey/vimconfig. Please feel free to download it, clone it or even fork it and make your own changes.

Running coder-review with all the whistles and bells

Let's first look at how we might run coder-review at the command line, with as many checks switched on as possible. Here's a reasonable candidate for doing this in Bash shell scripting:

#!/bin/bash
 
# Run coder-review with standard arguments
drush coder-review $1 \
  --comment --minor --sql --style $2 \
  2>&1 | grep -v "No Problems Found" | grep -v '^$'

This takes the coder-review command, runs it on a filename (first argument), passes optional extra parameters (second argument) and then filters the output to remove anything you don't need to see.

This solves two frequent problems with running coder-review:

  1. It's not always clear how to make validation cover as many as possible of the standards.
  2. Its output is very verbose, even when there's no actual errors.

As you can see in the Github repository, I've saved this shell script as ~/.vimrc.d/personal/coder_review_command.sh, for integration with Vim below.

You can run this straight away once you save it:

cd path/to/drupal-root/
path/to/coder_review_command.sh sites/default/modules/mymodule/mymodule.module

It will provide slightly terser, but comprehensive, review output for your code.

The only Vim-specific bit

Here's how you get Vim to: detect common Drupal file extensions, turn on PHP syntax highlighting on read, and call our custom Vim function on save.

augroup drupal_files
  autocmd!
  au BufNewFile,BufRead *.module,*.install,*.test,*.inc set filetype=php
  au BufWritePost,FileWritePost *.module,*.install,*.test,*.inc call CoderVerify()
augroup END 

It uses an augroup to avoid the issue whereby an autocmd can end up registered more than once, and hence running more than once on an event. Our custom function CoderVerify looks like this:

function! CoderVerify()
  let current_file = shellescape(expand('%:p'))
  let command = "!~/.vimrc.d/personal/coder_review.sh " . current_file
  execute command
endfunction

As you can see, it references a command, and passes it the full filename of the current buffer.

Setting the same thing up in other editors

I'd love to hear how this is set up in other editors. It really does boil down to just running an external executable, which I'll show you below, on the saving of a file. Most decent editors should support this!

Here's the specific line in the Vim configuration above that you need to replicate:

~/.vimrc.d/personal/coder_review.sh CURRENT_FILENAME

Please let me know in the comments if you get this working with your own editor of choice. If you're happy for me to do so, I'll edit this section and add your suggestion (crediting you, obviously) so it's clear to other readers.

Encapsulating coder-review to be run safely from an editor

Using the shell script above, and having integrated its execution into Vim (or your own editor) then you're ready to go, right?

Well, personally, I prefer to introduce another layer between editor and coder-review. This is because there are lots of situations in which you might not want coder-review to always run: Drush might be uninstalled (or broken); Coder might not be installed for your current Drupal build; your file might not even be in a Drupal build. And also, because sometimes I inherit code that would take too much time to make fully compliant with the coding standards, sometimes I just want to blacklist certain files or folders, so they're never run through coder-review.

That means that, in between Vim's BufWritePost/FileWritePost event, and coder_review_command.sh, I insert the following shell script. I've commented it as heavily as possible, so it should hopefully be self-explanatory. Save it as coder_review.sh or clone the Github repository, to start using it.

#!/bin/bash
 
# Work out the directory containing this (and any other) script.
dir=` cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )`
 
# Whatever happens, we really need drush.
# Quit with an error code if we can't find it.
which drush >/dev/null 2>&1
if [ "$?" == "1" ]; then
  echo "Drush not installed."
  exit 1
fi
 
# Blacklist of files to ignore: check this first (on complex, slow Drupal sites,
# this can be much quicker than firing up Drush.)
#
# To run this, you'll need the executable "blacklist_with_regexes.sh" from the
# same Github repository as everything else. You'll also need a file in your
# home directory called .coder.review.blacklist containing a list of filenames
# or regular expressions that you want NOT to run coder-review on.
match=$("$dir/blacklist_with_regexes.sh" ~/.coder.review.blacklist "$1")
if [ "$?" == "1" ]; then
  echo "Coder review skipped: filename matches a blacklist regex"
  echo "Regex details: $match"
  exit 2
fi
 
# OK, so we do want to run coder-review on this filename
# To use coder, it's best to be in the directory containing the filename.
cd `dirname $1`
 
# Now we're ostensibly in a Drupal build, we can run Drush and see if
# coder-review is an available command. If not, then either it's not
# installed, or we're not actually in a full Drupal codebase!
has_coder=$(drush | grep coder-review | wc -l)
if [ "$has_coder" == "0" ]; then
  echo "Coder not installed or not enabled on this build."
  exit 3
fi
 
# Determine the root of the Drupal build, and the relative path from that
# root to the actual location of the file.
root=$(drush dd root)
file=$(echo $1 | sed -e "s^$root/^^")
 
# If the computer has PHP Code Sniffer installed on it, coder-review will
# try to check even more compliance rules. But we should check for that
# and if not, don't pass the flag to coder-review (it would throw errors.)
sniffer_flag=""
which phpcs >/dev/null 2>&1
if [ "$?" == "0" ]; then
  sniffer_flag=--sniffer
fi
 
# OK. All checks done now! run coder-review on the file!
"$dir/coder_review_command.sh" "$file" "$sniffer_flag"

Phew. Not strictly necessary, but all of that should really improve the integration between coder-review and your editor. Use it or don't, as you prefer!

Summary

Running a simple coder-review on a Drupal file is straightforward. Running coder-review with a full set of options, to make it stricter, is also fairly straightforward, but can be abstracted into a shell script.

Once you've got that shell script, you can very quickly use the autocmd/augroup command to tie that into a file/buffer save event in Vim. It should also be straightforward to integrate it with any other editor and I'd love to hear how you do that.

Between the editor, and the simple shell script, it's also potentially worth inserting another, longer shell script. This is able to tackle lots of edge cases - builds without Coder installed, builds that you just don't want to critique for whatever reason - so that everything hangs together with fewer visible seams. But that's OK too, because an example of such an intermediate script is given above.

Now, go review your code. I promise you: it'll be an eye-opening experience!

Comments

Great article! Just the thing I was looking for to help me get through a bunch of code upgrade changes.  Thanks!