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 false negative for multiple-statements on try... else/finally lines #9917

Merged
merged 3 commits into from
Sep 8, 2024

Conversation

zenlyj
Copy link
Contributor

@zenlyj zenlyj commented Sep 7, 2024

Type of Changes

Type
βœ“ πŸ› Bug fix
✨ New feature
πŸ”¨ Refactoring
πŸ“œ Docs

Description

I took a look at 5bdc221 and noticed that some inference logic got removed in format.py while refactoring to use the updated Try node. The purpose of this PR is to reintroduce the inference of else and finally line numbers to check for multiple statements on else/finally lines in a try statement.

This check is a little explicit because of how else and finally is represented in the AST, without a tree node to represent an else or finally line. However, it should work for general cases, except for unusual cases like:

try:
    pass
except:
    pass
#  this comment will prevent multiple-statements from flagging in the next line
finally: pass

Closes #9759

This comment has been minimized.

@zenlyj zenlyj force-pushed the fix/9759-fn-multi-statement branch 2 times, most recently from 42b3e7c to e6df81f Compare September 7, 2024 16:04

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Sep 7, 2024

πŸ€– According to the primer, this change has no effect on the checked open source code. πŸ€–πŸŽ‰

This comment was generated for commit 2e5ff16

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

I think #9907 broke your coverage workflow.

@Pierre-Sassoulas can we merge this while looking into that?

@jacobtylerwalls jacobtylerwalls merged commit 659be29 into pylint-dev:main Sep 8, 2024
41 of 42 checks passed
@jacobtylerwalls jacobtylerwalls added this to the 3.3.0 milestone Sep 8, 2024
@zenlyj
Copy link
Contributor Author

zenlyj commented Sep 8, 2024

Appreciate the review! @DanielNoord

@zenlyj zenlyj deleted the fix/9759-fn-multi-statement branch September 8, 2024 15:43
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.

multiple-statements no longer detected in a finally: block
3 participants