From 03ed951c167920ab6bcca4b62ab65f483808c680 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 1 Sep 2021 14:14:18 +0900 Subject: [PATCH] [Fix #529] Fix a false positive for `Rails/LexicallyScopedActionFilter` Fixes #529. This PR fixes a false positive for `Rails/LexicallyScopedActionFilter` when action method is aliased by `alias_method`. --- CHANGELOG.md | 1 + .../rails/lexically_scoped_action_filter.rb | 25 +++++++++++-- .../lexically_scoped_action_filter_spec.rb | 35 +++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5611f5626e..aaf0edfe11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ### Bug fixes * [#528](https://github.com/rubocop/rubocop-rails/issues/528): Fix a false positive for `Rails/HasManyOrHasOneDependent` when specifying `:dependent` strategy with double splat. ([@koic][]) +* [#529](https://github.com/rubocop/rubocop-rails/issues/529): Fix a false positive for `Rails/LexicallyScopedActionFilter` when action method is aliased by `alias_method`. ([@koic][]) ## Changes diff --git a/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb b/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb index e27e6905ae..851a4dab6d 100644 --- a/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb +++ b/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb @@ -124,9 +124,10 @@ def on_send(node) block = parent.each_child_node(:begin).first return unless block - defined_methods = block.each_child_node(:def).map(&:method_name) + defined_action_methods = defined_action_methods(block) + methods = array_values(methods_node).reject do |method| - defined_methods.include?(method) + defined_action_methods.include?(method) end message = message(methods, parent) @@ -135,6 +136,26 @@ def on_send(node) private + def defined_action_methods(block) + defined_methods = block.each_child_node(:def).map(&:method_name) + + defined_methods + aliased_action_methods(block, defined_methods) + end + + def aliased_action_methods(node, defined_methods) + alias_methods = node.each_child_node(:send).select { |send_node| send_node.method?(:alias_method) } + + hash_of_alias_methods = alias_methods.each_with_object({}) do |alias_method, result| + result[alias_method.last_argument.value] = alias_method.first_argument.value + end + + defined_methods.each_with_object([]) do |defined_method, aliased_method| + if (new_method_name = hash_of_alias_methods[defined_method]) + aliased_method << new_method_name + end + end + end + # @param node [RuboCop::AST::Node] # @return [Array] def array_values(node) # rubocop:disable Metrics/MethodLength diff --git a/spec/rubocop/cop/rails/lexically_scoped_action_filter_spec.rb b/spec/rubocop/cop/rails/lexically_scoped_action_filter_spec.rb index f36e9325ca..491beb156f 100644 --- a/spec/rubocop/cop/rails/lexically_scoped_action_filter_spec.rb +++ b/spec/rubocop/cop/rails/lexically_scoped_action_filter_spec.rb @@ -115,6 +115,41 @@ def logout RUBY end + it 'does not register an offense when action method is aliased by `alias_method`' do + expect_no_offenses(<<~RUBY) + class FooController < ApplicationController + before_action :authorize!, only: %i[index show] + + def index + end + alias_method :show, :index + + private + + def authorize! + end + end + RUBY + end + + it 'registers an offense when action method is not aliased by `alias_method`' do + expect_offense(<<~RUBY) + class FooController < ApplicationController + before_action :authorize!, only: %i[foo show] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `foo` is not explicitly defined on the class. + + def index + end + alias_method :show, :index + + private + + def authorize! + end + end + RUBY + end + it "doesn't register an offense when using conditional statements" do expect_no_offenses <<~RUBY class Test < ActionController