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 Jun 6, 2019
1 parent c4e05e5 commit 2ba56ea
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

* Extract Rails cops from rubocop-hq/rubocop repository. ([@koic][])
* [#19](https://github.com/rubocop-hq/rubocop-rails/issues/19): Add new `Rails/HelperInstanceVariable` cop. ([@andyw8][])
* [#51](https://github.com/rubocop-hq/rubocop-rails/issues/51): Add ability to whitelist receiver class names in `Rails/DynamicFindBy`. ([@tejasbubane][])

[@koic]: https://github.com/koic
[@andyw8]: https://github.com/andyw8
[@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 @@ -141,6 +141,7 @@ Rails/DynamicFindBy:
VersionAdded: '0.44'
Whitelist:
- find_by_sql
- Gem::Specification

Rails/EnumUniqueness:
Description: 'Avoid duplicate integers in hash-syntax `enum` declaration.'
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
Expand Up @@ -9,6 +9,7 @@ module Cop
end

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

require_relative 'rails/action_filter'
require_relative 'rails/active_record_aliases'
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 2ba56ea

Please sign in to comment.