From dc6b3cbeffdfcc674a92e019e31fbdc78083e656 Mon Sep 17 00:00:00 2001
From: Koichi ITO <koic.ito@gmail.com>
Date: Tue, 27 Apr 2021 00:43:51 +0900
Subject: [PATCH] Fix a false positive for `Rails/DynamicFindBy`

Fixes https://github.com/rubocop/rubocop-jp/issues/54 (Ja)

This PR fixes a false positive for `Rails/DynamicFindBy`
when not inheriting `ApplicationRecord` and without no receiver.

A dynamic matcher without receiver is only possible if it inherits
`ApplicationRecord` (`ActiveRecord::Base`).

For example, Capybara's `find_by_id` and Token's `find_by_valid_token`
should be accepted by default.

- https://www.rubydoc.info/github/jnicklas/capybara/Capybara/Node/Finders#find_by_id-instance_method
- https://github.com/fnando/tokens
---
 CHANGELOG.md                                  |  1 +
 lib/rubocop/cop/rails/dynamic_find_by.rb      |  3 +-
 .../rubocop/cop/rails/dynamic_find_by_spec.rb | 44 +++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7cbd38e389..3e98986c55 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@
 * [#451](https://github.com/rubocop/rubocop-rails/issues/451): Fix a false negative for `Rails/RelativeDateConstant` when a method is chained after a relative date method. ([@koic][])
 * [#450](https://github.com/rubocop/rubocop-rails/issues/450): Fix a crash for `Rails/ContentTag` with nested content tags. ([@tejasbubane][])
 * [#103](https://github.com/rubocop/rubocop-rails/issues/103): Fix a false positive for `Rails/FindEach` when not inheriting `ActiveRecord::Base` and using `all.each`. ([@koic][])
+* [#466](https://github.com/rubocop/rubocop-rails/pull/466): Fix a false positive for `Rails/DynamicFindBy` when not inheriting `ApplicationRecord` and without no receiver. ([@koic][])
 
 ### Changes
 
diff --git a/lib/rubocop/cop/rails/dynamic_find_by.rb b/lib/rubocop/cop/rails/dynamic_find_by.rb
index 089baf254c..87a06c4ad6 100644
--- a/lib/rubocop/cop/rails/dynamic_find_by.rb
+++ b/lib/rubocop/cop/rails/dynamic_find_by.rb
@@ -32,13 +32,14 @@ module Rails
       #   # good
       #   Gem::Specification.find_by_name('backend').gem_dir
       class DynamicFindBy < Base
+        include ActiveRecordHelper
         extend AutoCorrector
 
         MSG = 'Use `%<static_name>s` instead of dynamic `%<method>s`.'
         METHOD_PATTERN = /^find_by_(.+?)(!)?$/.freeze
 
         def on_send(node)
-          return if allowed_invocation?(node)
+          return if node.receiver.nil? && !inherit_active_record_base?(node) || allowed_invocation?(node)
 
           method_name = node.method_name
           static_name = static_method_name(method_name)
diff --git a/spec/rubocop/cop/rails/dynamic_find_by_spec.rb b/spec/rubocop/cop/rails/dynamic_find_by_spec.rb
index b103e64c45..a70a5909e3 100644
--- a/spec/rubocop/cop/rails/dynamic_find_by_spec.rb
+++ b/spec/rubocop/cop/rails/dynamic_find_by_spec.rb
@@ -144,6 +144,50 @@
     RUBY
   end
 
+  context 'with no receiver' do
+    it 'does not register an offense when not inheriting any class' do
+      expect_no_offenses(<<~RUBY)
+        class C
+          def do_something
+            find_by_name(name)
+          end
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when not inheriting `ApplicationRecord`' do
+      expect_no_offenses(<<~RUBY)
+        class C < Foo
+          def do_something
+            find_by_name(name)
+          end
+        end
+      RUBY
+    end
+
+    it 'registers an offense when inheriting `ApplicationRecord`' do
+      expect_offense(<<~RUBY)
+        class C < ApplicationRecord
+          def do_something
+            find_by_name(name)
+            ^^^^^^^^^^^^^^^^^^ Use `find_by` instead of dynamic `find_by_name`.
+          end
+        end
+      RUBY
+    end
+
+    it 'registers an offense when inheriting `ActiveRecord::Base`' do
+      expect_offense(<<~RUBY)
+        class C < ActiveRecord::Base
+          def do_something
+            find_by_name(name)
+            ^^^^^^^^^^^^^^^^^^ Use `find_by` instead of dynamic `find_by_name`.
+          end
+        end
+      RUBY
+    end
+  end
+
   context 'with allowed receiver name' do
     let(:cop_config) do
       { 'AllowedReceivers' => %w[Gem::Specification] }