summaryrefslogtreecommitdiff
path: root/rubocop
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2018-06-18 10:31:41 +0200
committerBob Van Landuyt <bob@vanlanduyt.co>2018-06-20 18:13:13 +0200
commitf3f1df1476ba7fe223e5d8d6707a7675dc9fa597 (patch)
treeb821f022ed4fd4201a3b86888f0d4a65b60d0411 /rubocop
parent03b88937757238c26a0bd2f3446dde03e7041b36 (diff)
downloadgitlab-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.rb52
-rw-r--r--rubocop/rubocop.rb1
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'