Keep nested parens around lambda/ternary comprehension iterables (preview)#5200
Keep nested parens around lambda/ternary comprehension iterables (preview)#5200sarathfrancis90 wants to merge 2 commits into
Conversation
…view) `wrap_comprehension_in` keeps the parentheses around a lambda or conditional expression used as a comprehension's iterable, otherwise the trailing `for`/`if` clauses get absorbed into it and the result is invalid. psf#5176 handled the single-paren case, but `maybe_make_parens_invisible_in_atom` recurses through the atom, so a redundantly nested pair like `[x for x in ((lambda: 0))]` still had *both* layers stripped, producing `[x for x in lambda: 0]` and crashing with an ASTSafetyError. Look through redundant nested parens before deciding whether the atom wraps a lambda or ternary, so at least one pair is kept.
e592afd to
00e1441
Compare
|
Thanks for this fix! Would it be non-trivial to continue stripping the outer parentheses, only leaving one pair always? If that'd be a lot of extra work, I'd be fine deferring that and merging the PR as-is, but I'd appreciate that change if possible. |
Follow-up to the previous commit: instead of keeping every nested pair of parentheses around a lambda or conditional expression used as a comprehension's iterable, reduce them down to a single pair (e.g. `[i for i in ((lambda: 0))]` becomes `[i for i in (lambda: 0)]`). One pair is still required so the trailing `for`/`if` clauses aren't absorbed, but the extra pairs are redundant, matching how Black strips fully-optional parens elsewhere. Let maybe_make_parens_invisible_in_atom do its normal collapse (reusing its comment handling) and then restore a single visible pair. Extended the fixture with the collapsed outputs and a triple-nested case.
|
Done! Extended it to collapse all the way down to a single pair, so |
2 participants
While poking at
wrap_comprehension_inafter #5176, I hit a case it doesn't cover. #5176 keeps the parens around a lambda or ternary used as a comprehension's iterable so the trailingfor/ifclauses aren't absorbed into it. But when those parens are redundantly nested, e.g.maybe_make_parens_invisible_in_atomrecurses and strips both pairs, leaving[x for x in lambda: 0]— invalid code, which crashes with anASTSafetyErrorat the default line length. Same for a nested ternary and for generator/dict/set comprehensions, and at deeper nesting.The guard only inspected the atom's direct child, so a paren-wrapped atom slipped past it. I made it look through the redundant nested parens before deciding whether the iterable is a lambda/ternary, so at least one pair is kept (matching what the stable style does with such input).
Added the nested lambda/ternary cases to the existing
preview_wrap_comprehension_infixture — they crashed before and pass now. The full test suite, mypy, flake8 and the self-format check are green.