RuboCop Isn't Just Annoying Style Suggestions That Make You Want to Curse

During a RailsConf Keynote Interview, DHH noted that if their's one thing he swears at the most in Ruby, it's probably RuboCop.

I understand the sentiment. Many of the RuboCop style suggestions are annoying. Ruby is expressive. It's good to have the freedom to write statements in many different ways.

But I also really like RuboCop. It's not just about formatting. The Lint checks have helped me understand some quirks of Ruby. And they regularly help me quickly catch bugs.

Here are some examples.

Lint/Void

What does this method return?

def sum
  1
  + 2
end

The answer is 2! The problem is that the first line in this method is used within a void context and doesn't do anything. RuboCop will warn us about this with:

Lint/Void: Literal 1 used in void context.
  1
  ^

Lint/DuplicateMethods

Sometimes a duplicate method can slip into a file. This can occur when the file is large and you lose track of the methods that already exist. It can also happen when merge conflicts aren't handled correctly.

def my_method
  'a'
end

def my_method
  'b'
end

But RuboCop can help point out that there's a problem:

Lint/DuplicateMethods: Method Object#my_method is defined at both example.rb:1 and
example.rb:5.
def my_method
^^^^^^^^^^^^^

Lint/EnsureReturn

Say I have code that uses ensure to do some necessary clean-up after an error has occurred, but I still want the exception to be raised so I don't use rescue:

def do_something
  raise 'bug!'
ensure
  puts 'Doing some clean-up here'
  return
end

There's a problem here. Because I put an explicit return within the ensure block, the exception will be lost!

RuboCop will give some good guidance about this:

Lint/EnsureReturn: Do not return from an ensure block.
  return
  ^^^^^^

Lint/NonLocalExitFromIterator

Say I want to process a set of items but skip those that qualify for some condition:

def process_items
  items.each do |item|
    return if item.skip?

    item.process
  end
end

Oops. This code will work properly for some sets of items. But it will completely bail out of the method when it encounters the first item where skip? is true because I accidentally used return instead of next. RuboCop can spot this:

Lint/NonLocalExitFromIterator: Non-local exit from iterator, without return value.
next, break, Array#find, Array#any?, etc. is preferred.
    return if item.skip?
    ^^^^^^

Lint/UselessAssignment

Aren't tests going to catch all of my mistakes? Maybe. But what if I make a mistake in my test code?

I have this method:

def show_if_positive(number)
  return if number < 0

  number
end

And I test it with:

value1 = show_if_positive(-1)
assert_nil value1, 'Negative should return nil'

value2 = show_if_positive(0)
assert_nil value1, 'Zero should return nil'

value3 = show_if_positive(1)
assert_equal 1, value3, 'Positive should return a number'

The test assertions will pass. But I didn't actually test the value2 result. RuboCop will let me know that I made a variable assignment that I didn't actually use:

Lint/UselessAssignment: Useless assignment to variable - value2.
Did you mean value3?
      value2 = show_if_positive(0)
      ^^^^^^

RuboCop gets a bit of a bad rap. But I'd recommend it for the Lint checking.

Avoid the RuboCop curse. Disable the Style checks that you disagree with, but keep the Lint checks on to help track down bugs.