Commit 0896d694 authored by Sanad Liaquat's avatar Sanad Liaquat 🌴 Committed by Douwe Maan

Fix leading slash in redirects and add cop

parent f9320089
---
title: Fix leading slash in redirects and add rubocop cop
merge_request: 21828
author: Sanad Liaquat
type: fixed
# frozen_string_literal: true
namespace :instance_statistics do
root to: redirect('/-/instance_statistics/conversational_development_index')
root to: redirect('-/instance_statistics/conversational_development_index')
resources :cohorts, only: :index
resources :conversational_development_index, only: :index
......
......@@ -2,7 +2,7 @@ scope(controller: :wikis) do
scope(path: 'wikis', as: :wikis) do
get :git_access
get :pages
get '/', to: redirect('/%{namespace_id}/%{project_id}/wikis/home')
get '/', to: redirect('%{namespace_id}/%{project_id}/wikis/home')
post '/', to: 'wikis#create'
end
......
# frozen_string_literal: true
module RuboCop
module Cop
# Checks for a leading '/' in route redirects
# For more information see: https://gitlab.com/gitlab-org/gitlab-ce/issues/50645
#
# @example
# # bad
# root to: redirect('/-/instance/statistics/conversational_development_index')
#
# # good
# root to: redirect('-/instance/statistics/conversational_development_index')
#
class AvoidRouteRedirectLeadingSlash < RuboCop::Cop::Cop
MSG = 'Do not use a leading "/" in route redirects'
def_node_matcher :leading_slash_in_redirect?, <<~PATTERN
(send nil? :redirect (str #has_leading_slash?))
PATTERN
def on_send(node)
return unless in_routes?(node)
return unless leading_slash_in_redirect?(node)
add_offense(node)
end
def has_leading_slash?(str)
str.start_with?("/")
end
def in_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?('routes.rb')
end
def autocorrect(node)
lambda do |corrector|
corrector.replace(node.loc.expression, remove_leading_slash(node))
end
end
def remove_leading_slash(node)
node.source.sub('/', '')
end
end
end
end
......@@ -7,6 +7,7 @@ require_relative 'cop/gitlab/union'
require_relative 'cop/include_sidekiq_worker'
require_relative 'cop/avoid_return_from_blocks'
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/migration/add_column'
......
# frozen_string_literal: true
require 'spec_helper'
require 'rubocop'
require_relative '../../../rubocop/cop/avoid_route_redirect_leading_slash'
describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do
include CopHelper
subject(:cop) { described_class.new }
before do
allow(cop).to receive(:in_routes?).and_return(true)
end
it 'registers an offense when redirect has a leading slash' do
expect_offense(<<~PATTERN.strip_indent)
root to: redirect("/-/route")
^^^^^^^^^^^^^^^^^^^^ Do not use a leading "/" in route redirects
PATTERN
end
it 'does not register an offense when redirect does not have a leading slash' do
expect_no_offenses(<<~PATTERN.strip_indent)
root to: redirect("-/route")
PATTERN
end
it 'autocorrect `/-/route` to `-/route`' do
expect(autocorrect_source('redirect("/-/route")')).to eq('redirect("-/route")')
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