Skip to content

Commit

Permalink
[Fix rubocop#529] Fix a false positive for `Rails/LexicallyScopedActi…
Browse files Browse the repository at this point in the history
…onFilter`

Fixes rubocop#529.

This PR fixes a false positive for `Rails/LexicallyScopedActionFilter`
when action method is aliased by `alias_method`.
  • Loading branch information
koic committed Sep 2, 2021
1 parent 7e62ce2 commit 03ed951
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 23 additions & 2 deletions lib/rubocop/cop/rails/lexically_scoped_action_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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<Symbol>]
def array_values(node) # rubocop:disable Metrics/MethodLength
Expand Down
35 changes: 35 additions & 0 deletions spec/rubocop/cop/rails/lexically_scoped_action_filter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 03ed951

Please sign in to comment.