Skip to content

Commit

Permalink
[Fix rubocop#4967] Fix Rails/DynamicFindBy false-positives on non A…
Browse files Browse the repository at this point in the history
…ctiveRecord

Fixes rubocop#4967.
This change applies `Rails/DynamicFindBy` cop only to receiver classes
that inherit `ActiveRecord::Base` class.
  • Loading branch information
koic committed Nov 4, 2017
1 parent 09dcfc1 commit 0bde613
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* [#4987](https://github.com/bbatsov/rubocop/pull/4987): Skip permission check when using stdin option. ([@mtsmfm][])
* [#4909](https://github.com/bbatsov/rubocop/issues/4909): Make `Rails/HasManyOrHasOneDependent` aware of multiple associations in `with_options`. ([@koic][])
* [#4794](https://github.com/bbatsov/rubocop/issues/4794): Fix an error in `Layout/MultilineOperationIndentation` when an operation spans multiple lines and contains a ternary expression. ([@rrosenblum][])
* [#4967](https://github.com/bbatsov/rubocop/issues/4967): Fix `Rails/DynamicFindBy` false-positives on non ActiveRecord model. ([@koic][])

### Changes

Expand Down
10 changes: 10 additions & 0 deletions lib/rubocop/cop/rails/dynamic_find_by.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ class DynamicFindBy < Cop
METHOD_PATTERN = /^find_by_(.+?)(!)?$/

def on_send(node)
return if node.receiver.nil? ||
!inherited_active_record?(node.receiver.const_name)

method_name = node.method_name.to_s

return if whitelist.include?(method_name)
Expand Down Expand Up @@ -75,6 +78,13 @@ def static_method_name(method_name)
return nil unless match
match[2] ? 'find_by!' : 'find_by'
end

def inherited_active_record?(class_name)
klass = Object.const_get(class_name)

klass.respond_to?(:ancestors) &&
klass.ancestors.include?(ActiveRecord::Base)
end
end
end
end
Expand Down
23 changes: 23 additions & 0 deletions spec/rubocop/cop/rails/dynamic_find_by_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@
{ 'Whitelist' => %w[find_by_sql] }
end

before do
module ActiveRecord
class Base
end
end

class User < ActiveRecord::Base; end
end

after do
Object.send(:remove_const, 'User')
ActiveRecord.send(:remove_const, 'Base')
Object.send(:remove_const, 'ActiveRecord')
end

shared_examples 'register an offense and auto correct' do |message, corrected|
it 'registers an offense' do
inspect_source(source)
Expand Down Expand Up @@ -138,4 +153,12 @@
User.find_by_sql(["select * from users where name = ?", name])
RUBY
end

context 'When receiver class does not inherit `ActiveRecord::Base`' do
it 'accepts find_by_* method' do
expect_no_offenses(<<-RUBY.strip_indent)
Gem::Specification.find_by_name('loremipsum')
RUBY
end
end
end

0 comments on commit 0bde613

Please sign in to comment.