[Fix #499] Add IgnoreWhereFirst to Rails/FindBy cop

Fixes #499.

This PR add `IgnoreWhereFirst` option (`true` by default) to `Rails/FindBy` cop.

CHANGELOG.md

@@ -18,6 +18,7 @@
 ### Changes
 
 * [#288](https://github.com/rubocop/rubocop-rails/issues/288): Add `AllowToTime` option (`true` by default) to `Rails/Date`. ([@koic][])
+* [#499](https://github.com/rubocop/rubocop-rails/issues/499): Add `IgnoreWhereFirst` option (`true` by default) to `Rails/FindBy`. ([@koic][])
 
 ## 2.10.1 (2021-05-06)
 

config/default.yml

@@ -307,6 +307,8 @@ Rails/FindBy:
   StyleGuide: 'https://rails.rubystyle.guide#find_by'
   Enabled: true
   VersionAdded: '0.30'
+  VersionChanged: '2.11'
+  IgnoreWhereFirst: true
   Include:
     - app/models/**/*.rb
 

docs/modules/ROOT/pages/cops_rails.adoc

@@ -1540,29 +1540,52 @@ Rails.root.join('app/models/goober')
 | Yes
 | Yes
 | 0.30
-| -
+| 2.11
 |===
 
-This cop is used to identify usages of `where.first` and
-change them to use `find_by` instead.
+This cop is used to identify usages of `where.take` and change them to use `find_by` instead.
+
+And `where(...).first` can return different results from `find_by`.
+(They order records differently, so the "first" record can be different.)
+
+If you also want to detect `where.first`, you can set `IgnoreWhereFirst` to false.
 
 === Examples
 
 [source,ruby]
 ----
 # bad
-User.where(name: 'Bruce').first
 User.where(name: 'Bruce').take
 
 # good
 User.find_by(name: 'Bruce')
 ----
 
+==== IgnoreWhereFirst: true (default)
+
+[source,ruby]
+----
+# bad
+User.where(name: 'Bruce').first
+----
+
+==== IgnoreWhereFirst: false
+
+[source,ruby]
+----
+# good
+User.where(name: 'Bruce').first
+----
+
 === Configurable attributes
 
 |===
 | Name | Default value | Configurable values
 
+| IgnoreWhereFirst
+| `true`
+| Boolean
+
 | Include
 | `app/models/**/*.rb`
 | Array

lib/rubocop/cop/rails/find_by.rb

@@ -3,29 +3,36 @@
 module RuboCop
   module Cop
     module Rails
-      # This cop is used to identify usages of `where.first` and
-      # change them to use `find_by` instead.
+      # This cop is used to identify usages of `where.take` and change them to use `find_by` instead.
+      #
+      # And `where(...).first` can return different results from `find_by`.
+      # (They order records differently, so the "first" record can be different.)
+      #
+      # If you also want to detect `where.first`, you can set `IgnoreWhereFirst` to false.
       #
       # @example
       #   # bad
-      #   User.where(name: 'Bruce').first
       #   User.where(name: 'Bruce').take
       #
       #   # good
       #   User.find_by(name: 'Bruce')
+      #
+      # @example IgnoreWhereFirst: true (default)
+      #   # bad
+      #   User.where(name: 'Bruce').first
+      #
+      # @example IgnoreWhereFirst: false
+      #   # good
+      #   User.where(name: 'Bruce').first
       class FindBy < Base
         include RangeHelp
         extend AutoCorrector
 
         MSG = 'Use `find_by` instead of `where.%<method>s`.'
         RESTRICT_ON_SEND = %i[first take].freeze
 
-        def_node_matcher :where_first?, <<~PATTERN
-          (send ({send csend} _ :where ...) {:first :take})
-        PATTERN
-
         def on_send(node)
-          return unless where_first?(node)
+          return if ignore_where_first? && node.method?(:first)
 
           range = range_between(node.receiver.loc.selector.begin_pos, node.loc.selector.end_pos)
 
@@ -38,9 +45,6 @@ def on_send(node)
         private
 
         def autocorrect(corrector, node)
-          # Don't autocorrect where(...).first, because it can return different
-          # results from find_by. (They order records differently, so the
-          # 'first' record can be different.)
           return if node.method?(:first)
 
           where_loc = node.receiver.loc.selector
@@ -49,6 +53,10 @@ def autocorrect(corrector, node)
           corrector.replace(where_loc, 'find_by')
           corrector.replace(first_loc, '')
         end
+
+        def ignore_where_first?
+          cop_config.fetch('IgnoreWhereFirst', true)
+        end
       end
     end
   end

spec/rubocop/cop/rails/find_by_spec.rb

@@ -1,16 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.describe RuboCop::Cop::Rails::FindBy, :config do
-  it 'registers an offense when using `#first` and does not auto-correct' do
-    expect_offense(<<~RUBY)
-      User.where(id: x).first
-           ^^^^^^^^^^^^^^^^^^ Use `find_by` instead of `where.first`.
-    RUBY
-
-    expect_no_corrections
-  end
-
-  it 'registers an offense when using `#take`' do
+  it 'registers and corrects an offense when using `#take`' do
     expect_offense(<<~RUBY)
       User.where(id: x).take
            ^^^^^^^^^^^^^^^^^ Use `find_by` instead of `where.take`.
@@ -21,28 +12,47 @@
     RUBY
   end
 
+  context 'when using safe navigation operator' do
+    it 'registers an offense when using `#take`' do
+      expect_offense(<<~RUBY)
+        users&.where(id: x).take
+               ^^^^^^^^^^^^^^^^^ Use `find_by` instead of `where.take`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        users&.find_by(id: x)
+      RUBY
+    end
+  end
+
   it 'does not register an offense when using find_by' do
     expect_no_offenses('User.find_by(id: x)')
   end
 
-  it 'autocorrects where.take to find_by' do
-    new_source = autocorrect_source('User.where(id: x).take')
+  context 'when `IgnoreWhereFirst: true' do
+    let(:cop_config) do
+      { 'IgnoreWhereFirst' => true }
+    end
 
-    expect(new_source).to eq('User.find_by(id: x)')
+    it 'does not register an offense when using `#first`' do
+      expect_no_offenses(<<~RUBY)
+        User.where(id: x).first
+      RUBY
+    end
   end
 
-  it 'does not autocorrect where.first' do
-    new_source = autocorrect_source('User.where(id: x).first')
-
-    expect(new_source).to eq('User.where(id: x).first')
-  end
+  context 'when `IgnoreWhereFirst: false' do
+    let(:cop_config) do
+      { 'IgnoreWhereFirst' => false }
+    end
 
-  context 'when using safe navigation operator' do
-    it 'registers an offense when using `#first`' do
+    it 'registers an offense when using `#first` and does not auto-correct' do
       expect_offense(<<~RUBY)
-        User&.where(id: x).first
-              ^^^^^^^^^^^^^^^^^^ Use `find_by` instead of `where.first`.
+        User.where(id: x).first
+             ^^^^^^^^^^^^^^^^^^ Use `find_by` instead of `where.first`.
       RUBY
+
+      expect_no_corrections
     end
   end
 end