summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--rubocop/cop/safe_params.rb34
-rw-r--r--rubocop/rubocop.rb1
-rw-r--r--spec/rubocop/cop/safe_params_spec.rb39
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