From c040369b44bf2d39629463712d56c186ec7c6852 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 3 Dec 2019 13:07:46 +0200
Subject: [PATCH] Add rubocop rule to group routing

Do not allow new resources appear outside of /-/ scope

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
 config/routes/group.rb                        |  2 +
 rubocop/cop/put_group_routes_under_scope.rb   | 43 +++++++++++++++++
 rubocop/rubocop.rb                            |  1 +
 .../cop/put_group_routes_under_scope_spec.rb  | 48 +++++++++++++++++++
 4 files changed, 94 insertions(+)
 create mode 100644 rubocop/cop/put_group_routes_under_scope.rb
 create mode 100644 spec/rubocop/cop/put_group_routes_under_scope_spec.rb

diff --git a/config/routes/group.rb b/config/routes/group.rb
index 437c80b8c9278c..30671d4e0a1aff 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 00000000000000..bcdde01fdb526d
--- /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 9c9948e2a61c75..1465c73d570252 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 00000000000000..fc4d0015dde446
--- /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
-- 
GitLab