Merge pull request rubocop#812 from americodls/fix-808

[Fix rubocop#808] Flash is accepted only if followed by redirect_to

Author: koic

changelog/fix_false_positive_for_action_controller_flash_before_render.md

@@ -0,0 +1 @@
+* [#808](https://github.com/rubocop/rubocop-rails/issues/808): Fix false positive for `Rails/ActionControllerFlashBeforeRender` when `render` call precedes `flash` call. ([@americodls][])

lib/rubocop/cop/rails/action_controller_flash_before_render.rb

@@ -37,8 +37,8 @@ class ActionControllerFlashBeforeRender < Base
           ^(send (send nil? :flash) :[]= ...)
         PATTERN
 
-        def_node_search :render?, <<~PATTERN
-          (send nil? :render ...)
+        def_node_search :redirect_to?, <<~PATTERN
+          (send nil? :redirect_to ...)
         PATTERN
 
         def_node_search :action_controller?, <<~PATTERN
@@ -53,7 +53,7 @@ class ActionControllerFlashBeforeRender < Base
         def on_send(flash_node)
           return unless flash_assignment?(flash_node)
 
-          return unless followed_by_render?(flash_node)
+          return if followed_by_redirect_to?(flash_node)
 
           return unless instance_method_or_block?(flash_node)
 
@@ -66,12 +66,13 @@ def on_send(flash_node)
 
         private
 
-        def followed_by_render?(flash_node)
+        def followed_by_redirect_to?(flash_node)
           flash_assigment_node = find_ancestor(flash_node, type: :send)
           context = flash_assigment_node.parent
 
-          context.each_child_node.any? do |node|
-            render?(node)
+          flash_index = context.children.index(flash_assigment_node)
+          context.each_child_node.with_index.any? do |node, index|
+            index > flash_index && redirect_to?(node)
           end
         end
 

spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb

@@ -5,7 +5,7 @@
     context 'within an instance method' do
       %w[ActionController::Base ApplicationController].each do |parent_class|
         context "within a class inherited from #{parent_class}" do
-          it 'registers an offense and corrects' do
+          it 'registers an offense and corrects when the render call is explicit' do
             expect_offense(<<~RUBY)
               class HomeController < #{parent_class}
                 def create
@@ -25,6 +25,25 @@ def create
               end
             RUBY
           end
+
+          it 'registers an offense and corrects when the render call is implicit' do
+            expect_offense(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  flash[:alert] = "msg"
+                  ^^^^^ Use `flash.now` before `render`.
+                end
+              end
+            RUBY
+
+            expect_correction(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  flash.now[:alert] = "msg"
+                end
+              end
+            RUBY
+          end
         end
       end
 
@@ -128,4 +147,18 @@ def create
       RUBY
     end
   end
+
+  context 'when using `flash` after `render` but before a `redirect_to`' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        class HomeController < ApplicationController
+          def create
+            render :index and return if foo?
+            flash[:alert] = "msg"
+            redirect_to "https://www.example.com/"
+          end
+        end
+      RUBY
+    end
+  end
 end