Skip to content

Commit

Permalink
[Fix #3120] Fix #new visibility modifier scopes (#3126)
Browse files Browse the repository at this point in the history
Blocks passed to {Class,Module,Struct}.new create a new scope, in a
similar way to Klass.class_eval.
  • Loading branch information
owst authored and bbatsov committed May 11, 2016
1 parent 343e353 commit a65e56e
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Bug fixes

* [#3114](https://github.com/bbatsov/rubocop/issues/3114): Fix alignment `end` when auto-correcting `Style/EmptyElse`. ([@rrosenblum][])
* [#3120](https://github.com/bbatsov/rubocop/issues/3120): Fix `Lint/UselessAccessModifier` reporting useless access modifiers inside {Class,Module,Struct}.new blocks. ([@owst][])

## 0.40.0 (2016-05-09)

Expand Down
13 changes: 11 additions & 2 deletions lib/rubocop/cop/lint/useless_access_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def on_module(node)
end

def on_block(node)
return unless class_or_instance_eval?(node)
return unless instance_eval_or_new_call?(node)

check_node(node.children[2]) # block body
end
Expand All @@ -95,6 +95,10 @@ def on_sclass(node)
(block (send _ {:class_eval :instance_eval}) ...)
PATTERN

def_node_matcher :class_or_module_or_struct_new_call?, <<-PATTERN
(block (send (const nil {:Class :Module :Struct}) :new ...) ...)
PATTERN

def check_node(node)
return if node.nil?

Expand Down Expand Up @@ -143,7 +147,12 @@ def method_definition?(child)

def start_of_new_scope?(child)
child.module_type? || child.class_type? ||
child.sclass_type? || class_or_instance_eval?(child)
child.sclass_type? || instance_eval_or_new_call?(child)
end

def instance_eval_or_new_call?(child)
class_or_instance_eval?(child) ||
class_or_module_or_struct_new_call?(child)
end
end
end
Expand Down
26 changes: 26 additions & 0 deletions spec/rubocop/cop/lint/useless_access_modifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,26 @@
end
end

shared_examples 'def in new block' do |klass, modifier|
it "doesn't register an offense if a method is defined in #{klass}.new" do
src = ["#{klass}.new do",
" #{modifier}",
' def foo',
' end',
'end']
inspect_source(cop, src)
expect(cop.offenses).to be_empty
end

it "registers an offense if no method is defined in #{klass}.new" do
src = ["#{klass}.new do",
" #{modifier}",
'end']
inspect_source(cop, src)
expect(cop.offenses.size).to eq(1)
end
end

shared_examples 'method defined using instance_eval' do |modifier|
it "doesn't register an offense if a method is defined" do
src = ['A.instance_eval do',
Expand Down Expand Up @@ -709,6 +729,12 @@
it_behaves_like('method defined using instance_eval', modifier)
end

%w(Class Module Struct).each do |klass|
%w(protected private).each do |modifier|
it_behaves_like('def in new block', klass, modifier)
end
end

%w(module class).each do |keyword|
it_behaves_like('at the top of the body', keyword)
it_behaves_like('non-repeated visibility modifiers', keyword)
Expand Down

0 comments on commit a65e56e

Please sign in to comment.