diff --git a/.rubocop.yml b/.rubocop.yml index 8d8536c8267..ca4f42df018 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,7 +4,7 @@ require: - rubocop-performance - rubocop-rails - rubocop-minitest - - ./lib/custom_cops/action_controller_flash_before_render.rb + - ./test/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/lib/custom_cops/action_controller_flash_before_render.rb deleted file mode 100644 index 632d65c28af..00000000000 --- a/lib/custom_cops/action_controller_flash_before_render.rb +++ /dev/null @@ -1,48 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - 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 - # - # @example - # # bad - # flash[:alert] = "Warning!" - # render :index - # - # # good - # flash.now[:alert] = "Warning!" - # render :index - # - class ActionControllerFlashBeforeRender < Base - extend AutoCorrector - - MSG = 'Use `flash.now` before `render`.' - - def_node_search :flash_before_render?, <<~PATTERN - ^(send (send nil? :flash) :[]= ...) - PATTERN - - def_node_search :render?, <<~PATTERN - (send nil? :render ...) - PATTERN - - RESTRICT_ON_SEND = [:flash].freeze - - def on_send(node) - expression = flash_before_render?(node) - - return unless expression - - context = node.parent.parent - return unless context.descendants.any? { render?(_1) } - - add_offense(node) do |corrector| - corrector.replace(node, 'flash.now') - end - end - end - end - end -end diff --git a/test/action_controller_flash_before_render.rb b/test/action_controller_flash_before_render.rb new file mode 100644 index 00000000000..8ba93d6ef42 --- /dev/null +++ b/test/action_controller_flash_before_render.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +# 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!" +# render :index +# +# # good +# flash.now[:alert] = "Warning!" +# render :index +# +class RuboCop::Cop::Rails::ActionControllerFlashBeforeRender < RuboCop::Cop::Cop + extend RuboCop::Cop::AutoCorrector + + MSG = "Use `flash.now` before `render`." + + def_node_search :flash_assignment?, <<~PATTERN + ^(send (send nil? :flash) :[]= ...) + PATTERN + + def_node_search :render?, <<~PATTERN + (send nil? :render ...) + PATTERN + + RESTRICT_ON_SEND = [:flash].freeze + + def on_send(node) + expression = flash_assignment?(node) + return unless expression + + context = node.parent.parent + return unless context + + siblings = context.descendants + return unless siblings.any? { |sibling| render?(sibling) } + + add_offense(node) do |corrector| + corrector.replace(node, "flash.now") + end + end +end