From e625a3589375301361b8880b0d695537c8a298fa Mon Sep 17 00:00:00 2001 From: Americo Duarte Date: Sun, 28 Aug 2022 22:21:07 +0100 Subject: [PATCH] Add `Rails/ActionControllerFlashBeforeRender` cop --- ..._railsactioncontrollerflashbeforerender.md | 1 + config/default.yml | 8 ++ .../action_controller_flash_before_render.rb | 73 +++++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + ...ion_controller_flash_before_render_spec.rb | 66 +++++++++++++++++ 5 files changed, 149 insertions(+) create mode 100644 changelog/new_add_railsactioncontrollerflashbeforerender.md create mode 100644 lib/rubocop/cop/rails/action_controller_flash_before_render.rb create mode 100644 spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb diff --git a/changelog/new_add_railsactioncontrollerflashbeforerender.md b/changelog/new_add_railsactioncontrollerflashbeforerender.md new file mode 100644 index 0000000000..fd97953452 --- /dev/null +++ b/changelog/new_add_railsactioncontrollerflashbeforerender.md @@ -0,0 +1 @@ +* [#759](https://github.com/rubocop/rubocop-rails/pull/759): Add new `Rails/ActionControllerFlashBeforeRender` cop. ([@americodls][]) diff --git a/config/default.yml b/config/default.yml index 284b45cb11..286029a31a 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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: '<>' + Rails/ActionControllerTestCase: Description: 'Use `ActionDispatch::IntegrationTest` instead of `ActionController::TestCase`.' StyleGuide: 'https://rails.rubystyle.guide/#integration-testing' diff --git a/lib/rubocop/cop/rails/action_controller_flash_before_render.rb b/lib/rubocop/cop/rails/action_controller_flash_before_render.rb new file mode 100644 index 0000000000..fa54e472c4 --- /dev/null +++ b/lib/rubocop/cop/rails/action_controller_flash_before_render.rb @@ -0,0 +1,73 @@ +# 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(node) + expression = flash_assignment?(node) + return unless expression + + context = node.each_ancestor(:begin).first + return unless context + + siblings = context.descendants + return unless siblings.any? { |sibling| render?(sibling) } + + return unless inherit_action_controller_base?(node) + + add_offense(node) do |corrector| + corrector.replace(node, 'flash.now') + end + end + + def inherit_action_controller_base?(node) + class_node = node.each_ancestor(:class).first + return unless class_node + + def_node = node.each_ancestor(:def).first + return unless def_node + + action_controller?(class_node) + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 2f1e0996d1..b8c7612879 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -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' 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 new file mode 100644 index 0000000000..85a8ab49ff --- /dev/null +++ b/spec/rubocop/cop/rails/action_controller_flash_before_render_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::ActionControllerFlashBeforeRender, :config do + it 'registers an offense when using `flash` before `render` in rails controller' do + expect_offense(<<~RUBY) + class HomeController < ApplicationController + def create + flash[:alert] = "msg" + ^^^^^ Use `flash.now` before `render`. + render :index + end + end + RUBY + + expect_correction(<<~RUBY) + class HomeController < ApplicationController + def create + flash.now[:alert] = "msg" + render :index + end + end + RUBY + end + + it 'does not register an offense when using `flash` before `redirect_to` in rails controller instance method' do + expect_no_offenses(<<~RUBY) + class HomeController < ApplicationController + def create + flash[:alert] = "msg" + redirect_to "https://www.google.com/" + end + end + RUBY + end + + it 'does not register an offense when using `flash` before `redirect_to` in rails controller class method' do + expect_no_offenses(<<~RUBY) + class HomeController < ApplicationController + def self.create + flash[:alert] = "msg" + redirect_to "https://www.google.com/" + end + end + RUBY + end + + it 'does not register an offense when using `flash` before `render` in rails controller class' do + expect_no_offenses(<<~RUBY) + class HomeController < ApplicationController + flash[:alert] = "msg" + render :index + end + RUBY + end + + it 'does not register an offense when using `flash` before `render` but it is not in rails controller' do + expect_no_offenses(<<~RUBY) + class NonController < ApplicationRecord + def create + flash[:alert] = "msg" + render :index + end + end + RUBY + end +end