Skip to content

Commit

Permalink
[Fix #4172] Fix false positives in Style/MixinGrouping cop
Browse files Browse the repository at this point in the history
This cop would register an offense when one of the methods `#include`,
`#extend`, or `#prepend` was sent to an explicit receiver, or used as an
argument to another method.

This change fixes that.
  • Loading branch information
Drenmi authored and bbatsov committed Mar 29, 2017
1 parent d602d0a commit 12399a5
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* [#4171](https://github.com/bbatsov/rubocop/pull/4171): Prevent `Rails/Blank` from breaking when RHS of `or` is a naked falsiness check. ([@drenmi][])
* [#4189](https://github.com/bbatsov/rubocop/pull/4189): Make `Lint/AmbiguousBlockAssociation` aware of lambdas passed as arguments. ([@drenmi][])
* [#4179](https://github.com/bbatsov/rubocop/pull/4179): Prevent `Rails/Blank` from breaking when LHS of `or` is a naked falsiness check. ([@rrosenblum][])
* [#4172](https://github.com/bbatsov/rubocop/pull/4172): Fix false positives in `Style/MixinGrouping` cop. ([@drenmi][])

## 0.48.0 (2017-03-26)

Expand Down
11 changes: 8 additions & 3 deletions lib/rubocop/ast/node/send_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ module AST
# node when the builder constructs the AST, making its methods available
# to all `send` nodes within RuboCop.
class SendNode < Node
MACRO_PARENT_NODES = %i(class module).freeze

# The receiving node of the method invocation.
#
# @return [Node, nil] the receiver of the invoked method or `nil`
Expand Down Expand Up @@ -38,7 +36,7 @@ def method?(name)
#
# @return [Boolean] whether the method is a macro method
def macro?
!receiver && MACRO_PARENT_NODES.include?(parent && parent.type)
!receiver && macro_scope?
end

# Checks whether the method name matches the argument and has an
Expand Down Expand Up @@ -185,6 +183,13 @@ def splat_argument?
def node_parts
[*self]
end

private # rubocop:disable Lint/UselessAccessModifier

def_matcher :macro_scope?, <<-PATTERN
{^({class module} ...)
^^({class module} ... (begin ...))}
PATTERN
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/mixin_grouping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class MixinGrouping < Cop
MSG = 'Put `%s` mixins in %s.'.freeze

def on_send(node)
return unless MIXIN_METHODS.include?(node.method_name)
return unless node.macro? && MIXIN_METHODS.include?(node.method_name)

check(node)
end
Expand Down
6 changes: 4 additions & 2 deletions spec/rubocop/ast/send_node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,25 @@
describe '#macro?' do
context 'without a receiver' do
context 'when parent is a class' do
let(:send_node) { parse_source(source).ast.children[2] }
let(:send_node) { parse_source(source).ast.children[2].children[0] }

let(:source) do
['class Foo',
' bar :baz',
' bar :qux',
'end'].join("\n")
end

it { expect(send_node.macro?).to be_truthy }
end

context 'when parent is a module' do
let(:send_node) { parse_source(source).ast.children[1] }
let(:send_node) { parse_source(source).ast.children[1].children[0] }

let(:source) do
['module Foo',
' bar :baz',
' bar :qux',
'end'].join("\n")
end

Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/style/mixin_grouping_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@
' include Qux',
'end'].join("\n")
end

context 'when include call is an argument to another method' do
it_behaves_like 'code without offense',
'expect(foo).to include(bar, baz)'
end
end

context 'when using `extend`' do
Expand Down Expand Up @@ -142,6 +147,12 @@
' include Bar, Qux',
'end'].join("\n")
end

context 'when include has an explicit receiver' do
it_behaves_like 'code without offense',
['config.include Foo',
'config.include Bar'].join("\n")
end
end

context 'when using `extend`' do
Expand Down

0 comments on commit 12399a5

Please sign in to comment.