diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-06-18 10:31:41 +0200 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-06-20 18:13:13 +0200 |
commit | f3f1df1476ba7fe223e5d8d6707a7675dc9fa597 (patch) | |
tree | b821f022ed4fd4201a3b86888f0d4a65b60d0411 /rubocop | |
parent | 03b88937757238c26a0bd2f3446dde03e7041b36 (diff) | |
download | gitlab-ce-f3f1df1476ba7fe223e5d8d6707a7675dc9fa597.tar.gz |
Add a cop for `FinderMethods`bvl-finder-methods-cop
This notifies developers when calling `find(_by!)` chained on
`execute`. And suggests using the methods from `FinderMethods`. These
will perform the correct authorization checks on the resource when it
is found.
Diffstat (limited to 'rubocop')
-rw-r--r-- | rubocop/cop/gitlab/finder_with_find_by.rb | 52 | ||||
-rw-r--r-- | rubocop/rubocop.rb | 1 |
2 files changed, 53 insertions, 0 deletions
diff --git a/rubocop/cop/gitlab/finder_with_find_by.rb b/rubocop/cop/gitlab/finder_with_find_by.rb new file mode 100644 index 00000000000..f45a37ddc06 --- /dev/null +++ b/rubocop/cop/gitlab/finder_with_find_by.rb @@ -0,0 +1,52 @@ +module RuboCop + module Cop + module Gitlab + class FinderWithFindBy < RuboCop::Cop::Cop + FIND_PATTERN = /\Afind(_by\!?)?\z/ + ALLOWED_MODULES = ['FinderMethods'].freeze + + def message(used_method) + <<~MSG + Don't chain finders `#execute` method with `##{used_method}`. + Instead include `FinderMethods` in the Finder and call `##{used_method}` + directly on the finder instance. + + This will make sure all authorization checks are performed on the resource. + MSG + end + + def on_send(node) + if find_on_execute?(node) && !allowed_module?(node) + add_offense(node, location: :selector, message: message(node.method_name)) + end + end + + def autocorrect(node) + lambda do |corrector| + upto_including_execute = node.descendants.first.source_range + before_execute = node.descendants[1].source_range + range_to_remove = node.source_range + .with(begin_pos: before_execute.end_pos, + end_pos: upto_including_execute.end_pos) + + corrector.remove(range_to_remove) + end + end + + def find_on_execute?(node) + chained_on_node = node.descendants.first + node.method_name.to_s =~ FIND_PATTERN && + chained_on_node&.method_name == :execute + end + + def allowed_module?(node) + ALLOWED_MODULES.include?(node.parent_module_name) + end + + def method_name_for_node(node) + children[1].to_s + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index f05990232ab..aa7ae601f75 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -2,6 +2,7 @@ require_relative 'cop/gitlab/module_with_instance_variables' require_relative 'cop/gitlab/predicate_memoization' require_relative 'cop/gitlab/httparty' +require_relative 'cop/gitlab/finder_with_find_by' require_relative 'cop/include_sidekiq_worker' require_relative 'cop/avoid_return_from_blocks' require_relative 'cop/avoid_break_from_strong_memoize' |