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

include tuples in b018 useless-statement check #432

Merged
merged 3 commits into from Dec 1, 2023

Conversation

r-downing
Copy link
Contributor

I was looking for something to detect useless tuple statements, noticed this had a useless-statement check but didn't include tuples. Any reason for that? Otherwise, figured we could add it?

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.

Read again this morning and still confused at some of the test results ... I feel I'm missing something - Can you explain my missing context please :)

@@ -270,6 +270,8 @@ def test_b018_functions(self):
expected = [B018(line, 4) for line in range(15, 25)]
expected.append(B018(28, 4))
expected.append(B018(31, 4))
expected.append(B018(32, 4))
expected.append(B018(33, 4))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we not need 34 here when we include 32 for modules tests? What am I missing here, cause the variable assign happens in a function it's fine?

Copy link
Contributor Author

@r-downing r-downing Nov 30, 2023

Choose a reason for hiding this comment

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

Oh it's a little confusing because the line numbers for the three different test cases are referring to the 3 different b018_....py files (but the line numbers are almost the same for 2 of the files). Added the same 3 lines to each of the files and corresponding test cases, in all cases the first 2 lines are bad and the 3rd is good

    (1,)  # bad
    (2, 3)  # bad
    t = (4, 5)  # good

tests/b018_modules.py Outdated Show resolved Hide resolved
tests/b018_functions.py Outdated Show resolved Hide resolved
tests/b018_classes.py Outdated Show resolved Hide resolved
@r-downing
Copy link
Contributor Author

Something else to note here - a dangling comma after a single thing (e.g. a function call) will turn it into a tuple, which would get flagged as a useless statement:

print("abc")  # this is fine
print("abc"),  # the comma essentially makes this a tuple (None,) so this would be flagged as a useless expression.

^ wondering if this might be confusing and should be labelled a little differently? Or even as separate error? E.g. useless tuple expression - suggest removing the comma

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.

Ok, I get it now. Yeah, I like maybe adding the "Do you have a trailing comma" at least in the README should help some lost people ...

@cooperlees cooperlees merged commit 6686e52 into PyCQA:main Dec 1, 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.

None yet

2 participants