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

Incorrectly identifying parens as superfluous in comprehensions with ternary operators #3608

Closed
MarkCBell opened this issue May 9, 2020 · 4 comments · Fixed by #4784
Closed
Labels
Milestone

Comments

@MarkCBell
Copy link
Contributor

Pylint incorrectly identifies parens as superfluous when they are used to isolate a ternary operator that determines the iterable inside of a list comprehension (or set comprehension, dict comprehension etc.).

Steps to reproduce

  1. Create a file called scratch.py with contents:
""" A test script. """

print([i**2 for i in (range(10) if 7 > 5 else range(3))])
  1. Evaluate this using the latest version of pylint:
    $ pylint scratch.py --enable=all

Current behavior

Pylint reports that there is one issue with this code:
scratch.py:3:0: C0325: Unnecessary parens after 'in' keyword (superfluous-parens)

Expected behavior

Pylint should report no issues with this code since the parenthesis are required. Following Pylints suggestion and changing the file to:

""" A test script. """

print([i**2 for i in range(10) if 7 > 5 else range(3)])

results in invalid Python code:

$ python scratch.py
  File "scratch.py", line 3
    print([i**2 for i in range(10) if 7 > 5 else range(3)])
                                            ^
SyntaxError: invalid syntax

pylint --version output

$ pylint --version
pylint 2.5.2
astroid 2.4.1
Python 3.8.1 (default, Mar  7 2020, 12:46:28)
[Clang 10.0.0 (clang-1000.10.44.4)]
@Pierre-Sassoulas
Copy link
Member

Thank you for creating the issue, I can reproduce it on the latest pylint.

@brycepg
Copy link
Contributor

brycepg commented Jun 17, 2020

I bisected this and I think it's from a combination of 716bcc4 and
a6d7ffc . The fix is non obvious because both those commits fixed other issues

@MarkCBell
Copy link
Contributor Author

@brycepg so I'm not sure that those are the source of this bug. I think the error is triggered by this line https://github.com/PyCQA/pylint/blob/master/pylint/checkers/format.py#L421. It looks to me like there needs to be another block here https://github.com/PyCQA/pylint/blob/master/pylint/checkers/format.py#L436-L441 to handle the case where parens are required around a conditional statement. To ensure that statements like X = (5 if 5 > 7 else 1) are still flagged as superfluous this will likely require another found_if_else flag that is set if a ternary operator is encountered after an in.

@brycepg
Copy link
Contributor

brycepg commented Jun 19, 2020

My bad I commented on the wrong issue

DanielNoord added a commit to DanielNoord/pylint that referenced this issue Aug 1, 2021
This fixes the false positives identified in pylint-dev#2818, pylint-dev#3249, pylint-dev#3608 & pylint-dev#4346
All false positives reported fell under keywords before walrus operator
or if-keyword within generators/comprehension.
This closes pylint-dev#2818, closes pylint-dev#3429, closes pylint-dev#3608, closes pylint-dev#4346
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.10.0 milestone Aug 1, 2021
DanielNoord added a commit to DanielNoord/pylint that referenced this issue Aug 1, 2021
This fixes the false positives identified in pylint-dev#2818, pylint-dev#3249, pylint-dev#3608 & pylint-dev#4346
All false positives reported fell under keywords before walrus operator
or if-keyword within generators/comprehension.
This closes pylint-dev#2818, closes pylint-dev#3429, closes pylint-dev#3608, closes pylint-dev#4346
DanielNoord added a commit to DanielNoord/pylint that referenced this issue Aug 1, 2021
This fixes the false positives identified in pylint-dev#2818, pylint-dev#3249, pylint-dev#3608 & pylint-dev#4346
All false positives reported fell under keywords before walrus operator
or if-keyword within generators/comprehension.
This closes pylint-dev#2818, closes pylint-dev#3429, closes pylint-dev#3608, closes pylint-dev#4346
Pierre-Sassoulas added a commit that referenced this issue Aug 3, 2021
* Split functional tests for ``superfluous-parents``

* Fix false positives for superfluous-parens
This fixes the false positives identified in #2818, #3249, #3608 & #4346
All false positives reported fell under keywords before walrus operator
or if-keyword within generators/comprehension.
This closes #2818, closes #3429, closes #3608, closes #4346

* Move the superfluous functional tests to functional/s/super

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants