Skip to content

Commit

Permalink
[Fix rubocop#51] Add ability to whitelist receiver class names
Browse files Browse the repository at this point in the history
In `Rails/DynamicFindBy`.

Fixes rubocop#51.
  • Loading branch information
tejasbubane committed Aug 13, 2019
1 parent 3f5dc44 commit 2b3d477
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 10 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### New features

* [#51](https://github.com/rubocop-hq/rubocop-rails/issues/51): Add ability to whitelist receiver class names in `Rails/DynamicFindBy`. ([@tejasbubane][])

## 2.3.0 (2019-08-13)

### New features
Expand Down Expand Up @@ -65,3 +69,4 @@
[@santib]: https://github.com/santib
[@prathamesh-sonpatki]: https://github.com/prathamesh-sonpatki
[@MaximeLaurenty]: https://github.com/MaximeLaurenty
[@tejasbubane]: https://github.com/tejasbubane
1 change: 1 addition & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ Rails/DynamicFindBy:
VersionAdded: '0.44'
Whitelist:
- find_by_sql
- Gem::Specification

Rails/EnumHash:
Description: 'Prefer hash syntax over array syntax when defining enums.'
Expand Down
28 changes: 28 additions & 0 deletions lib/rubocop/cop/mixin/whitelist.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true

module RuboCop
module Cop
# Common functionality for checking whitelisted methods & class names.
module Whitelist
def whitelisted?(node)
whitelisted_method?(node) || whitelisted_receiver?(node)
end

def whitelisted_method?(node)
whitelist.include?(node.method_name.to_s)
end

def whitelisted_receiver?(node)
return unless node.receiver

whitelist.include?(node.receiver.source.to_s)
end

private

def whitelist
cop_config['Whitelist']
end
end
end
end
14 changes: 5 additions & 9 deletions lib/rubocop/cop/rails/dynamic_find_by.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,21 @@ module Rails
# # good
# User.find_by!(email: email)
class DynamicFindBy < Cop
include Whitelist

MSG = 'Use `%<static_name>s` instead of dynamic `%<method>s`.'
METHOD_PATTERN = /^find_by_(.+?)(!)?$/.freeze

def on_send(node)
method_name = node.method_name.to_s

return if whitelist.include?(method_name)
return if whitelisted?(node)

method_name = node.method_name
static_name = static_method_name(method_name)

return unless static_name

add_offense(node,
message: format(MSG, static_name: static_name,
method: node.method_name))
method: method_name))
end
alias on_csend on_send

Expand Down Expand Up @@ -68,10 +68,6 @@ def autocorrect_argument_keywords(corrector, node, keywords)
end
end

def whitelist
cop_config['Whitelist']
end

def column_keywords(method)
keyword_string = method.to_s[METHOD_PATTERN, 1]
keyword_string.split('_and_').map { |keyword| "#{keyword}: " }
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require_relative 'mixin/target_rails_version'
require_relative 'mixin/whitelist'

require_relative 'rails/action_filter'
require_relative 'rails/active_record_aliases'
Expand Down
2 changes: 1 addition & 1 deletion manual/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ User.find_by!(email: email)

Name | Default value | Configurable values
--- | --- | ---
Whitelist | `find_by_sql` | Array
Whitelist | `find_by_sql`, `Gem::Specification` | Array

### References

Expand Down
12 changes: 12 additions & 0 deletions spec/rubocop/cop/rails/dynamic_find_by_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@
RUBY
end

context 'with whitelisted classname' do
let(:cop_config) do
{ 'Whitelist' => %w[find_by_sql Gem::Specification] }
end

it 'accepts class methods in whitelist' do
expect_no_offenses(<<~RUBY)
Gem::Specification.find_by_name("backend").gem_dir
RUBY
end
end

context 'when using safe navigation operator' do
context 'with dynamic find_by_*' do
let(:source) { 'user&.find_by_name(name)' }
Expand Down

0 comments on commit 2b3d477

Please sign in to comment.