From 19659353c6831841e3c765bb144f355ef56a90f9 Mon Sep 17 00:00:00 2001 From: ydah <13041216+ydah@users.noreply.github.com> Date: Tue, 14 Jun 2022 06:36:03 +0900 Subject: [PATCH] [Fix #10512] Fix a false positive for `Lint/ShadowingOuterLocalVariable` conditional statement and block variable --- ...tive_for_shadowing_outer_local_variable.md | 1 + .../lint/shadowing_outer_local_variable.rb | 10 +- .../shadowing_outer_local_variable_spec.rb | 114 +++++++++++++----- 3 files changed, 93 insertions(+), 32 deletions(-) create mode 100644 changelog/fix_a_false_positive_for_shadowing_outer_local_variable.md diff --git a/changelog/fix_a_false_positive_for_shadowing_outer_local_variable.md b/changelog/fix_a_false_positive_for_shadowing_outer_local_variable.md new file mode 100644 index 00000000000..97bad27c105 --- /dev/null +++ b/changelog/fix_a_false_positive_for_shadowing_outer_local_variable.md @@ -0,0 +1 @@ +* [#10512](https://github.com/rubocop/rubocop/issues/10512): Fix a false positive for `Lint/ShadowingOuterLocalVariable` conditional statement and block variable. ([@ydah][]) diff --git a/lib/rubocop/cop/lint/shadowing_outer_local_variable.rb b/lib/rubocop/cop/lint/shadowing_outer_local_variable.rb index 29f4a83eff3..a381f8e87a1 100644 --- a/lib/rubocop/cop/lint/shadowing_outer_local_variable.rb +++ b/lib/rubocop/cop/lint/shadowing_outer_local_variable.rb @@ -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 diff --git a/spec/rubocop/cop/lint/shadowing_outer_local_variable_spec.rb b/spec/rubocop/cop/lint/shadowing_outer_local_variable_spec.rb index ffd3993ef01..29f557ce6a8 100644 --- a/spec/rubocop/cop/lint/shadowing_outer_local_variable_spec.rb +++ b/spec/rubocop/cop/lint/shadowing_outer_local_variable_spec.rb @@ -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? @@ -79,17 +79,41 @@ 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 @@ -97,18 +121,40 @@ def some_method 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 @@ -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 @@ -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 @@ -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