Skip to content

Commit

Permalink
Add Rails/ActionControllerFlashBeforeRender cop
Browse files Browse the repository at this point in the history
  • Loading branch information
americodls committed Aug 30, 2022
1 parent bceaa02 commit 51eeee2
Show file tree
Hide file tree
Showing 5 changed files with 230 additions and 0 deletions.
@@ -0,0 +1 @@
* [#759](https://github.com/rubocop/rubocop-rails/pull/759): Add new `Rails/ActionControllerFlashBeforeRender` cop. ([@americodls][])
8 changes: 8 additions & 0 deletions config/default.yml
Expand Up @@ -65,6 +65,14 @@ Rails:
Enabled: true
DocumentationBaseURL: https://docs.rubocop.org/rubocop-rails

Rails/ActionControllerFlashBeforeRender:
Description: 'Use `flash.now` instead of `flash` before `render`.'
StyleGuide: 'https://rails.rubystyle.guide/#flash-before-render'
Reference: 'https://api.rubyonrails.org/classes/ActionController/FlashBeforeRender.html'
Enabled: 'pending'
SafeAutocorrect: false
VersionAdded: '<<next>>'

Rails/ActionControllerTestCase:
Description: 'Use `ActionDispatch::IntegrationTest` instead of `ActionController::TestCase`.'
StyleGuide: 'https://rails.rubystyle.guide/#integration-testing'
Expand Down
89 changes: 89 additions & 0 deletions lib/rubocop/cop/rails/action_controller_flash_before_render.rb
@@ -0,0 +1,89 @@
# 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
#
# @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 ActionControllerFlashBeforeRender < Base
extend 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

def_node_search :action_controller?, <<~PATTERN
{
(const nil? :ApplicationController)
(const (const nil? :ActionController) :Base)
}
PATTERN

RESTRICT_ON_SEND = [:flash].freeze

def on_send(flash_node)
return unless flash_assignment?(flash_node)

return unless followed_by_render?(flash_node)

return unless instance_method_or_block?(flash_node)

return unless inherit_action_controller_base?(flash_node)

add_offense(flash_node) do |corrector|
corrector.replace(flash_node, 'flash.now')
end
end

private

def followed_by_render?(flash_node)
flash_assigment_node = find_ancestor(flash_node, type: :send)
context = flash_assigment_node.parent

context.each_child_node.any? do |node|
render?(node)
end
end

def inherit_action_controller_base?(node)
class_node = find_ancestor(node, type: :class)
return unless class_node

action_controller?(class_node)
end

def instance_method_or_block?(node)
def_node = find_ancestor(node, type: :def)
block_node = find_ancestor(node, type: :block)

def_node || block_node
end

def find_ancestor(node, type:)
node.each_ancestor(type).first
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Expand Up @@ -8,6 +8,7 @@
require_relative 'mixin/migrations_helper'
require_relative 'mixin/target_rails_version'

require_relative 'rails/action_controller_flash_before_render'
require_relative 'rails/action_controller_test_case'
require_relative 'rails/action_filter'
require_relative 'rails/active_record_aliases'
Expand Down
131 changes: 131 additions & 0 deletions spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb
@@ -0,0 +1,131 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::ActionControllerFlashBeforeRender, :config do
context 'when using `flash` before `render`' do
context 'within an instance method' do
%w[ActionController::Base ApplicationController].each do |parent_class|
context "within a class inherited from #{parent_class}" do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class HomeController < #{parent_class}
def create
flash[:alert] = "msg"
^^^^^ Use `flash.now` before `render`.
render :index
end
end
RUBY

expect_correction(<<~RUBY)
class HomeController < #{parent_class}
def create
flash.now[:alert] = "msg"
render :index
end
end
RUBY
end
end
end

context 'within a non Rails controller class' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class NonController < ApplicationRecord
def create
flash[:alert] = "msg"
render :index
end
end
RUBY
end
end
end

context 'within a block' do
%w[ActionController::Base ApplicationController].each do |parent_class|
context "within a class inherited from #{parent_class}" do
it 'registers an offense and corrects' do
expect_offense(<<~RUBY)
class HomeController < #{parent_class}
before_action do
flash[:alert] = "msg"
^^^^^ Use `flash.now` before `render`.
render :index
end
end
RUBY

expect_correction(<<~RUBY)
class HomeController < #{parent_class}
before_action do
flash.now[:alert] = "msg"
render :index
end
end
RUBY
end
end
end

context 'within a non Rails controller class' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class NonController < ApplicationRecord
before_action do
flash[:alert] = "msg"
render :index
end
end
RUBY
end
end
end

context 'within a class method' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class HomeController < ApplicationController
def self.create
flash[:alert] = "msg"
render :index
end
end
RUBY
end
end

context 'within a class body' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class HomeController < ApplicationController
flash[:alert] = "msg"
render :index
end
RUBY
end
end

context 'with no context' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
flash[:alert] = "msg"
render :index
RUBY
end
end
end

context 'when using `flash` before `redirect_to`' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class HomeController < ApplicationController
def create
flash[:alert] = "msg"
redirect_to "https://www.example.com/"
end
end
RUBY
end
end
end

0 comments on commit 51eeee2

Please sign in to comment.