Skip to content

Commit

Permalink
[Fix rubocop#10512] Fix a false positive for `Lint/ShadowingOuterLoca…
Browse files Browse the repository at this point in the history
…lVariable` conditional statement and block variable
  • Loading branch information
ydah committed Jun 13, 2022
1 parent 63948bb commit 1965935
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 32 deletions.
@@ -0,0 +1 @@
* [#10512](https://github.com/rubocop/rubocop/issues/10512): Fix a false positive for `Lint/ShadowingOuterLocalVariable` conditional statement and block variable. ([@ydah][])
10 changes: 9 additions & 1 deletion lib/rubocop/cop/lint/shadowing_outer_local_variable.rb
Expand Up @@ -67,10 +67,18 @@ def same_conditions_node_different_branch?(variable, outer_local_variable)
variable_node = variable.scope.node.parent
return false unless variable_node.conditional?

outer_local_variable_node = outer_local_variable.scope.node
outer_local_variable_node =
find_conditional_node_from_ascendant(outer_local_variable.declaration_node)

outer_local_variable_node.conditional? && variable_node == outer_local_variable_node
end

def find_conditional_node_from_ascendant(node)
return unless (parent = node.parent)
return parent if parent.conditional?

find_conditional_node_from_ascendant(parent)
end
end
end
end
Expand Down
114 changes: 83 additions & 31 deletions spec/rubocop/cop/lint/shadowing_outer_local_variable_spec.rb
Expand Up @@ -61,7 +61,7 @@ def some_method
end

context 'when a block local variable has same name as an outer scope variable' \
'with same branches of same `if` condition node' do
'with same branches of same `if` condition node not in the method definition' do
it 'registers an offense' do
expect_offense(<<~RUBY)
if condition?
Expand All @@ -79,36 +79,82 @@ def some_method
end

context 'when a block local variable has same name as an outer scope variable' \
'with same branches of same `unless` condition node' do
'with same branches of same `if` condition node' do
it 'registers an offense' do
expect_offense(<<~RUBY)
unless condition?
foo = 1
puts foo
bar.each do |foo|
^^^ Shadowing outer local variable - `foo`.
def some_method
if condition?
foo = 1
puts foo
bar.each do |foo|
^^^ Shadowing outer local variable - `foo`.
end
else
bar.each do |foo|
end
end
else
bar.each do |foo|
end
RUBY
end
end

context 'when a block local variable has same name as an outer scope variable' \
'with same branches of same nested `if` condition node' do
it 'registers an offense' do
expect_offense(<<~RUBY)
def some_method
if condition?
foo = 1
puts foo
if other_condition?
bar.each do |foo|
^^^ Shadowing outer local variable - `foo`.
end
end
else
bar.each do |foo|
end
end
end
RUBY
end
end

context 'when a block local variable has same name as an outer scope variable' \
'with same branches of same `case` condition node' do
'with same branches of same `unless` condition node' do
it 'registers an offense' do
expect_offense(<<~RUBY)
case condition
when foo then
foo = 1
puts foo
bar.each do |foo|
^^^ Shadowing outer local variable - `foo`.
def some_method
unless condition?
foo = 1
puts foo
bar.each do |foo|
^^^ Shadowing outer local variable - `foo`.
end
else
bar.each do |foo|
end
end
else
bar.each do |foo|
end
RUBY
end
end

context 'when a block local variable has same name as an outer scope variable' \
'with same branches of same `case` condition node' do
it 'registers an offense' do
expect_offense(<<~RUBY)
def some_method
case condition
when foo then
foo = 1
puts foo
bar.each do |foo|
^^^ Shadowing outer local variable - `foo`.
end
else
bar.each do |foo|
end
end
end
RUBY
Expand All @@ -119,10 +165,12 @@ def some_method
'with different branches of same `if` condition node' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
if condition?
foo = 1
else
bar.each do |foo|
def some_method
if condition?
foo = 1
else
bar.each do |foo|
end
end
end
RUBY
Expand All @@ -133,10 +181,12 @@ def some_method
'with different branches of same `unless` condition node' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
unless condition?
foo = 1
else
bar.each do |foo|
def some_method
unless condition?
foo = 1
else
bar.each do |foo|
end
end
end
RUBY
Expand All @@ -147,11 +197,13 @@ def some_method
'with different branches of same `case` condition node' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
case condition
when foo then
foo = 1
else
bar.each do |foo|
def some_method
case condition
when foo then
foo = 1
else
bar.each do |foo|
end
end
end
RUBY
Expand Down

0 comments on commit 1965935

Please sign in to comment.