Making friends with Rubocop
Published: 2019-09-12 14:46:30

---
title: Making friends with Rubocop
published: true
tags: 
canonical_url: https://prathamesh.tech/2019/09/11/making-friends-with-rubocop/
---

At Memory.ai, we started using Rubocop heavily. This is a story of how we integrated Rubocop into our existing app.

> This is not an introductory post to Rubocop. Check out what Rubocop is before diving into our experience report.

We started with `rubocop`, `rubocop-performance`, `rubocop-rails` and `rubocop-rspec` gems. We enabled all cops by default, this was our initial configuration in `rubocop.yml`

<!--kg-card-begin: code-->
```
require:
  - rubocop-rspec
  - rubocop-rails
  - rubocop-performance

AllCops:
  EnabledByDefault: true
  TargetRubyVersion: 2.6.3
  Exclude:
    - 'app/views/**/*'
    - 'db/**/*'
    - 'bin/**/*'
    - 'csv/**/*'
    - 'slate/**/*'
    - 'vendor/bundle/**/*'
    - 'node_modules/**/*'
```
<!--kg-card-end: code-->

If you enable all the cops that come with Rubocop in an existing project, you will get tons and tons of warnings and offenses. This was pretty evident when we ran `rubocop` after adding above configuration.

### Work in progress

For existing projects, there is a better way to integrate Rubocop. Rubocop provides a file `.rubocop_todo.yml` which records all the offenses from our codebase. We can fix these offenses one by one as we touch that particular piece of code. So we don't have to fix everything to start with.

To start using the `.rubocop_todo.yml`, perform following steps.

<!--kg-card-begin: code-->
```
bundle exec rubocop --auto-gen-config
Added inheritance from `.rubocop_todo.yml` in `.rubocop.yml`.
Phase 1 of 2: run Metrics/LineLength cop
Inspecting 38 files
CC....C..C..C...C..CCC..CC....CC..C..C

38 files inspected, 40 offenses detected
Created .rubocop_todo.yml.
Phase 2 of 2: run all cops
Inspecting 38 files
CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC

38 files inspected, 104 offenses detected
Created .rubocop_todo.yml.
```
<!--kg-card-end: code-->

You will see similar output but number of offenses might be overwhelmingly large or extremely less depending on the state of your codebase :)

This command did three things.

- Inherited `.rubocop.yml` from `.rubocop_todo.yml`(We will come back to it later)
- Ran `rubocop` on all of our code
- Generated a new file `.rubocop_todo.yml`

Let's see [contents](https://gist.github.com/prathamesh-sonpatki/147e4299572235762bac6abdae555574) of the `.rubocop_todo.yml`. It has recorded all the offenses present in our code. Let's see example of one of the cop for better understanding.

<!--kg-card-begin: code-->
```
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: TreatCommentsAsGroupSeparators, Include.
# Include: **/*.gemfile,** /Gemfile, **/gems.rb
Bundler/OrderedGems:
  Exclude:
    - 'Gemfile'
```
<!--kg-card-end: code-->

This little piece of code tells us that our codebase has one offense for `Bundler/OrderedGems` cop and how we can fix it. But more importantly, it marks the file where this offense is present as **excluded** from the list of files on which Rubocop will be run.

What does this mean? Well let's try to run `rubocop` on the entire project now.

<!--kg-card-begin: code-->
```
▶ bundle exec rubocop
Inspecting 38 files
......................................

38 files inspected, no offenses detected
```
<!--kg-card-end: code-->

Woah! Our code is clean, it passes all the cops, time for party!

Well, not really. This is where the comment _Inherited `.rubocop.yml` from `.rubocop_todo.yml`_ needs to be looked at.

<!--kg-card-begin: code-->
```
▶ cat .rubocop.yml
inherit_from: .rubocop_todo.yml
```
<!--kg-card-end: code-->

`.rubocop_todo.yml` records all the offenses of our codebase and also excludes those files from the list on which `rubocop` will be run.

`rubocop.yml` inherits from `.rubocop_todo.yml` so it also excludes those files from the list on which `rubocop` will be run.

When we run `bundle exec rubocop`, it picks up configuration from `.rubocop.yml` which in turn picks up the configuration from `.rubocop_todo.yml` which has excluded all the files which have offenses. All of this results into a green output from `bundle exec rubocop` command.

