From 9efc6a0e8268bfb79ebdd13dc425f849c823f184 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Tue, 27 Aug 2024 10:30:36 +0200 Subject: [PATCH] [Issue #1269] Add tests for `Rails/ActionControllerFlashBeforeRender` from issue #1269 This cop would need to do control flow analysis which it just doesn't do. RuboCop also has no mechanism for that. So just reverting this for now to fix the newly introduces false positives --- changelog/fix_revert_flash_before_render.md | 1 + ...ion_controller_flash_before_render_spec.rb | 67 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 changelog/fix_revert_flash_before_render.md diff --git a/changelog/fix_revert_flash_before_render.md b/changelog/fix_revert_flash_before_render.md new file mode 100644 index 0000000000..f2409cd09a --- /dev/null +++ b/changelog/fix_revert_flash_before_render.md @@ -0,0 +1 @@ +* [#1269](https://github.com/rubocop/rubocop-rails/issues/1269): Fix positives for `Rails/ActionControllerFlashBeforeRender` in combination with implicit returns. ([@earlopain][]) 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 index 4704d07a29..e3eb5ba1e8 100644 --- a/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb +++ b/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb @@ -328,4 +328,71 @@ def create RUBY end end + + context 'when using `flash` after `render` and `redirect_to` is used in implicit return branch ' \ + 'and render is is used in the other branch' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + class HomeController < ApplicationController + def create + if foo.update(params) + flash[:success] = 'msg' + + if redirect_to_index? + redirect_to index + else + redirect_to path(foo) + end + else + flash.now[:alert] = 'msg' + render :edit, status: :unprocessable_entity + end + end + end + RUBY + end + end + + context 'when using `flash` after `render` and `render` is part of a different preceeding branch' \ + 'that implicitly returns' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + class HomeController < ApplicationController + def create + if remote_request? || sandbox? + if current_user.nil? + render :index + else + head :forbidden + end + elsif current_user.nil? + redirect_to sign_in_path + else + flash[:alert] = 'msg' + if request.referer.present? + redirect_to(request.referer) + else + redirect_to(root_path) + end + end + end + end + RUBY + end + end + + context 'when using `flash` in `rescue` and `redirect_to` in `ensure`' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + class HomeController < ApplicationController + def create + rescue + flash[:alert] = 'msg' + ensure + redirect_to :index + end + end + RUBY + end + end end