Skip to content

Commit

Permalink
Only traverse send nodes in def_modifier?
Browse files Browse the repository at this point in the history
A send node is a "def modifier" if it's part of a chain of method calls
preceding a method definition. However the previous implementation was
too permissive and matched any method definition in the send node's AST.

In the following example, `include` was considered to be a def modifier:

    include Module.new {
      def foo
      end
    }
  • Loading branch information
eugeneius authored and marcandre committed Oct 22, 2020
1 parent 6e14c43 commit 12734b6
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,4 @@
[@fatkodima]: https://github.com/fatkodima
[@koic]: https://github.com/koic
[@dvandersluis]: https://github.com/dvandersluis
[@eugeneius]: https://github.com/eugeneius
1 change: 1 addition & 0 deletions changelog/fix_only_traverse_send_nodes_in_def_modifier.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#142](https://github.com/rubocop-hq/rubocop-ast/pull/142): Only traverse send nodes in `MethodDispatchNode#def_modifier?`. ([@eugeneius][])
2 changes: 1 addition & 1 deletion lib/rubocop/ast/node/mixin/method_dispatch_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def arithmetic_operation?
# @return [Boolean] whether the dispatched method is a `def` modifier
def def_modifier?
send_type? &&
[self, *each_descendant(:send)].any?(&:adjacent_def_modifier?)
adjacent_def_modifier? || each_child_node(:send).any?(&:def_modifier?)
end

# Checks whether this is a lambda. Some versions of parser parses
Expand Down
8 changes: 7 additions & 1 deletion spec/rubocop/ast/send_node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1099,10 +1099,16 @@ def bar
end

context 'with several prefixed def modifiers' do
let(:source) { 'foo bar def baz; end' }
let(:source) { 'foo bar baz def qux; end' }

it { expect(send_node).to be_def_modifier }
end

context 'with a block containing a method definition' do
let(:source) { 'foo bar { baz def qux; end }' }

it { expect(send_node).not_to be_def_modifier }
end
end

describe '#negation_method?' do
Expand Down

0 comments on commit 12734b6

Please sign in to comment.