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

B018 to detect useless-statements at all levels #434

Merged
merged 2 commits into from
Dec 2, 2023

Conversation

r-downing
Copy link
Contributor

The B018 useless-statement checker was only running at the top level of modules, classes, and functions. This meant that it wouldn't detect things inside any nested blocks - e.g. if, for, while, try, etc. Did a minor refactor to make the checker run during the basic visit function so it hits things in all scopes.

Example. See added test file for more

[1, 2, 3]  # useless list - would catch this

if x:
    [1, 2, 3]  # useless list - would not catch this

@cooperlees
Copy link
Collaborator

O, I was literally about to release a new version. Will see how CI likes this and review soon.

@@ -1164,31 +1163,30 @@ def check_for_b903(self, node):
self.errors.append(B903(node.lineno, node.col_offset))

def check_for_b018(self, node):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is mostly the same - just removing the toplevel for loop and de-indenting, renaming loopvar subnode to node. Can turn on "ignore-whitespace" option on PR view for easier review

Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

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

The AST is fun ... I always struggle with it ... Thanks for noticing and fixing this.

@cooperlees cooperlees merged commit 89f950c into PyCQA:main Dec 2, 2023
6 checks passed
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.

2 participants