Rubocop: Rails/FindBy doesn’t look at arguments to `where`

[bquorning]

Expected behavior

Rails/FindBy should not find any offense when the argument to where is a variable or a method call.

Actual behavior

Rails/FindBy tells me to change my method call complex_arel_query to find_by, although that may not be possible.

Steps to reproduce the problem

# test.rb
def foo
  Bar.where(complex_arel_query).first
end

» bin/rubocop --only Rails/FindBy test.rb
Inspecting 1 file
C

Offenses:

test.rb:2:7: C: Use find_by instead of where.first.
  Bar.where(complex_arel_query).first
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

RuboCop version

Include the output of rubocop -V. Here's an example:

» rubocop -V
0.47.1 (using Parser 2.4.0.0, running on ruby 2.3.3 x86_64-darwin15)

[pocke]

although that may not be possible.

In which case can we not replace with find_by?

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation/finder_methods.rb#L77-L81

I guess we can replace it in all cases, because find_by and where.take are almost the same.

[bquorning]

Yeah, I see now that my reproduction example was too simplified. The offending code is more like this:

def foo
  Bar.where(a: 42).
    where(b: "hello world").
    where(complex_arel_query_1).
    where(complex_arel_query_2).
    first
end

So, the problem is more the chaining than the argument type. Sorry about the confusion.

But yeah, you can always change the last where call to a find_by, but is that recommendable here?

def foo
  Bar.where(a: 42).
    where(b: "hello world").
    where(complex_arel_query_1).
    find_by(complex_arel_query_2)
end

I think it looks (slightly) more confusing than just using a chain of wheres.

[bquorning]

cc @smakagon who is working on a fix already (#4099).

[smakagon]

@bquorning I'm working on that (#4099)

[bquorning]

@smakagon I was thinking, perhaps you should wait with the fix until we finished discussing what the issue actually is ;-)

Bar.find_by(complex_arel_query) is actually ok with me, now that I’ve realized that it works as expected. The problem is what to do when chaining where calls.

[smakagon]

@bquorning ok, good. That makes sense. If you can use complex_arel_query in find_by - there is no problem with current behavior.

[bquorning]

Except that this looks strange, I think:

def foo
  Bar.where(a: 42).
    where(b: "hello world").
    where(complex_arel_query_1).
    find_by(complex_arel_query_2)
end

I think, in this case, that it is more explicit what I’m doing, by calling where 4 times and then first at the end.

[bbatsov]

We're in the process of moving all Rails-related functionality to a standalone library (gem) named rubocop-rails. Please, migrate this issue to its issue tracker https://github.com/rubocop-hq/rubocop-rails/issues/