Skip to content

Commit

Permalink
[Fix rubocop#808] Flash is accepted only if followed by redirect_to
Browse files Browse the repository at this point in the history
  • Loading branch information
americodls committed Oct 12, 2022
1 parent f361219 commit 61778d1
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 8 deletions.
15 changes: 8 additions & 7 deletions lib/rubocop/cop/rails/action_controller_flash_before_render.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module RuboCop
module Cop
module Rails
# Using `flash` assignment before `render` in Rails controllers will persist the message for too long.
# Using `flash` without a following `redirect_to` in Rails controllers will persist the message for too long.
# Check https://guides.rubyonrails.org/action_controller_overview.html#flash-now
#
# @safety
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -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.each_with_index.any? do |node, index|
index > flash_index && redirect_to?(node)
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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

0 comments on commit 61778d1

Please sign in to comment.