diff --git a/config/routes/group.rb b/config/routes/group.rb index 437c80b8c9278cabf91291b393285facd7d39adb..30671d4e0a1affc9ad3eeb60e7f9f22cc090e255 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -1,8 +1,10 @@ # frozen_string_literal: true +# rubocop: disable Cop/PutGroupRoutesUnderScope resources :groups, only: [:index, :new, :create] do post :preview_markdown end +# rubocop: enable Cop/PutGroupRoutesUnderScope constraints(::Constraints::GroupUrlConstrainer.new) do scope(path: 'groups/*id', diff --git a/rubocop/cop/put_group_routes_under_scope.rb b/rubocop/cop/put_group_routes_under_scope.rb new file mode 100644 index 0000000000000000000000000000000000000000..bcdde01fdb526d3bd21154f8df8d4cb76ce25827 --- /dev/null +++ b/rubocop/cop/put_group_routes_under_scope.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Checks for a group routes outside '/-/' scope. + # For more information see: https://gitlab.com/gitlab-org/gitlab/issues/29572 + class PutGroupRoutesUnderScope < RuboCop::Cop::Cop + MSG = 'Put new group routes under /-/ scope' + + def_node_matcher :dash_scope?, <<~PATTERN + (:send nil? :scope (hash <(pair (sym :path)(str "groups/*group_id/-")) ...>)) + PATTERN + + def on_send(node) + return unless in_group_routes?(node) + return unless resource?(node) + return unless outside_scope?(node) + + add_offense(node) + end + + def outside_scope?(node) + node.each_ancestor(:block).none? do |parent| + dash_scope?(parent.to_a.first) + end + end + + def in_group_routes?(node) + path = node.location.expression.source_buffer.name + dirname = File.dirname(path) + filename = File.basename(path) + + dirname.end_with?('config/routes') && + filename.end_with?('group.rb') + end + + def resource?(node) + node.method_name == :resource || + node.method_name == :resources + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 9c9948e2a61c75b9e0a5d4c50ec9902e96e7d21b..1465c73d570252b4f8ef37bb4ecbc259768e63b4 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -15,6 +15,7 @@ require_relative 'cop/line_break_around_conditional_block' require_relative 'cop/prefer_class_methods_over_module' require_relative 'cop/put_project_routes_under_scope' +require_relative 'cop/put_group_routes_under_scope' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' diff --git a/spec/rubocop/cop/put_group_routes_under_scope_spec.rb b/spec/rubocop/cop/put_group_routes_under_scope_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..fc4d0015dde4462e88e8a80252cfaa28076b8178 --- /dev/null +++ b/spec/rubocop/cop/put_group_routes_under_scope_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rubocop' +require_relative '../../../rubocop/cop/put_group_routes_under_scope' + +describe RuboCop::Cop::PutGroupRoutesUnderScope do + include CopHelper + + subject(:cop) { described_class.new } + + before do + allow(cop).to receive(:in_group_routes?).and_return(true) + end + + it 'registers an offense when route is outside scope' do + expect_offense(<<~PATTERN.strip_indent) + scope(path: 'groups/*group_id/-', module: :groups) do + resource :issues + end + + resource :notes + ^^^^^^^^^^^^^^^ Put new group routes under /-/ scope + PATTERN + end + + it 'does not register an offense when resource inside the scope' do + expect_no_offenses(<<~PATTERN.strip_indent) + scope(path: 'groups/*group_id/-', module: :groups) do + resource :issues + resource :notes + end + PATTERN + end + + it 'does not register an offense when resource is deep inside the scope' do + expect_no_offenses(<<~PATTERN.strip_indent) + scope(path: 'groups/*group_id/-', module: :groups) do + resource :issues + resource :projects do + resource :issues do + resource :notes + end + end + end + PATTERN + end +end