Fix false negatives for implicit render or rescue blocks

rubocop · Jul 10, 2024 · f8c4d50 · f8c4d50
1 parent 3e91c03
commit f8c4d50
Show file tree

Hide file tree

Showing 3 changed files with 184 additions and 6 deletions.
diff --git a/changelog/fix_false_negatives_for_action_controller_flash_before_render.md b/changelog/fix_false_negatives_for_action_controller_flash_before_render.md
@@ -0,0 +1 @@
+* [#1311](https://github.com/rubocop/rubocop-rails/pull/1311): Fix false negatives for `Rails/ActionControllerFlashBeforeRender` when using implicit render or rescue blocks. ([@tldn0718][])
diff --git a/lib/rubocop/cop/rails/action_controller_flash_before_render.rb b/lib/rubocop/cop/rails/action_controller_flash_before_render.rb
@@ -72,13 +72,13 @@ def followed_by_render?(flash_node)
           if (node = context.each_ancestor(:if, :rescue).first)
             return false if use_redirect_to?(context)
 
-            context = node
-          elsif context.right_siblings.empty?
-            return true
+            context = node.rescue_type? ? node.parent : node
           end
-          context = context.right_siblings
 
-          context.compact.any? do |render_candidate|
+          siblings = context.right_siblings
+          return true if siblings.empty?
+
+          siblings.compact.any? do |render_candidate|
             render?(render_candidate)
           end
         end

diff --git a/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb b/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb
@@ -128,9 +128,36 @@ def create
                 end
               end
             RUBY
+
+            expect_offense(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  flash[:alert] = "msg" if condition
+                  ^^^^^ Use `flash.now` before `render`.
+                end
+              end
+            RUBY
+
+            expect_correction(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  flash.now[:alert] = "msg" if condition
+                end
+              end
+            RUBY
           end
 
           it 'does not register an offense when using `flash` before `redirect_to`' do
+            expect_no_offenses(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  flash[:alert] = "msg" if condition
+
+                  redirect_to :index
+                end
+              end
+            RUBY
+
             expect_no_offenses(<<~RUBY)
               class HomeController < #{parent_class}
                 def create
@@ -145,6 +172,16 @@ def create
           end
 
           it 'does not register an offense when using `flash` before `redirect_back`' do
+            expect_no_offenses(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  flash[:alert] = "msg" if condition
+
+                  redirect_back fallback_location: root_path
+                end
+              end
+            RUBY
+
             expect_no_offenses(<<~RUBY)
               class HomeController < #{parent_class}
                 def create
@@ -158,7 +195,7 @@ def create
             RUBY
           end
 
-          it 'registers an offense when using `flash` in multiline `if` branch before `render_to`' do
+          it 'registers an offense when using `flash` in multiline `if` branch before `render`' do
             expect_offense(<<~RUBY)
               class HomeController < #{parent_class}
                 def create
@@ -185,6 +222,29 @@ def create
                 end
               end
             RUBY
+
+            expect_offense(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  if condition
+                    do_something
+                    flash[:alert] = "msg"
+                    ^^^^^ Use `flash.now` before `render`.
+                  end
+                end
+              end
+            RUBY
+
+            expect_correction(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  if condition
+                    do_something
+                    flash.now[:alert] = "msg"
+                  end
+                end
+              end
+            RUBY
           end
 
           it 'does not register an offense when using `flash` in multiline `if` branch before `redirect_to`' do
@@ -217,6 +277,81 @@ def create
                 end
               end
             RUBY
+
+            expect_no_offenses(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  if condition
+                    flash[:alert] = "msg"
+                    redirect_to :index
+
+                    return
+                  end
+                end
+              end
+            RUBY
+          end
+
+          it 'registers an offense when using `flash` in multiline `rescue` branch before `render`' do
+            expect_offense(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  begin
+                    do_something
+                    flash[:alert] = "msg in begin"
+                    ^^^^^ Use `flash.now` before `render`.
+                  rescue
+                    flash[:alert] = "msg in rescue"
+                    ^^^^^ Use `flash.now` before `render`.
+                  end
+
+                  render :index
+                end
+              end
+            RUBY
+
+            expect_correction(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  begin
+                    do_something
+                    flash.now[:alert] = "msg in begin"
+                  rescue
+                    flash.now[:alert] = "msg in rescue"
+                  end
+
+                  render :index
+                end
+              end
+            RUBY
+
+            expect_offense(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  begin
+                    do_something
+                    flash[:alert] = "msg in begin"
+                    ^^^^^ Use `flash.now` before `render`.
+                  rescue
+                    flash[:alert] = "msg in rescue"
+                    ^^^^^ Use `flash.now` before `render`.
+                  end
+                end
+              end
+            RUBY
+
+            expect_correction(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  begin
+                    do_something
+                    flash.now[:alert] = "msg in begin"
+                  rescue
+                    flash.now[:alert] = "msg in rescue"
+                  end
+                end
+              end
+            RUBY
           end
 
           it 'does not register an offense when using `flash` in multiline `rescue` branch before `redirect_to`' do
@@ -235,6 +370,48 @@ def create
               end
             RUBY
           end
+
+          it 'does not register an offense when using `flash` before `redirect_to` in `rescue` branch' do
+            expect_no_offenses(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  begin
+                    do_something
+                    flash[:alert] = "msg in begin"
+                    redirect_to :index
+
+                    return
+                  rescue
+                    flash[:alert] = "msg in rescue"
+                    redirect_to :index
+ 
+                    return
+                  end
+
+                  render :index
+                end
+              end
+            RUBY
+
+            expect_no_offenses(<<~RUBY)
+              class HomeController < #{parent_class}
+                def create
+                  begin
+                    do_something
+                    flash[:alert] = "msg in begin"
+                    redirect_to :index
+
+                    return
+                  rescue
+                    flash[:alert] = "msg in rescue"
+                    redirect_to :index
+ 
+                    return
+                  end
+                end
+              end
+            RUBY
+          end
         end
       end