So we still have offensive code that Rubocop does not like, but we have a strategy to fix it incrementally rather than fixing it all at once.

> Pro Tip: Always generate the `.rubocop_todo.yml` when you are integrating Rubocop into an existing project.

### Git workflow

Next natural step was to act on the `.rubocop_todo.yml` file and fix the offenses as we touch the existing offensive code and make sure that the new code that we write is following the cops.

We decided to add a git hook which will run Rubocop on staged changes. Rubocop provides an option `--safe-autocorrect` which automatically corrects certain pieces of code based on whether a particular cop is considered safe for autocorrect or not. We leveraged this in our git commit hook so that developers don't have to worry about manually fixing all the offenses. When the machine can do it, why not?

We used `husky` and `lint-staged` npm packages to achieve this.

<!--kg-card-begin: code-->
```
npm install --save-dev lint-staged husky
```
<!--kg-card-end: code-->

Add following in `package.json`

<!--kg-card-begin: code-->
```
{
  "scripts": {
    "precommit": "lint-staged"
  },
  "lint-staged": {
    {app,spec}/**/*.rb": [
      "bundle exec rubocop --safe-autocorrect",
      "git add"
    ]
  },
  "devDependencies": {
    "husky": "^0.13.4",
    "lint-staged": "^3.6.0"
  }
}
```
<!--kg-card-end: code-->

This configuration makes sure that whenever a developer tries to commit code the offenses in that code which are deemed safe by Rubocop and the cops which support [Auto Correct mechanism](http://docs.rubocop.org/en/stable/auto_correct/), are already fixed. After this our developers did not have to do anything extra apart from fixing offenses which are not considered safe by Rubocop manually.

### Work in progress continues

Once the autocorrect git hook was in action, our list of offenses started coming down. We had to regenerate the `.rubocop_todo.yml` in between to get the accurate list of offenses. We decided against adding regeneration of `.rubocop_todo.yml` into Git hook as it was slow in our case.

> `.rubocop_todo.yml` can be regenerated from the same command that generated it.

### Happiness or Pain?

After all of this, we were expecting the offensive code to go down and code quality to improve day by day without any bugs. But we faced few challenges.

The autocorrect hook did some unexpected changes to our code in the process.

We had a piece of code which has a method `update_attributes`. `rubocop-rails` gem has a cop `ActiveRecord/Aliases` which changes calls to `update_attributes` with `update`. Though this change should happen only on Active Record models, the cop does not check whether the method is called on Active Record model or not. So in our case, it changed the call to custom `update_attributes` to `update` but did not change the method definition. It remained `update_attributes`. This resulted into error. As we had enabled `safe-autocorrect` Git hook, the developers came to know of this only when the build failed in CI.

We also faced issues with few other cops such as `Rails/SaveBang` and `Style/StringHashKeys` where they changed the code which was supposed to be not changed.

In the end, we decided to remove the `safe autocorrect` hook and relied on manual fixing of the offensive code.

### Contributing back

When we faced the issues related to some of the cops which were not really safe for the `safe autocorrect` option, we tried to fix them by submitting patches to Rubocop.

[https://github.com/rubocop-hq/rubocop-rails/pull/101](https://github.com/rubocop-hq/rubocop-rails/pull/101)

[https://github.com/rubocop-hq/rubocop-rails/pull/98](https://github.com/rubocop-hq/rubocop-rails/pull/98)

[https://github.com/rubocop-hq/rubocop/pull/7312](https://github.com/rubocop-hq/rubocop/pull/7312)

I will encourage everyone to do the same as it makes Rubocop better for everyone.

In conclusion, I will list down what we learned from this adoption.

- Incremental adoption is key.
- Autocorrect can be tricky, depends on your test coverage and lot of other factors, so don't use it if you don't have to.
- Disable the cops that your team does not agree upon. We disabled cops such as `RSpec/AnyInstance`, `RSpec/ExpectInHook`, `RSpec/AlignRightLetBrace`
- Contribute back to the community with your findings, code fixes so that critical tools such as Rubocop become better for everyone.

Want to know how we do Ruby and Rails at Memory.ai, subscribe to my newsletter [here](https://prathamesh.tech/mailing-list).