diff --git a/.rubocop.yml b/.rubocop.yml index 8d8536c8267..eec10fd71e3 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,10 +1,10 @@ inherit_from: .rubocop_todo.yml require: + - ./test/action_controller_flash_before_render.rb - ./test/safe_navigation_cop.rb - rubocop-performance - rubocop-rails - rubocop-minitest - - ./lib/custom_cops/action_controller_flash_before_render.rb AllCops: Exclude: diff --git a/app/controllers/api_keys_controller.rb b/app/controllers/api_keys_controller.rb index 0c534bff618..f30ac8ab401 100644 --- a/app/controllers/api_keys_controller.rb +++ b/app/controllers/api_keys_controller.rb @@ -21,7 +21,7 @@ def create @api_key = ApiKey.new(build_params) if @api_key.errors.present? - flash[:error] = @api_key.errors.full_messages.to_sentence + flash.now[:error] = @api_key.errors.full_messages.to_sentence @api_key = current_user.api_keys.build(api_key_params.merge(rubygem_id: nil)) return render :new end @@ -32,7 +32,7 @@ def create session[:api_key] = key redirect_to profile_api_keys_path, flash: { notice: t(".success") } else - flash[:error] = @api_key.errors.full_messages.to_sentence + flash.now[:error] = @api_key.errors.full_messages.to_sentence render :new end end @@ -50,14 +50,14 @@ def update @api_key.assign_attributes(api_key_params) if @api_key.errors.present? - flash[:error] = @api_key.errors.full_messages.to_sentence + flash.now[:error] = @api_key.errors.full_messages.to_sentence return render :edit end if @api_key.save redirect_to profile_api_keys_path, flash: { notice: t(".success") } else - flash[:error] = @api_key.errors.full_messages.to_sentence + flash.now[:error] = @api_key.errors.full_messages.to_sentence render :edit end end diff --git a/app/controllers/multifactor_auths_controller.rb b/app/controllers/multifactor_auths_controller.rb index 1ddfe2469b3..761ba2e3a77 100644 --- a/app/controllers/multifactor_auths_controller.rb +++ b/app/controllers/multifactor_auths_controller.rb @@ -19,7 +19,7 @@ def create flash[:error] = current_user.errors[:base].join redirect_to edit_settings_url else - flash[:success] = t(".success") + flash.now[:success] = t(".success") render :recovery end end diff --git a/app/controllers/owners_controller.rb b/app/controllers/owners_controller.rb index 560cb753fbc..f1a9d4088a3 100644 --- a/app/controllers/owners_controller.rb +++ b/app/controllers/owners_controller.rb @@ -77,7 +77,7 @@ def notify_owner_added(ownership) def index_with_error(msg, status) @ownerships = @rubygem.ownerships_including_unconfirmed.includes(:user, :authorizer) - flash[:alert] = msg + flash.now[:alert] = msg render :index, status: status end diff --git a/lib/custom_cops/action_controller_flash_before_render.rb b/test/action_controller_flash_before_render.rb similarity index 63% rename from lib/custom_cops/action_controller_flash_before_render.rb rename to test/action_controller_flash_before_render.rb index 632d65c28af..045cbf0a739 100644 --- a/lib/custom_cops/action_controller_flash_before_render.rb +++ b/test/action_controller_flash_before_render.rb @@ -6,6 +6,10 @@ module Rails # Using `flash` assignment before `render` will persist the message for too long. # Check https://guides.rubyonrails.org/action_controller_overview.html#flash-now # + # @safety + # This cop's autocorrection is unsafe because it replaces `flash` by `flash.now`. + # Even though it is usually a mistake, it might be used intentionally. + # # @example # # bad # flash[:alert] = "Warning!" @@ -18,9 +22,9 @@ module Rails class ActionControllerFlashBeforeRender < Base extend AutoCorrector - MSG = 'Use `flash.now` before `render`.' + MSG = "Use `flash.now` before `render`." - def_node_search :flash_before_render?, <<~PATTERN + def_node_search :flash_assignment?, <<~PATTERN ^(send (send nil? :flash) :[]= ...) PATTERN @@ -31,15 +35,17 @@ class ActionControllerFlashBeforeRender < Base RESTRICT_ON_SEND = [:flash].freeze def on_send(node) - expression = flash_before_render?(node) - + expression = flash_assignment?(node) return unless expression context = node.parent.parent - return unless context.descendants.any? { render?(_1) } + return unless context + + siblings = context.descendants + return unless siblings.any? { |sibling| render?(sibling) } add_offense(node) do |corrector| - corrector.replace(node, 'flash.now') + corrector.replace(node, "flash.now") end end end