Commit cf09b573 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Improve route cop

By ignoring root routes like:

get "/"
post "/"
Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 207688e5
...@@ -23,9 +23,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do ...@@ -23,9 +23,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
get 'archived', action: :show, as: :group_archived # rubocop:disable Cop/PutGroupRoutesUnderScope get 'archived', action: :show, as: :group_archived # rubocop:disable Cop/PutGroupRoutesUnderScope
end end
# These routes are legit and the cop rule will be improved in get '/', action: :show, as: :group_canonical
# https://gitlab.com/gitlab-org/gitlab/-/issues/230703
get '/', action: :show, as: :group_canonical # rubocop:disable Cop/PutGroupRoutesUnderScope
end end
scope(path: 'groups/*group_id/-', scope(path: 'groups/*group_id/-',
...@@ -112,11 +110,9 @@ constraints(::Constraints::GroupUrlConstrainer.new) do ...@@ -112,11 +110,9 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
as: :group, as: :group,
constraints: { id: Gitlab::PathRegex.full_namespace_route_regex, format: /(html|json|atom)/ }, constraints: { id: Gitlab::PathRegex.full_namespace_route_regex, format: /(html|json|atom)/ },
controller: :groups) do controller: :groups) do
# These routes are legit and the cop rule will be improved in get '/', action: :show
# https://gitlab.com/gitlab-org/gitlab/-/issues/230703 patch '/', action: :update
get '/', action: :show # rubocop:disable Cop/PutGroupRoutesUnderScope put '/', action: :update
patch '/', action: :update # rubocop:disable Cop/PutGroupRoutesUnderScope delete '/', action: :destroy
put '/', action: :update # rubocop:disable Cop/PutGroupRoutesUnderScope
delete '/', action: :destroy # rubocop:disable Cop/PutGroupRoutesUnderScope
end end
end end
...@@ -12,6 +12,7 @@ module RuboCop ...@@ -12,6 +12,7 @@ module RuboCop
def on_send(node) def on_send(node)
return unless route_method?(node) return unless route_method?(node)
return unless outside_scope?(node) return unless outside_scope?(node)
return if root_route?(node)
add_offense(node) add_offense(node)
end end
...@@ -25,5 +26,13 @@ module RuboCop ...@@ -25,5 +26,13 @@ module RuboCop
def route_method?(node) def route_method?(node)
ROUTE_METHODS.include?(node.method_name) ROUTE_METHODS.include?(node.method_name)
end end
def root_route?(node)
first_argument = node.arguments.first
if first_argument.respond_to?(:value)
first_argument.value == '/'
end
end
end end
end end
...@@ -46,4 +46,18 @@ RSpec.describe RuboCop::Cop::PutGroupRoutesUnderScope, type: :rubocop do ...@@ -46,4 +46,18 @@ RSpec.describe RuboCop::Cop::PutGroupRoutesUnderScope, type: :rubocop do
end end
PATTERN PATTERN
end end
it 'does not register an offense for the root route' do
expect_no_offenses(<<~PATTERN)
get '/'
PATTERN
end
it 'does not register an offense for the root route within scope' do
expect_no_offenses(<<~PATTERN)
scope(path: 'groups/*group_id/-', module: :groups) do
get '/'
end
PATTERN
end
end end
...@@ -46,4 +46,18 @@ RSpec.describe RuboCop::Cop::PutProjectRoutesUnderScope, type: :rubocop do ...@@ -46,4 +46,18 @@ RSpec.describe RuboCop::Cop::PutProjectRoutesUnderScope, type: :rubocop do
end end
PATTERN PATTERN
end end
it 'does not register an offense for the root route' do
expect_no_offenses(<<~PATTERN)
get '/'
PATTERN
end
it 'does not register an offense for the root route within scope' do
expect_no_offenses(<<~PATTERN)
scope '-' do
get '/'
end
PATTERN
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment