From d8c93c3e7b750c2064685f099e80e0525826ba90 Mon Sep 17 00:00:00 2001 From: Hartley McGuire Date: Fri, 20 Jun 2025 16:03:42 -0400 Subject: [PATCH] Add ConstantInRoutes Cop In applications with large route files, its common to have a desire to extract regexes, constraints, etc. that are used multiple times to prevent duplication. If the value is never going to change, extracting the value to a constant would be a very normal thing to do in typical Ruby classes/modules. However, the `config/routes.rb` file is reloadable, so a constant defined there will causing a constant redefinition warning when/if the routes are reloaded (typically during development or test, especially if using Spring). This commit adds a ConstantInRoutes Cop to prevent constants from being defined in `config/routes.rb` files. --- changelog/new_add_constant_in_routes_cop.md | 1 + config/default.yml | 6 +++ lib/rubocop/cop/rails/constant_in_routes.rb | 43 +++++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/constant_in_routes_spec.rb | 41 ++++++++++++++++++ 5 files changed, 92 insertions(+) create mode 100644 changelog/new_add_constant_in_routes_cop.md create mode 100644 lib/rubocop/cop/rails/constant_in_routes.rb create mode 100644 spec/rubocop/cop/rails/constant_in_routes_spec.rb diff --git a/changelog/new_add_constant_in_routes_cop.md b/changelog/new_add_constant_in_routes_cop.md new file mode 100644 index 0000000000..be9f5f227d --- /dev/null +++ b/changelog/new_add_constant_in_routes_cop.md @@ -0,0 +1 @@ +* [#1492](https://github.com/rubocop/rubocop-rails/pull/1492) Add new `ConstantInRoutes` Cop. ([@skipkayhil][]) diff --git a/config/default.yml b/config/default.yml index 26a00ce501..f47d706a6c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -298,6 +298,12 @@ Rails/CompactBlank: Safe: false VersionAdded: '2.13' +Rails/ConstantInRoutes: + Description: 'Checks for constants defined in config/routes.rb files.' + Enabled: pending + Safe: false + VersionAdded: <> + Rails/ContentTag: Description: 'Use `tag.something` instead of `tag(:something)`.' Reference: diff --git a/lib/rubocop/cop/rails/constant_in_routes.rb b/lib/rubocop/cop/rails/constant_in_routes.rb new file mode 100644 index 0000000000..556446daad --- /dev/null +++ b/lib/rubocop/cop/rails/constant_in_routes.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Prevent defining constants in config/routes.rb files. + # + # Defining constants in a routes.rb file can lead to constant redefinition + # warnings because routes.rb is a reloadable file. + # + # @example + # # bad + # Rails.application.routes.draw do + # ADMIN_CONSTRAINT = ->(req) { req.subdomain == 'admin' } + # + # get "/users", constraints: ADMIN_CONSTRAINT + # end + # + # # good + # Rails.application.routes.draw do + # admin_constraint = ->(req) { req.subdomain == 'admin' } + # + # get "/users", constraints: admin_constraint + # end + # + class ConstantInRoutes < Base + MSG = 'Do not define constants in config/routes.rb. Use a local variable or move this constant somewhere else.' + + def on_casgn(node) + return unless in_routes_file? + + add_offense(node) + end + + private + + def in_routes_file? + processed_source.file_path&.end_with?('config/routes.rb') + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index d3b24ec9e9..fa117e5061 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -33,6 +33,7 @@ require_relative 'rails/blank' require_relative 'rails/bulk_change_table' require_relative 'rails/compact_blank' +require_relative 'rails/constant_in_routes' require_relative 'rails/content_tag' require_relative 'rails/create_table_with_timestamps' require_relative 'rails/dangerous_column_names' diff --git a/spec/rubocop/cop/rails/constant_in_routes_spec.rb b/spec/rubocop/cop/rails/constant_in_routes_spec.rb new file mode 100644 index 0000000000..169e670cfe --- /dev/null +++ b/spec/rubocop/cop/rails/constant_in_routes_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::ConstantInRoutes, :config do + context 'when in config/routes.rb file' do + let(:source) { 'config/routes.rb' } + + it 'registers an offense when defining constants in routes' do + expect_offense(<<~RUBY, source) + API_VERSION = 'v1' + ^^^^^^^^^^^^^^^^^^ Do not define constants in config/routes.rb. Use a local variable or move this constant somewhere else. + Rails.application.routes.draw do + BASE_PATH = '/api' + ^^^^^^^^^^^^^^^^^^ Do not define constants in config/routes.rb. Use a local variable or move this constant somewhere else. + get "\#{BASE_PATH}\#{API_VERSION}/users", to: 'users#index' + end + RUBY + end + + it 'registers an offense when defining nested constants' do + expect_offense(<<~RUBY, source) + Api::VERSION = 'v1' + ^^^^^^^^^^^^^^^^^^^ Do not define constants in config/routes.rb. Use a local variable or move this constant somewhere else. + Rails.application.routes.draw do + get "/users", to: 'users#index' + end + RUBY + end + + it 'registers an offense when defining constants in opened classes' do + expect_offense(<<~RUBY, source) + class Api + VERSION = 'v1' + ^^^^^^^^^^^^^^ Do not define constants in config/routes.rb. Use a local variable or move this constant somewhere else. + end + Rails.application.routes.draw do + get "/users", to: 'users#index' + end + RUBY + end + end +end