Skip to content

Commit 0f703db

Browse files
koickoic
authored
Merge pull request #1311 from tldn0718/fix-false-negatives-for-action-controller-flash-before-render
Fix false negatives for Rails/ActionControllerFlashBeforeRender when using implicit render or rescue block
2 parents 0f63f00 + f8c4d50 commit 0f703db

File tree

3 files changed

+184
-6
lines changed

3 files changed

+184
-6
lines changed

‎changelog/fix_false_negatives_for_action_controller_flash_before_render.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1311](https://github.com/rubocop/rubocop-rails/pull/1311): Fix false negatives for `Rails/ActionControllerFlashBeforeRender` when using implicit render or rescue blocks. ([@tldn0718][])

‎lib/rubocop/cop/rails/action_controller_flash_before_render.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ def followed_by_render?(flash_node)
7272
if (node = context.each_ancestor(:if, :rescue).first)
7373
return false if use_redirect_to?(context)
7474

75-
context = node
76-
elsif context.right_siblings.empty?
77-
return true
75+
context = node.rescue_type? ? node.parent : node
7876
end
79-
context = context.right_siblings
8077

81-
context.compact.any? do |render_candidate|
78+
siblings = context.right_siblings
79+
return true if siblings.empty?
80+
81+
siblings.compact.any? do |render_candidate|
8282
render?(render_candidate)
8383
end
8484
end

‎spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb

Lines changed: 178 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,36 @@ def create
128128
end
129129
end
130130
RUBY
131+
132+
expect_offense(<<~RUBY)
133+
class HomeController < #{parent_class}
134+
def create
135+
flash[:alert] = "msg" if condition
136+
^^^^^ Use `flash.now` before `render`.
137+
end
138+
end
139+
RUBY
140+
141+
expect_correction(<<~RUBY)
142+
class HomeController < #{parent_class}
143+
def create
144+
flash.now[:alert] = "msg" if condition
145+
end
146+
end
147+
RUBY
131148
end
132149

133150
it 'does not register an offense when using `flash` before `redirect_to`' do
151+
expect_no_offenses(<<~RUBY)
152+
class HomeController < #{parent_class}
153+
def create
154+
flash[:alert] = "msg" if condition
155+
156+
redirect_to :index
157+
end
158+
end
159+
RUBY
160+
134161
expect_no_offenses(<<~RUBY)
135162
class HomeController < #{parent_class}
136163
def create
@@ -145,6 +172,16 @@ def create
145172
end
146173

147174
it 'does not register an offense when using `flash` before `redirect_back`' do
175+
expect_no_offenses(<<~RUBY)
176+
class HomeController < #{parent_class}
177+
def create
178+
flash[:alert] = "msg" if condition
179+
180+
redirect_back fallback_location: root_path
181+
end
182+
end
183+
RUBY
184+
148185
expect_no_offenses(<<~RUBY)
149186
class HomeController < #{parent_class}
150187
def create
@@ -158,7 +195,7 @@ def create
158195
RUBY
159196
end
160197

161-
it 'registers an offense when using `flash` in multiline `if` branch before `render_to`' do
198+
it 'registers an offense when using `flash` in multiline `if` branch before `render`' do
162199
expect_offense(<<~RUBY)
163200
class HomeController < #{parent_class}
164201
def create
@@ -185,6 +222,29 @@ def create
185222
end
186223
end
187224
RUBY
225+
226+
expect_offense(<<~RUBY)
227+
class HomeController < #{parent_class}
228+
def create
229+
if condition
230+
do_something
231+
flash[:alert] = "msg"
232+
^^^^^ Use `flash.now` before `render`.
233+
end
234+
end
235+
end
236+
RUBY
237+
238+
expect_correction(<<~RUBY)
239+
class HomeController < #{parent_class}
240+
def create
241+
if condition
242+
do_something
243+
flash.now[:alert] = "msg"
244+
end
245+
end
246+
end
247+
RUBY
188248
end
189249

190250
it 'does not register an offense when using `flash` in multiline `if` branch before `redirect_to`' do
@@ -217,6 +277,81 @@ def create
217277
end
218278
end
219279
RUBY
280+
281+
expect_no_offenses(<<~RUBY)
282+
class HomeController < #{parent_class}
283+
def create
284+
if condition
285+
flash[:alert] = "msg"
286+
redirect_to :index
287+
288+
return
289+
end
290+
end
291+
end
292+
RUBY
293+
end
294+
295+
it 'registers an offense when using `flash` in multiline `rescue` branch before `render`' do
296+
expect_offense(<<~RUBY)
297+
class HomeController < #{parent_class}
298+
def create
299+
begin
300+
do_something
301+
flash[:alert] = "msg in begin"
302+
^^^^^ Use `flash.now` before `render`.
303+
rescue
304+
flash[:alert] = "msg in rescue"
305+
^^^^^ Use `flash.now` before `render`.
306+
end
307+
308+
render :index
309+
end
310+
end
311+
RUBY
312+
313+
expect_correction(<<~RUBY)
314+
class HomeController < #{parent_class}
315+
def create
316+
begin
317+
do_something
318+
flash.now[:alert] = "msg in begin"
319+
rescue
320+
flash.now[:alert] = "msg in rescue"
321+
end
322+
323+
render :index
324+
end
325+
end
326+
RUBY
327+
328+
expect_offense(<<~RUBY)
329+
class HomeController < #{parent_class}
330+
def create
331+
begin
332+
do_something
333+
flash[:alert] = "msg in begin"
334+
^^^^^ Use `flash.now` before `render`.
335+
rescue
336+
flash[:alert] = "msg in rescue"
337+
^^^^^ Use `flash.now` before `render`.
338+
end
339+
end
340+
end
341+
RUBY
342+
343+
expect_correction(<<~RUBY)
344+
class HomeController < #{parent_class}
345+
def create
346+
begin
347+
do_something
348+
flash.now[:alert] = "msg in begin"
349+
rescue
350+
flash.now[:alert] = "msg in rescue"
351+
end
352+
end
353+
end
354+
RUBY
220355
end
221356

222357
it 'does not register an offense when using `flash` in multiline `rescue` branch before `redirect_to`' do
@@ -235,6 +370,48 @@ def create
235370
end
236371
RUBY
237372
end
373+
374+
it 'does not register an offense when using `flash` before `redirect_to` in `rescue` branch' do
375+
expect_no_offenses(<<~RUBY)
376+
class HomeController < #{parent_class}
377+
def create
378+
begin
379+
do_something
380+
flash[:alert] = "msg in begin"
381+
redirect_to :index
382+
383+
return
384+
rescue
385+
flash[:alert] = "msg in rescue"
386+
redirect_to :index
387+
388+
return
389+
end
390+
391+
render :index
392+
end
393+
end
394+
RUBY
395+
396+
expect_no_offenses(<<~RUBY)
397+
class HomeController < #{parent_class}
398+
def create
399+
begin
400+
do_something
401+
flash[:alert] = "msg in begin"
402+
redirect_to :index
403+
404+
return
405+
rescue
406+
flash[:alert] = "msg in rescue"
407+
redirect_to :index
408+
409+
return
410+
end
411+
end
412+
end
413+
RUBY
414+
end
238415
end
239416
end
240417

0 commit comments

Comments
 (0)