App-specific feedback with RuboCop

RuboCop is a brilliant static analysis tool for the Ruby world. It’s used in nearly every Ruby project I’ve seen and has tons of built-in cops to detect common bugs / styling issues and prevent reviewers from having to point them out in PRs.

At One Medical I’ve taken this one step further: there are a number of comments I make regularly on PRs that could be coded into RuboCop, but don’t actually make sense in an open-source gem. Sometimes they involve deprecating a specific coding pattern that we’ve used in the past but no longer want to spread; other times it’s guarding against a strange bug that’s specific to our setup.

Either way, coding these rules into RuboCop has been really powerful for us: a robot can respond to more PRs than I can, and we’ve used it to prevent real bugs from shipping. Our solution is comprised of a couple components: first, the cops themselves, and second, using Pronto to post the cop results to the pull request.

Creating your internal gem

Before you can write your cop, you’ll want to create a home for those cops. Existing RuboCop extensions are a great reference point. Here are the steps I followed:

1. Create your gem — follow Bundler’s guide. Name your gem rubocop-<company name> (this follows the naming convention for gem extensions).
2. Copy this cops_documentation.rake file from one of the RuboCop extensions. You probably don’t need to make any changes to this file; as it stands it will write documentation to ./manual/*.md. Do make sure you require the documentation rake file from your Rakefile: Dir[“tasks/**/*.rake”].each { |t| load t }
3. Add a CI step to make sure cops documentation is committed¹.
4. Create cops in lib/rubocop/cop/<company name>/ .
5. Make it easy for other devs to contribute by lowering the barrier to adding a new cop. Here’s a rake task I wrote that generates some scaffolding.

¹ CI step to ensure cops documentation is committed:

After that your gem should be up-and-running. Incorporate it into your main project by loading a custom extension and adding the gem to your Gemfile and you should be good-to-go!

If only. We use Code Climate to help track code quality, and to run RuboCop rules. Unfortunately it does not (at the time of writing) support custom extensions, so you have to write your own CI step to run your rules. Here’s where Pronto helps a lot: it lets you turn those cops into PR comments, making it easier for developers to act on feedback, or (when appropriate) dismiss it entirely. Other devs don’t have to dig through build logs, nor do you have to fail a build just because of a cop that’s more of a suggestion than a requirement.

Pronto’s GitHub integration and RuboCop extension are all fairly straightforward. Once you authorize it and configure it to run your cops against the diff, you’ll get automated feedback straight in the pull request!

Creating your first Cop

I won’t go into too many details about how to write cops. Reading these three key bits of documentation (general cop writing, the node pattern, and node pattern syntax “cheat sheet”) should help a ton. I will, however, dive into how to pick what to write a cop about.

One less-obvious win is anytime there’s a pattern of code that you want to deprecate, you can probably target it with a cop. A while ago some devs established a pattern of testing our GraphQL code with public_send and dynamically-created tests:

subject { execute_graphql_code(user).public_send(field) }

%w(field_one field_two).each do |field|
  it field do
    # …
  end
end

After getting agreement from others that this wasn’t a good practice, and deciding there were too many violations to fix in a reasonable time frame, I opted to write a cop! Then anytime another engineer unwittingly duplicated this pattern, they’d get automatic feedback telling them to change course!

Another example is that GraphQL code would often get committed without pagination. I wrote an extremely simple cop that looks for lists of records getting returned and will ask users if this needs to be paginated.

Yet another is this cop I wrote for the open-source gem that unfortunately was rejected: Style/InstanceVarsInClass. Its goal is to prevent the use of instance variables in favor of attr_readers. Even though it might not be a great idea for RuboCop core, it’s still useful for my company.

Conclusion

It’s always better to contribute to an open source project than it is to create your own internal-only rules. But some rules aren’t accepted, or aren’t applicable, widely. Don’t let that stop you from implementing programmatic feedback in your PRs! It’s an extremely powerful way to limit the spread of bad practices or call out common “gotchas” in your codebase. And maybe down the road some of those cops will evolve into something you can contribute back to RuboCop core.