adamchainz / flake8-comprehensions

❄️ A flake8 plugin to help you write better list/set/dict comprehensions.
MIT License
465 stars 23 forks source link

Misdetects non-generator parentheses #591

Closed dfandrich closed 3 months ago

dfandrich commented 3 months ago

Python Version

3.10.11

flake8 Version

7.1.1

Package Version

3.15.0

Description

The following Python code:

l = (['abc']
     + [x for x in ['xyz']])

results in flake8 complaining with tst.py:2:8: C416 Unnecessary list comprehension - rewrite using list().. Rewriting the code as as suggested:

l = (['abc']
     + list(x for x in ['xyz']))

instead results in the complaint tst.py:2:8: C400 Unnecessary generator - rewrite as a list comprehension. This is a catch-22: there is no way to silence this error (without resorting to noqa).

I suspect the parser sees the parenthesis, gets confused, and assumes it's introducing a generator, when in fact it's a simple parenthesis grouping in order to split the compound statement over two lines.

adamchainz commented 3 months ago

Thanks for the clear issue. There is no parser mistake - Flake8 plugins generally just look at the abstract syntax tree, generated by Python’s own parser, and that’s the case for flake8-comprehensions here.

Instead, I think you’ve misunderstood what the rule is asking you to do. Per the C416 documentation, the fix for your example would be:

l = (['abc']
     + list(['xyz']))

or more simply:

l = (['abc']
     + ['xyz'])

[x for x in ['xyz']] does nothing but deconstruct and reconstruct a list with a single 'xyz' element:

>>> [x for x in ['xyz']]
['xyz']

Hope that helps!

dfandrich commented 3 months ago

The example was a simplified version of the original code which used a variable (not a static one-element list), but you're right—I completely glossed over the fact that the whole list comprehension was unnecessary and could be replaced by the list itself. Thanks for the pointer!