Skip to content

Commit 1fc8b1d

Browse files
committed
[Fix rubocop#808] Flash is accepted only if followed by redirect_to
1 parent f361219 commit 1fc8b1d

File tree

3 files changed

+42
-7
lines changed

3 files changed

+42
-7
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ class ActionControllerFlashBeforeRender < Base
3737
^(send (send nil? :flash) :[]= ...)
3838
PATTERN
3939

40-
def_node_search :render?, <<~PATTERN
41-
(send nil? :render ...)
40+
def_node_search :redirect_to?, <<~PATTERN
41+
(send nil? :redirect_to ...)
4242
PATTERN
4343

4444
def_node_search :action_controller?, <<~PATTERN
@@ -53,7 +53,7 @@ class ActionControllerFlashBeforeRender < Base
5353
def on_send(flash_node)
5454
return unless flash_assignment?(flash_node)
5555

56-
return unless followed_by_render?(flash_node)
56+
return if followed_by_redirect_to?(flash_node)
5757

5858
return unless instance_method_or_block?(flash_node)
5959

@@ -66,12 +66,13 @@ def on_send(flash_node)
6666

6767
private
6868

69-
def followed_by_render?(flash_node)
69+
def followed_by_redirect_to?(flash_node)
7070
flash_assigment_node = find_ancestor(flash_node, type: :send)
7171
context = flash_assigment_node.parent
7272

73-
context.each_child_node.any? do |node|
74-
render?(node)
73+
flash_index = context.children.index(flash_assigment_node)
74+
context.each_child_node.with_index.any? do |node, index|
75+
index > flash_index && redirect_to?(node)
7576
end
7677
end
7778

spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
context 'within an instance method' do
66
%w[ActionController::Base ApplicationController].each do |parent_class|
77
context "within a class inherited from #{parent_class}" do
8-
it 'registers an offense and corrects' do
8+
it 'registers an offense and corrects when the render call is explicit' do
99
expect_offense(<<~RUBY)
1010
class HomeController < #{parent_class}
1111
def create
@@ -25,6 +25,25 @@ def create
2525
end
2626
RUBY
2727
end
28+
29+
it 'registers an offense and corrects when the render call is implicit' do
30+
expect_offense(<<~RUBY)
31+
class HomeController < #{parent_class}
32+
def create
33+
flash[:alert] = "msg"
34+
^^^^^ Use `flash.now` before `render`.
35+
end
36+
end
37+
RUBY
38+
39+
expect_correction(<<~RUBY)
40+
class HomeController < #{parent_class}
41+
def create
42+
flash.now[:alert] = "msg"
43+
end
44+
end
45+
RUBY
46+
end
2847
end
2948
end
3049

@@ -128,4 +147,18 @@ def create
128147
RUBY
129148
end
130149
end
150+
151+
context 'when using `flash` after `render` but before a `redirect_to`' do
152+
it 'does not register an offense' do
153+
expect_no_offenses(<<~RUBY)
154+
class HomeController < ApplicationController
155+
def create
156+
render :index and return if foo?
157+
flash[:alert] = "msg"
158+
redirect_to "https://www.example.com/"
159+
end
160+
end
161+
RUBY
162+
end
163+
end
131164
end

0 commit comments

Comments
 (0)