Merge branch 'dz-rubocop-group-routes' into 'master'

Add rubocop rule to group routing

See merge request gitlab-org/gitlab!21114

config/routes/group.rb

 # 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',

rubocop/cop/put_group_routes_under_scope.rb

+# 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

rubocop/rubocop.rb

@@ -15,6 +15,7 @@ require_relative 'cop/avoid_route_redirect_leading_slash'
 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'

spec/rubocop/cop/put_group_routes_under_scope_spec.rb

+# 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