[pylint] Fix missing parens in unsafe fix for unnecessary-dunder-call (PLC2801)

[mishamsk]

Summary

This fixes #15745

Also, I haven't found a trait/helper to get expression precedence. Even though at least 3 rules check precedence when generating fixes. Generally speaking, unless/until fixes are done via AST generation - many (all?) fixes involving expression rewrites would need to consult precedence to handle parens. Unless I am mistaken.

Thus, I've implemented an enum and a trait. But I didn't dare doing that in semantic/AST crates, although I believe it would be helpful to move them there.

@AlexWaygood @dylwil3 you'll know the codebase better, let me know if you think it should be elevated to some core base crate. If you do, I am happy to implement this, but will need your guidance on where you'd like it to reside.

Test Plan

• Updated test case with 2 examples from the issue + a couple of mine, including one that would generate invalid python program before this fix
• cargo nextest run (check updated snapshot)

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[MichaReiser]

Thanks. This overall looks good. Extracting a common OperatorPrecedence enum into the ast crate seems like a good idea because this PR now adds the third version of OperatorPrecedence enum but I suggest that we do this as a follow up PR. For now, I recommend aligning the enum and variant names with the other OperatorPrecedence enums

[MichaReiser]

I'd avoid a custom trait here and instead implement From<&Expr> for ExprPrecedence, as well as From, FromandFrom`

[mishamsk]

I thought about it. The reason I went a custom trait route is that From either encourages to either use into() which is less readable outside of IDE (when types are not apparent), or would force writing ExprPrecedence::from(some_node) which is readable but longer. I expect compiler to emit exact same code for both.

Interestingly enough, both parser and formatter code include the same method for one specific expression too

ruff/crates/ruff_python_parser/src/parser/expression.rs

Lines 2427 to 2435 in 138e70b

	/// Returns the [`OperatorPrecedence`] for the given operator token or [None] if the token
	/// isn't an operator token.
	fn precedence(&self) -> OperatorPrecedence {
	match self {
	BinaryLikeOperator::Boolean(bool_op) => OperatorPrecedence::from(*bool_op),
	BinaryLikeOperator::Comparison(_) => OperatorPrecedence::ComparisonsMembershipIdentity,
	BinaryLikeOperator::Binary(bin_op) => OperatorPrecedence::from(*bin_op),
	}
	}

but rely on OperatorPrecedence::from(some_node) in all other cases.

I am not married to the custom trait idea, if you have a strong opinion here - I can easily change that. Either in this PR, or as a follow-up while moving the whole thing to AST module.

[MichaReiser]

I removed the trait for now as it isn't needed when we move to the ruff_python_ast crate (because we can directly implement it on Expr).

I also renamed the enum to OperatorPrecedence to better match what we have in other crates.

[MichaReiser]

Could we align the names (enum and variants) with what we have in the formatter and parser. It should simplify unifying the enums long-term

[mishamsk]

First of all, I do not think that having even 3 enums is necessary bad. At least two separate ones make sense:

• Formatter enum is indeed an operator precedence one, as it is purpose is to split operators
• The one I've created and the parser one are both expression precedence and I would keep that naming.
  • But unifying the parser one would go against @dhruvmanila comment, that explicitly excludes some expressions. I didn't investigate why, but I trust there were good reasons to do so

As for variant names - tbh. I like the shorter naming that aligns with Python docs better (formatter & mine), but since my enum logically closer to parser I changed names to mostly match parser version, where possible.

[dhruvmanila]

For the parser, the enum only contains precedence levels for the operators and not for all expressions. The main reason is that the parser utilizes the Pratt parsing algorithm to parse binary expressions which is the main use case for this enum. While, the other expressions are parsed normally and implemented as separate methods. But, that doesn't necessarily mean that it can't be unified to include other variants as well. We'll just have to be careful that the parser starts the binary expression parsing from the correct precedence level.