Interesting, that Black has a custom logic especially for generators.

Can you add an example with a GeneratorExpr that also has comments?

len(
	( # leading
	a for b in c
	 # trailing
	)
)

Nit: Let's move the parentheses check from line 53..59 into the other => branch. We don't need to test if the argument is parenthesized if we're going to remove the parentheses anyway

this isn't based on black but rather the rules for generator expressions. see e.g. "details, §2b" in the pep https://peps.python.org/pep-0289/#the-details

note that your example will differ to black, in my understanding due to the same black bug that keeps _existing parens

note that your example will differ to black, in my understanding due to the same black bug that keeps _existing parens

Black's behavior is to always preserve parentheses in nested expressions and I think we should do the same for generators, except if we have very good reasons not to (for consistency).

I think the idiomatic way to implement this then is to change the generator's NeedsParentheses implementation to return Never if it is the only argument, and Always otherwise (except there are other places where the parentheses should be omitted).

note that your example will differ to black, in my understanding due to the same black bug that keeps _existing parens

Black's behavior is to always preserve parentheses in nested expressions and I think we should do the same for generators, except if we have very good reasons not to (for consistency).

is this a reference to https://github.com/astral-sh/ruff/pull/5804/files#diff-dba753754ca8da36599acf528a6bfe006404be2c4cd9d00f18db1a3cf1a7ec21R9
or https://github.com/astral-sh/ruff/pull/5804/files#diff-dba753754ca8da36599acf528a6bfe006404be2c4cd9d00f18db1a3cf1a7ec21R19
?

I think the idiomatic way to implement this then is to change the generator's NeedsParentheses implementation to return Never if it is the only argument, and Always otherwise (except there are other places where the parentheses should be omitted).

how does NeedsParentheses figure out if it's a no-sibling function argument?

ok. i'm afraid i don't quite understand your fix suggestion. atm NeedsParentheses (afict) is only called if there is a maybe_parenthesize_expression wrapper. is that right? if so, where are you suggesting that should go? apologies if i'm missing something obvious here

The proposal with NeedsParentheses is mainly if we want to match Black's behavior:

• Because, by default, expressions preserve their parentheses. Meaning the generators would have needed parentheses already before. Or are you aware of situations where a generator is not parenthesized in the source, but would need to be parenthesized after formatting (e.g. because we insert a trailing comma)?
• Overriding NeedsParentheses and returning Always will add/preserve parentheses in the places where we, otherwise would omit them (e.g. in an if header).

The reason why I think that NeedsParentheses would be a good fit is, because it should inform the formatted when it is necessary to parenthesize an expression. Tuples are different, because they have their own parentheses (you can have a parenthesized tuple expression ((a, b)), but (a, b) is a tuple expression with parentheses. Okay this sounds confusing. One is about it is a parenthesized expression, the other is that tuples have their own parentheses)

maybe i'm misunderstanding you, but in my understanding of generator expressions, they do have their "own parentheses" just like tuples do (but they can be omitted in some contexts. (fewer contexts than for tuples, only if they are a sole call argument))

maybe we merge this without preserving "double wrapped" generators and see how common when we compare ourselves against black on the various code bases we test. in general i'm in favour of matching black closer if that helps adoption

Oh I'm sorry. That's my bad. I assumed that parenthesizing the generator is optional. But from Python's full grammar:

genexp:
    | '(' ( assignment_expression | expression !':=') for_if_clauses ')'

https://docs.python.org/3/reference/grammar.html

(Oddly, the parens are not optional)

i wonder if the function call case is covered here

primary:
    | primary '.' NAME 
    | primary genexp                     <--  generator expr instead of brackets and arguments
    | primary '(' [arguments] ')'      <-- (regular function call)