Skip to content

Commit 210f42e

Browse files
koickoic
authored
Merge pull request #1344 from Earlopain/revert-1311
Revert #1311 - false negatives for `Rails/ActionControllerFlashBeforeRender`
2 parents 5e66785 + 4c97338 commit 210f42e

File tree

3 files changed

+74
-183
lines changed

3 files changed

+74
-183
lines changed

‎changelog/fix_revert_flash_before_render.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1269](https://github.com/rubocop/rubocop-rails/issues/1269): Fix false positives for `Rails/ActionControllerFlashBeforeRender` in combination with implicit returns. ([@earlopain][])

‎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.rescue_type? ? node.parent : node
75+
context = node
76+
elsif context.right_siblings.empty?
77+
return true
7678
end
79+
context = context.right_siblings
7780

78-
siblings = context.right_siblings
79-
return true if siblings.empty?
80-
81-
siblings.compact.any? do |render_candidate|
81+
context.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: 68 additions & 178 deletions
Original file line numberDiff line numberDiff line change
@@ -128,36 +128,9 @@ 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
148131
end
149132

150133
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-
161134
expect_no_offenses(<<~RUBY)
162135
class HomeController < #{parent_class}
163136
def create
@@ -172,16 +145,6 @@ def create
172145
end
173146

174147
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-
185148
expect_no_offenses(<<~RUBY)
186149
class HomeController < #{parent_class}
187150
def create
@@ -195,7 +158,7 @@ def create
195158
RUBY
196159
end
197160

198-
it 'registers an offense when using `flash` in multiline `if` branch before `render`' do
161+
it 'registers an offense when using `flash` in multiline `if` branch before `render_to`' do
199162
expect_offense(<<~RUBY)
200163
class HomeController < #{parent_class}
201164
def create
@@ -222,29 +185,6 @@ def create
222185
end
223186
end
224187
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
248188
end
249189

250190
it 'does not register an offense when using `flash` in multiline `if` branch before `redirect_to`' do
@@ -277,81 +217,6 @@ def create
277217
end
278218
end
279219
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
355220
end
356221

357222
it 'does not register an offense when using `flash` in multiline `rescue` branch before `redirect_to`' do
@@ -370,48 +235,6 @@ def create
370235
end
371236
RUBY
372237
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
415238
end
416239
end
417240

@@ -505,4 +328,71 @@ def create
505328
RUBY
506329
end
507330
end
331+
332+
context 'when using `flash` after `render` and `redirect_to` is used in implicit return branch ' \
333+
'and render is is used in the other branch' do
334+
it 'does not register an offense' do
335+
expect_no_offenses(<<~RUBY)
336+
class HomeController < ApplicationController
337+
def create
338+
if foo.update(params)
339+
flash[:success] = 'msg'
340+
341+
if redirect_to_index?
342+
redirect_to index
343+
else
344+
redirect_to path(foo)
345+
end
346+
else
347+
flash.now[:alert] = 'msg'
348+
render :edit, status: :unprocessable_entity
349+
end
350+
end
351+
end
352+
RUBY
353+
end
354+
end
355+
356+
context 'when using `flash` after `render` and `render` is part of a different preceding branch' \
357+
'that implicitly returns' do
358+
it 'does not register an offense' do
359+
expect_no_offenses(<<~RUBY)
360+
class HomeController < ApplicationController
361+
def create
362+
if remote_request? || sandbox?
363+
if current_user.nil?
364+
render :index
365+
else
366+
head :forbidden
367+
end
368+
elsif current_user.nil?
369+
redirect_to sign_in_path
370+
else
371+
flash[:alert] = 'msg'
372+
if request.referer.present?
373+
redirect_to(request.referer)
374+
else
375+
redirect_to(root_path)
376+
end
377+
end
378+
end
379+
end
380+
RUBY
381+
end
382+
end
383+
384+
context 'when using `flash` in `rescue` and `redirect_to` in `ensure`' do
385+
it 'does not register an offense' do
386+
expect_no_offenses(<<~RUBY)
387+
class HomeController < ApplicationController
388+
def create
389+
rescue
390+
flash[:alert] = 'msg'
391+
ensure
392+
redirect_to :index
393+
end
394+
end
395+
RUBY
396+
end
397+
end
508398
end

0 commit comments

Comments
 (0)