diff --git a/config/routes/project.rb b/config/routes/project.rb index 00d674575c4991b0bc4e34ec87bc1649ee310ca2..078fa15e40f1f09f4c2fea283fbacbf1f771eac9 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -1,10 +1,12 @@ # frozen_string_literal: true +# rubocop: disable Cop/PutProjectRoutesUnderScope resources :projects, only: [:index, :new, :create] draw :git_http get '/projects/:id' => 'projects#resolve' +# rubocop: enable Cop/PutProjectRoutesUnderScope constraints(::Constraints::ProjectUrlConstrainer.new) do # If the route has a wildcard segment, the segment has a regex constraint, @@ -210,6 +212,10 @@ end # End of the /-/ scope. + # All new routes should go under /-/ scope. + # Look for scope '-' at the top of the file. + # rubocop: disable Cop/PutProjectRoutesUnderScope + # # Templates # @@ -524,6 +530,10 @@ draw :wiki draw :repository + # All new routes should go under /-/ scope. + # Look for scope '-' at the top of the file. + # rubocop: enable Cop/PutProjectRoutesUnderScope + # Legacy routes. # Introduced in 12.0. # Should be removed with https://gitlab.com/gitlab-org/gitlab/issues/28848. @@ -535,6 +545,7 @@ :cycle_analytics, :mattermost, :variables, :triggers) end + # rubocop: disable Cop/PutProjectRoutesUnderScope resources(:projects, path: '/', constraints: { id: Gitlab::PathRegex.project_route_regex }, @@ -556,5 +567,6 @@ put :new_issuable_address end end + # rubocop: enable Cop/PutProjectRoutesUnderScope end end diff --git a/ee/config/routes/project.rb b/ee/config/routes/project.rb index 9d0cc6e8c3743653c593943e23271d57932ed0a0..8c84cfd64a0fb801a94f32b02fe1236790e643df 100644 --- a/ee/config/routes/project.rb +++ b/ee/config/routes/project.rb @@ -64,6 +64,10 @@ end # End of the /-/ scope. + # All new routes should go under /-/ scope. + # Look for scope '-' at the top of the file. + # rubocop: disable Cop/PutProjectRoutesUnderScope + resources :path_locks, only: [:index, :destroy] do collection do post :toggle @@ -206,6 +210,10 @@ end resources :audit_events, only: [:index] + + # All new routes should go under /-/ scope. + # Look for scope '-' at the top of the file. + # rubocop: enable Cop/PutProjectRoutesUnderScope end end end diff --git a/rubocop/cop/put_project_routes_under_scope.rb b/rubocop/cop/put_project_routes_under_scope.rb new file mode 100644 index 0000000000000000000000000000000000000000..02189f43ea03d0b8d5e1f00987ebe3afceb8082a --- /dev/null +++ b/rubocop/cop/put_project_routes_under_scope.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Checks for a project routes outside '/-/' scope. + # For more information see: https://gitlab.com/gitlab-org/gitlab/issues/29572 + class PutProjectRoutesUnderScope < RuboCop::Cop::Cop + MSG = 'Put new project routes under /-/ scope' + + def_node_matcher :dash_scope?, <<~PATTERN + (:send nil? :scope (:str "-")) + PATTERN + + def on_send(node) + return unless in_project_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_project_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?('project.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 159892ae0c1ea510f47c939337a9e532a65fab50..49d582bf034f9aeed79e00d004d2ad427c299b89 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -14,6 +14,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/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_project_routes_under_scope_spec.rb b/spec/rubocop/cop/put_project_routes_under_scope_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b0f1e52f3972d6b51199c6b4972c21778a1bae62 --- /dev/null +++ b/spec/rubocop/cop/put_project_routes_under_scope_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rubocop' +require_relative '../../../rubocop/cop/put_project_routes_under_scope' + +describe RuboCop::Cop::PutProjectRoutesUnderScope do + include CopHelper + + subject(:cop) { described_class.new } + + before do + allow(cop).to receive(:in_project_routes?).and_return(true) + end + + it 'registers an offense when route is outside scope' do + expect_offense(<<~PATTERN.strip_indent) + scope '-' do + resource :issues + end + + resource :notes + ^^^^^^^^^^^^^^^ Put new project routes under /-/ scope + PATTERN + end + + it 'does not register an offense when resource inside the scope' do + expect_no_offenses(<<~PATTERN.strip_indent) + scope '-' 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 '-' do + resource :issues + resource :projects do + resource :issues do + resource :notes + end + end + end + PATTERN + end +end