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 #9588] Fix causing a variable to be shadowed from outside the rescue block #9961

Merged
merged 1 commit into from Sep 28, 2021
Merged

[Fix #9588] Fix causing a variable to be shadowed from outside the rescue block #9961

merged 1 commit into from Sep 28, 2021

Conversation

lilisako
Copy link
Contributor

@lilisako lilisako commented Jul 30, 2021

Fixes #9588
Fix causing a variable to be shadowed from outside the rescue block.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@lilisako lilisako marked this pull request as draft July 30, 2021 10:28
@lilisako lilisako marked this pull request as ready for review July 30, 2021 10:54
@lilisako lilisako marked this pull request as draft July 30, 2021 10:54
@lilisako lilisako marked this pull request as ready for review July 30, 2021 11:20
@lilisako lilisako marked this pull request as draft August 12, 2021 00:53
@@ -75,6 +75,9 @@ def on_resbody(node)
preferred_name = preferred_name(offending_name)
return if preferred_name.to_sym == offending_name

# check variable shadowing for exception variable
return if check_shadowed_variable_name(node)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the caller name as well?

Suggested change
return if check_shadowed_variable_name(node)
return if shadowed_variable_name?(node)

node.each_descendant(:lvar) do |n|
return true if n.children.first.to_s == preferred_name(n)
end
false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that it can be simplified by using any?.

         def shadowed_variable_name?(node)
-          node.each_descendant(:lvar) do |n|
-            return true if n.children.first.to_s == preferred_name(n)
-          end
-          false
+          node.each_descendant(:lvar).any? { |n| n.children.first.to_s == preferred_name(n) }
         end

@dvandersluis
Copy link
Member

@lilisako ping!

@koic
Copy link
Member

koic commented Sep 28, 2021

@lilisako Can you make this CI successful?

@lilisako lilisako marked this pull request as ready for review September 28, 2021 03:59
@dvandersluis
Copy link
Member

Looks like you need to rebase your branch.

@lilisako
Copy link
Contributor Author

Sorry for getting back to you late. I'll do it within 5-6 hours! When it's done I'm doing to mention you. Thanks!

…scue block

This PR fixes causing a variable to be shadowed from outside the rescue block from Naming/RescuedExceptionsVariableName
@lilisako
Copy link
Contributor Author

lilisako commented Sep 28, 2021

@koic @dvandersluis
Just passed all CI tests. Could you re-review them? Thanks! Again my apology to the late response.

@koic koic merged commit a94b9fa into rubocop:master Sep 28, 2021
@koic
Copy link
Member

koic commented Sep 28, 2021

Thanks!

@lilisako lilisako deleted the fix-shadow-variable-in-rescued-exceptions-variable-name branch September 28, 2021 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Naming/RescuedExceptionsVariableName can cause a variable to be shadowed from outside the rescue block
3 participants