From ca8189787957e1640b6a7500c35dbfd9b1962ffd Mon Sep 17 00:00:00 2001 From: r7kamura Date: Wed, 14 Aug 2024 10:36:26 +0900 Subject: [PATCH] Add `Rails/HashKeyValueRoute` cop --- .../new_add_rails_hash_key_value_route_cop.md | 1 + config/default.yml | 9 +++ lib/rubocop/cop/rails/hash_key_value_route.rb | 65 +++++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/hash_key_value_route_spec.rb | 50 ++++++++++++++ 5 files changed, 126 insertions(+) create mode 100644 changelog/new_add_rails_hash_key_value_route_cop.md create mode 100644 lib/rubocop/cop/rails/hash_key_value_route.rb create mode 100644 spec/rubocop/cop/rails/hash_key_value_route_spec.rb diff --git a/changelog/new_add_rails_hash_key_value_route_cop.md b/changelog/new_add_rails_hash_key_value_route_cop.md new file mode 100644 index 0000000000..0deee441f3 --- /dev/null +++ b/changelog/new_add_rails_hash_key_value_route_cop.md @@ -0,0 +1 @@ +* [#1329](https://github.com/rubocop/rubocop-rails/pull/1329): Add `Rails/HashKeyValueRoute` cop. ([@r7kamura][]) diff --git a/config/default.yml b/config/default.yml index 561b36484a..cce5b4b28a 100644 --- a/config/default.yml +++ b/config/default.yml @@ -540,6 +540,15 @@ Rails/HasManyOrHasOneDependent: Include: - app/models/**/*.rb +Rails/HashKeyValueRoute: + Description: 'Avoid Hash key-value routing.' + Enabled: pending + Safe: false + VersionAdded: '<>' + Include: + - config/routes.rb + - config/routes/**/*.rb + Rails/HelperInstanceVariable: Description: 'Do not use instance variables in helpers.' Enabled: true diff --git a/lib/rubocop/cop/rails/hash_key_value_route.rb b/lib/rubocop/cop/rails/hash_key_value_route.rb new file mode 100644 index 0000000000..a47ac2bb21 --- /dev/null +++ b/lib/rubocop/cop/rails/hash_key_value_route.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Avoid Hash key-value routing. + # Drawing a route in this style will be removed in Rails 8.1. + # + # @safety + # This cop is unsafe because it is implemented on the assumption that + # the first element of Hash always contains path and endpoint, based on + # the background that it is often so written. + # + # @example + # # bad + # get '/users' => 'users#index' + # + # # good + # get '/users', to: 'users#index' + # + # # bad + # mount MyApp => '/my_app' + # + # # good + # mount MyApp, at: '/my_app' + class HashKeyValueRoute < Base + extend AutoCorrector + + MSG = 'Avoid Hash key-value routing.' + + RESTRICT_ON_SEND = %i[delete get match mount options patch post put].freeze + + # @!method hash_key_value_route(node) + def_node_matcher :hash_key_value_route, <<~PATTERN + (send + nil? + _ + (hash + $(pair $_ $_) + ... + ) + ) + PATTERN + + def on_send(node) + hash_key_value_route(node) do |pair, key, value| + add_offense(pair) do |corrector| + corrector.replace(pair, "#{key.source}, #{option_name(node)}: #{value.source}") + end + end + end + + private + + def option_name(node) + if node.method?(:mount) + 'at' + else + 'to' + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index e5173baeb0..2e126bd35c 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -59,6 +59,7 @@ require_relative 'rails/freeze_time' require_relative 'rails/has_and_belongs_to_many' require_relative 'rails/has_many_or_has_one_dependent' +require_relative 'rails/hash_key_value_route' require_relative 'rails/helper_instance_variable' require_relative 'rails/http_positional_arguments' require_relative 'rails/http_status' diff --git a/spec/rubocop/cop/rails/hash_key_value_route_spec.rb b/spec/rubocop/cop/rails/hash_key_value_route_spec.rb new file mode 100644 index 0000000000..38744190ac --- /dev/null +++ b/spec/rubocop/cop/rails/hash_key_value_route_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::HashKeyValueRoute, :config do + context 'when using hash key-value route' do + it 'registers an offense' do + expect_offense(<<~RUBY) + get '/foos' => 'foos#index' + ^^^^^^^^^^^^^^^^^^^^^^^ Avoid Hash key-value routing. + RUBY + + expect_correction(<<~RUBY) + get '/foos', to: 'foos#index' + RUBY + end + end + + context 'with other options' do + it 'registers an offense' do + expect_offense(<<~RUBY) + get '/foos' => 'foos#index', as: 'foos' + ^^^^^^^^^^^^^^^^^^^^^^^ Avoid Hash key-value routing. + RUBY + + expect_correction(<<~RUBY) + get '/foos', to: 'foos#index', as: 'foos' + RUBY + end + end + + context 'with `mount`' do + it 'registers an offense' do + expect_offense(<<~RUBY) + mount MyApp => '/my_app' + ^^^^^^^^^^^^^^^^^^ Avoid Hash key-value routing. + RUBY + + expect_correction(<<~RUBY) + mount MyApp, at: '/my_app' + RUBY + end + end + + context 'without hash key-value route' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + get '/foos', to: 'foos#index' + RUBY + end + end +end