arcanist - lint, unit test, and submit for code review

I briefly mentioned arcanist in a previous article. It is a command line tool that wraps around git, hg, and svn and provides some handy features for working on a shared codebase. It is meant to be used in conjunction with phabricator, and all my examples assume that situation. As I mentioned previously, a typical workflow while using arc might look something like this:

1. Create a local branch git checkout -b mygreatfeature
2. hack on that branch
3. Run arc lint to run your changes through several possible code linters
4. Run arc unit to run the unit tests that are associated with your changes
5. Run arc diff to submit your patch for review

(Note: just running arc diff will call both the lint and the unit test steps if you have them configured)

arc lint

I have arc configured to run php codesniffer to enforce a common coding standard, a text linter to warn about line-length violations, a spelling linter to look for often misspelled words, a php syntax checker to be sure .php files are actually valid and free of syntax errors and finally xhpast an abstract syntax tree for PHP which can target a specific version of the language. This way, if someone calls a function that’s available in 5.4 but we’re currently targeting 5.3, arc will let us know.

All this helps me, and my team be sure that the code we submit for review is as close as possible to our shared standard, and catches some common errors early, like using a variable that’s not defined.

Arc runs the whole file through each linter, and does one of two things depending on whether the violation it encounters is an error or a warning. If there is an error anywhere in the affected file, arc will report that to you and ask you to deal with it before submitting your patch. If there is a warning arc will only report it if it’s in the section of code that you changed. The reasoning for this is that errors should be show-stoppers anywhere in the file, while warnings are probably fine as long as they aren’t in the code you just changed.

I find this makes for a good balance, though you may have to override some of your errors. In our phpcs setup for example, EVERYTHING is an error - this makes for some disgustingly long arc lint output, and causes developers to simply ignore the warnings, and force the change through. Luckily, this is easy to configure. I was able to just tell arcanist that any error from phpcs should be treated as a warning. You can get very fine grained with this as well, if you only want to suppress certain errors, it’s very easy to do. Since I don’t have control of our phpcs standard, and can’t easily change those errors at the source, the next best option is being able to tell arcanist to just treat them as warnings.

A nice feature of several of the basic linters provided with arcanist, is the ability to suggest auto-fixes for your code. For example, if by convention you should have newlines at the end of every file, arcanist will offer to add them for you. Another example is if you use spaces instead of tabs, or MSDOS style line endings instead of UNIX, arcanist can also automatically adjust your commit so it conforms with the shared standard. Obviously this can’t work for all types of errors, but I’ve found that it’s quite helpful in the cases I’ve mentioned.

Aside from the linters that come with arc, you can write your own custom linters that will enforce just about anything you can imagine. Think all function names should be in pig latin? Arcanistway ancay elphay ouyay ithway atthay.

I’ll explore more of arc’s features in a future article, as this is already getting quite long in the tooth.

Here’s a simple .arclint file that configures a linter for the go programming language, and runs it on any change that’s made in a file that ends in .go

{
    "linters": {
        "golint": {
            "type": "golint",
            "include": "(\\.go$)"
        }
    }
}