Commit 6899f06d authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets Committed by Heinrich Lee Yu

Add rubocop rule for project routing

Ensure that new routes are added to /-/ scope
Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 13863d6a
# 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 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
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
#
......@@ -522,6 +528,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
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.
......@@ -533,6 +543,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
:cycle_analytics, :mattermost, :variables, :triggers)
end
# rubocop: disable Cop/PutProjectRoutesUnderScope
resources(:projects,
path: '/',
constraints: { id: Gitlab::PathRegex.project_route_regex },
......@@ -554,5 +565,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
put :new_issuable_address
end
end
# rubocop: enable Cop/PutProjectRoutesUnderScope
end
end
......@@ -64,6 +64,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
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 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
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
......
# 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
......@@ -14,6 +14,7 @@ require_relative 'cop/avoid_break_from_strong_memoize'
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'
......
# 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
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