diff options
-rw-r--r-- | rubocop/cop/safe_params.rb | 34 | ||||
-rw-r--r-- | rubocop/rubocop.rb | 1 | ||||
-rw-r--r-- | spec/rubocop/cop/safe_params_spec.rb | 39 |
3 files changed, 74 insertions, 0 deletions
diff --git a/rubocop/cop/safe_params.rb b/rubocop/cop/safe_params.rb new file mode 100644 index 00000000000..250c16232e4 --- /dev/null +++ b/rubocop/cop/safe_params.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + class SafeParams < RuboCop::Cop::Cop + MSG = 'Use `safe_params` instead of `params` in url_for.'.freeze + + METHOD_NAME_PATTERN = :url_for + UNSAFE_PARAM = :params + + def on_send(node) + return unless method_name(node) == METHOD_NAME_PATTERN + + add_offense(node, location: :expression) unless safe_params?(node) + end + + private + + def safe_params?(node) + node.descendants.each do |param_node| + next unless param_node.descendants.empty? + + return false if method_name(param_node) == UNSAFE_PARAM + end + + true + end + + def method_name(node) + node.children[1] + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 6c9b8753c1a..4489159f422 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -5,6 +5,7 @@ require_relative 'cop/gitlab/httparty' require_relative 'cop/gitlab/finder_with_find_by' require_relative 'cop/gitlab/union' require_relative 'cop/include_sidekiq_worker' +require_relative 'cop/safe_params' require_relative 'cop/avoid_return_from_blocks' require_relative 'cop/avoid_break_from_strong_memoize' require_relative 'cop/avoid_route_redirect_leading_slash' diff --git a/spec/rubocop/cop/safe_params_spec.rb b/spec/rubocop/cop/safe_params_spec.rb new file mode 100644 index 00000000000..4f02b8e9008 --- /dev/null +++ b/spec/rubocop/cop/safe_params_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../rubocop/cop/safe_params' + +describe RuboCop::Cop::SafeParams do + include CopHelper + + subject(:cop) { described_class.new } + + it 'flags the params as an argument of url_for' do + expect_offense(<<~SOURCE) + url_for(params) + ^^^^^^^^^^^^^^^ Use `safe_params` instead of `params` in url_for. + SOURCE + end + + it 'flags the merged params as an argument of url_for' do + expect_offense(<<~SOURCE) + url_for(params.merge(additional_params)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `safe_params` instead of `params` in url_for. + SOURCE + end + + it 'flags the merged params arg as an argument of url_for' do + expect_offense(<<~SOURCE) + url_for(something.merge(additional).merge(params)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `safe_params` instead of `params` in url_for. + SOURCE + end + + it 'does not flag other argument of url_for' do + expect_no_offenses(<<~SOURCE) + url_for(something) + SOURCE + end +end |