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

Make superfluous-parens consider string combinations #4792

Closed
DanielNoord opened this issue Aug 3, 2021 · 5 comments · Fixed by #7752
Closed

Make superfluous-parens consider string combinations #4792

DanielNoord opened this issue Aug 3, 2021 · 5 comments · Fixed by #7752
Assignees
Labels
Enhancement ✨ Improvement to a component False Negative 🦋 No message is emitted but something is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@DanielNoord
Copy link
Collaborator

Current problem

Although #4784 fixes some problems with superfluous-parens it still does not consider the following cases:

Z = "TestString"
X = ("Test " + "String") # Bad
Y = ("Test " + "String") in Z # Bad
assert "" + ("Version " + "String") in Z # Bad

This is because the current checker focuses on unnecessary parens after keywords (if, else, etc.) and only if the ( immediately follows the keyword. See

if tokens[start + 1].string != "(":
            return

on line 373 in format.py and the fact that we only run when Keywords are present.

Ideally, the checker would also consider string combinations and seeing if parens are necessary there.

Desired solution

Raise superfluous-parens message on the test cases described above.

Additional context

No response

@DanielNoord DanielNoord added Enhancement ✨ Improvement to a component Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 3, 2021
@Pierre-Sassoulas Pierre-Sassoulas added False Negative 🦋 No message is emitted but something is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 3, 2021
@RaenonX

This comment has been minimized.

@Pierre-Sassoulas

This comment has been minimized.

@RaenonX

This comment has been minimized.

@Pierre-Sassoulas Pierre-Sassoulas added Needs PR This issue is accepted, sufficiently specified and now needs an implementation Help wanted 🙏 Outside help would be appreciated, good for new contributors labels Jul 4, 2022
@emmeowzing
Copy link

Still seeing this

@clavedeluna
Copy link
Collaborator

I'll work on this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component False Negative 🦋 No message is emitted but something is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants