Skip to content

Commit

Permalink
Fix flash assignments before render
Browse files Browse the repository at this point in the history
  • Loading branch information
americodls committed Aug 29, 2022
1 parent eb05be4 commit 4b95efd
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 13 deletions.
2 changes: 1 addition & 1 deletion .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:
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/api_keys_controller.rb
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/multifactor_auths_controller.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/owners_controller.rb
Expand Up @@ -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

Expand Down
Expand Up @@ -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!"
Expand All @@ -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

Expand All @@ -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
Expand Down

0 comments on commit 4b95efd

Please sign in to comment.