Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #10512] Fix a false positive for Lint/ShadowingOuterLocalVariable conditional statement and block variable #10513

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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