Written by Cassidy Scheffer
on 

Don't Trust the Cops: Sometimes Rubocop is Wrong

My team at work recently upgraded our version of Rubocop, the popular linter used to enforce good Ruby code style. With the upgrade we got a whole bunch of new suggestions and warnings about style violations.

One of them that tripped us up was the Performance/Count rule.

According to the Rubocop docs:

This cop is used to identify usages of count on an Enumerable that follow calls to select or reject. Querying logic can instead be passed to the count call.

So in plain Ruby, the cop sees that you are filtering an array with select or reject and then calling count on it. This is inefficient because count can actually do all this work for you.

But! What if you’re in a Rails project and you have some code that looks like this?

@users.select { |u| u.admin? }.count

Rubocop will autocorrect this to:

@users.count { |u| u.admin? }

Looks okay? Well, this will likely execute a SQL count that ignores the block!!! Rubocop incorrectly assumes that @users is an Array. Why else would you be calling select on it, right?

In our case, though, @users was actually an ActiveRecord object that hadn’t been loaded yet! Since ActiveRecord lazy-loads things, sending count to this object executes an SQL COUNT that ignores the block passed in!

Here’s what it looks like in action:

User.count { |u| u.admin? }
# (0.4ms)  SELECT COUNT(*) FROM "users"
# => 320
User.count
# (0.6ms)  SELECT COUNT(*) FROM "users"
# => 320

See how we got the same number each time here? This is because ActiveRecord ignores the block passed into count without raising an error and counts all users!.

The correct thing to do here is to pass your filter parameters into a where and then do a count.

User.where(type: 'admin').count
# (0.3ms)  SELECT COUNT(*) FROM "users" WHERE "users"."type" = $1  [["type", "Admin"]]
# => 10

TL;DR