Merge pull request #1541 from davidenglishmusic/redirect-back-or-to-cop

Add RedirectBackOrTo cop

Author: koic

changelog/new_redirect_back_or_to_cop.md

@@ -0,0 +1 @@
+* [#1541](https://github.com/rubocop/rubocop-rails/pull/1541): Add new `Rails/RedirectBackOrTo` cop to suggest using `redirect_back_or_to` instead of `redirect_back` with `fallback_location`. ([@davidenglishmusic][])

config/default.yml

@@ -880,6 +880,14 @@ Rails/ReadWriteAttribute:
   Include:
     - '**/app/models/**/*.rb'
 
+Rails/RedirectBackOrTo:
+  Description: >-
+                  Use `redirect_back_or_to` instead of `redirect_back` with
+                  `fallback_location` option.
+  Enabled: pending
+  Severity: warning
+  VersionAdded: '<<next>>'
+
 Rails/RedundantActiveRecordAllMethod:
   Description: Detect redundant `all` used as a receiver for Active Record query methods.
   StyleGuide: 'https://rails.rubystyle.guide/#redundant-all'

lib/rubocop/cop/rails/redirect_back_or_to.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Checks for uses of `redirect_back(fallback_location: ...)` and
+      # suggests using `redirect_back_or_to(...)` instead.
+      #
+      # `redirect_back(fallback_location: ...)` was soft deprecated in Rails 7.0 and
+      # `redirect_back_or_to` was introduced as a replacement.
+      #
+      # @example
+      #   # bad
+      #   redirect_back(fallback_location: root_path)
+      #
+      #   # good
+      #   redirect_back_or_to(root_path)
+      #
+      #   # bad
+      #   redirect_back(fallback_location: root_path, allow_other_host: false)
+      #
+      #   # good
+      #   redirect_back_or_to(root_path, allow_other_host: false)
+      #
+      class RedirectBackOrTo < Base
+        extend AutoCorrector
+        extend TargetRailsVersion
+
+        minimum_target_rails_version 7.0
+
+        MSG = 'Use `redirect_back_or_to` instead of `redirect_back` with `:fallback_location` keyword argument.'
+        RESTRICT_ON_SEND = %i[redirect_back].freeze
+
+        def_node_matcher :redirect_back_with_fallback_location, <<~PATTERN
+          (send nil? :redirect_back
+            (hash <$(pair (sym :fallback_location) $_) ...>)
+          )
+        PATTERN
+
+        def on_send(node)
+          redirect_back_with_fallback_location(node) do |fallback_pair, fallback_value|
+            add_offense(node.loc.selector) do |corrector|
+              correct_redirect_back(corrector, node, fallback_pair, fallback_value)
+            end
+          end
+        end
+
+        private
+
+        def correct_redirect_back(corrector, node, fallback_pair, fallback_value)
+          corrector.replace(node.loc.selector, 'redirect_back_or_to')
+
+          hash_arg = node.first_argument
+
+          if hash_arg.pairs.one?
+            corrector.replace(hash_arg, fallback_value.source)
+          else
+            remove_fallback_location_pair(corrector, hash_arg, fallback_pair)
+            first_pair = hash_arg.pairs.find { |pair| pair != fallback_pair }
+            corrector.insert_before(first_pair, "#{fallback_value.source}, ")
+          end
+        end
+
+        def remove_fallback_location_pair(corrector, hash_node, fallback_pair)
+          pairs = hash_node.pairs
+          index = pairs.index(fallback_pair)
+
+          if pairs.one?
+            corrector.remove(fallback_pair)
+          elsif index.zero?
+            remove_first_pair(corrector, fallback_pair, pairs[1])
+          else
+            remove_non_first_pair(corrector, fallback_pair, pairs[index - 1])
+          end
+        end
+
+        def remove_first_pair(corrector, fallback_pair, next_pair)
+          range = fallback_pair.source_range.join(next_pair.source_range.begin)
+          corrector.remove(range)
+        end
+
+        def remove_non_first_pair(corrector, fallback_pair, prev_pair)
+          range = prev_pair.source_range.end.join(fallback_pair.source_range.end)
+          corrector.remove(range)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -104,6 +104,7 @@
 require_relative 'rails/redundant_presence_validation_on_belongs_to'
 require_relative 'rails/redundant_receiver_in_with_options'
 require_relative 'rails/redundant_travel_back'
+require_relative 'rails/redirect_back_or_to'
 require_relative 'rails/reflection_class_name'
 require_relative 'rails/refute_methods'
 require_relative 'rails/relative_date_constant'

spec/rubocop/cop/rails/redirect_back_or_to_spec.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::RedirectBackOrTo, :config do
+  context 'Rails >= 7.0', :rails70 do
+    it 'registers an offense and corrects when using redirect_back with a fallback_location' do
+      expect_offense(<<~RUBY)
+        redirect_back(fallback_location: root_path)
+        ^^^^^^^^^^^^^ Use `redirect_back_or_to` instead of `redirect_back` with `:fallback_location` keyword argument.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        redirect_back_or_to(root_path)
+      RUBY
+    end
+
+    it 'registers an offense and corrects with additional options' do
+      expect_offense(<<~RUBY)
+        redirect_back(fallback_location: root_path, status: 303, allow_other_host: true)
+        ^^^^^^^^^^^^^ Use `redirect_back_or_to` instead of `redirect_back` with `:fallback_location` keyword argument.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        redirect_back_or_to(root_path, status: 303, allow_other_host: true)
+      RUBY
+    end
+
+    it 'registers no offense when using redirect_back_or_to' do
+      expect_no_offenses(<<~RUBY)
+        redirect_back_or_to(root_path)
+      RUBY
+    end
+
+    it 'registers no offense when using redirect_back without fallback_location' do
+      expect_no_offenses(<<~RUBY)
+        redirect_back(allow_other_host: false)
+      RUBY
+    end
+  end
+
+  context 'Rails <= 6.1', :rails61 do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        redirect_back(fallback_location: root_path)
+      RUBY
+    end
+  end
+end