diff --git a/CHANGELOG.md b/CHANGELOG.md index c3a9f64498..4ed2d12aab 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 `%s` instead of dynamic `%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] }