Skip to content

Commit

Permalink
[Fix #5261] Fix false positive for Style/MixinUsage when using insi…
Browse files Browse the repository at this point in the history
…de module

Fixes #5261.

Correspondence to use case for `include` belonging to module was
insufficient. This PR added a reproduction test and fixed it.
  • Loading branch information
koic authored and bbatsov committed Dec 20, 2017
1 parent bd76eab commit 02cfcfe
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@
* [#5234](https://github.com/bbatsov/rubocop/issues/5234): Fix a false positive for `Rails/HasManyOrHasOneDependent` when using `class_name` option. ([@koic][])
* [#5273](https://github.com/bbatsov/rubocop/issues/5273): Fix `Style/EvalWithLocation` reporting bad line offset. ([@pocke][])
* [#5228](https://github.com/bbatsov/rubocop/issues/5228): Handle overridden `Metrics/LineLength:Max` for `--auto-gen-config`. ([@jonas054][])
* [#5261](https://github.com/bbatsov/rubocop/issues/5261): Fix a false positive for `Style/MixinUsage` when using inside class or module. ([@koic][])

### Changes

Expand Down
15 changes: 13 additions & 2 deletions lib/rubocop/cop/style/mixin_usage.rb
Expand Up @@ -53,8 +53,9 @@ class MixinUsage < Cop

def on_send(node)
include_statement(node) do |statement|
return if node.argument?
return if accepted_include?(node)
return if node.argument? ||
accepted_include?(node) ||
belongs_to_class_or_module?(node)

add_offense(node, message: format(MSG, statement: statement))
end
Expand All @@ -65,6 +66,16 @@ def on_send(node)
def accepted_include?(node)
node.parent && node.macro?
end

def belongs_to_class_or_module?(node)
if !node.parent
false
else
return true if node.parent.class_type? || node.parent.module_type?

belongs_to_class_or_module?(node.parent)
end
end
end
end
end
Expand Down
32 changes: 32 additions & 0 deletions spec/rubocop/cop/style/mixin_usage_spec.rb
Expand Up @@ -29,6 +29,16 @@ class C
RUBY
end

it 'registers an offense when using `include` in method definition ' \
'outside class or module' do
expect_offense(<<-RUBY.strip_indent)
def foo
include M
^^^^^^^^^ `include` is used at the top level. Use inside `class` or `module`.
end
RUBY
end

it 'does not register an offense when using outside class' do
expect_no_offenses(<<-RUBY.strip_indent)
Foo.include M
Expand Down Expand Up @@ -58,6 +68,28 @@ class C
RUBY
end

it 'does not register an offense when using `include` in method ' \
'definition inside class' do
expect_no_offenses(<<-RUBY.strip_indent)
class X
def foo
include M
end
end
RUBY
end

it 'does not register an offense when using `include` in method ' \
'definition inside module' do
expect_no_offenses(<<-RUBY.strip_indent)
module X
def foo
include M
end
end
RUBY
end

context 'Multiple definition classes in one' do
it 'does not register an offense when using inside class' do
expect_no_offenses(<<-RUBY.strip_indent)
Expand Down

0 comments on commit 02cfcfe

Please sign in to comment.