From 8156b451736224ecf773ca9748ede010417f25ee Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 17 Nov 2025 09:11:36 -0600 Subject: [PATCH] Avoid syntax error when formatting attribute expressions with outer parentheses, parenthesized value, and trailing comment on value (#20418) Closes #19350 This fixes a syntax error caused by formatting. However, the new tests reveal that there are some cases where formatting attributes with certain comments behaves strangely, both before and after this PR, so some more polish may be in order. For example, without parentheses around the value, and both before and after this PR, we have: ```python # unformatted variable = ( something # a comment .first_method("some string") ) # formatted variable = something.first_method("some string") # a comment ``` which is probably not where the comment ought to go. --- .../fixtures/ruff/expression/attribute.py | 50 +++++++++ .../src/expression/expr_attribute.rs | 17 ++- .../format@expression__attribute.py.snap | 100 ++++++++++++++++++ 3 files changed, 166 insertions(+), 1 deletion(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py index 6f5a7ac1a6..a760491b3d 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py @@ -169,3 +169,53 @@ result = ( # dangling before dot .b # trailing end-of-line ) + +# Regression test for https://github.com/astral-sh/ruff/issues/19350 +variable = ( + (something) # a comment + .first_method("some string") +) + +variable = ( + something # a commentdddddddddddddddddddddddddddddd + .first_method("some string") +) + +if ( + (something) # a commentdddddddddddddddddddddddddddddd + .first_method("some string") +): pass + +variable = ( + (something # a comment + ).first_method("some string") +) + +if ( + (something # a comment + ).first_method("some string") # second comment +): pass + +variable = ( # 1 + # 2 + (something) # 3 + # 4 + .first_method("some string") # 5 + # 6 +) # 7 + + +if ( + (something + # trailing own line on value + ) + .first_method("some string") +): ... + +variable = ( + (something + # 1 + ) # 2 + .first_method("some string") +) + diff --git a/crates/ruff_python_formatter/src/expression/expr_attribute.rs b/crates/ruff_python_formatter/src/expression/expr_attribute.rs index ebb0093ccb..781b8b4601 100644 --- a/crates/ruff_python_formatter/src/expression/expr_attribute.rs +++ b/crates/ruff_python_formatter/src/expression/expr_attribute.rs @@ -179,7 +179,22 @@ impl NeedsParentheses for ExprAttribute { context.comments().ranges(), context.source(), ) { - OptionalParentheses::Never + // We have to avoid creating syntax errors like + // ```python + // variable = (something) # trailing + // .my_attribute + // ``` + // See https://github.com/astral-sh/ruff/issues/19350 + if context + .comments() + .trailing(self.value.as_ref()) + .iter() + .any(|comment| comment.line_position().is_end_of_line()) + { + OptionalParentheses::Multiline + } else { + OptionalParentheses::Never + } } else { self.value.needs_parentheses(self.into(), context) } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap index 89ce7d237b..4677ca06de 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap @@ -175,6 +175,56 @@ result = ( # dangling before dot .b # trailing end-of-line ) + +# Regression test for https://github.com/astral-sh/ruff/issues/19350 +variable = ( + (something) # a comment + .first_method("some string") +) + +variable = ( + something # a commentdddddddddddddddddddddddddddddd + .first_method("some string") +) + +if ( + (something) # a commentdddddddddddddddddddddddddddddd + .first_method("some string") +): pass + +variable = ( + (something # a comment + ).first_method("some string") +) + +if ( + (something # a comment + ).first_method("some string") # second comment +): pass + +variable = ( # 1 + # 2 + (something) # 3 + # 4 + .first_method("some string") # 5 + # 6 +) # 7 + + +if ( + (something + # trailing own line on value + ) + .first_method("some string") +): ... + +variable = ( + (something + # 1 + ) # 2 + .first_method("some string") +) + ``` ## Output @@ -328,4 +378,54 @@ result = ( # dangling before dot .b # trailing end-of-line ) + +# Regression test for https://github.com/astral-sh/ruff/issues/19350 +variable = ( + (something) # a comment + .first_method("some string") +) + +variable = something.first_method( # a commentdddddddddddddddddddddddddddddd + "some string" +) + +if ( + (something) # a commentdddddddddddddddddddddddddddddd + .first_method("some string") +): + pass + +variable = ( + something # a comment +).first_method("some string") + +if ( + ( + something # a comment + ).first_method("some string") # second comment +): + pass + +variable = ( # 1 + # 2 + (something) # 3 + # 4 + .first_method("some string") # 5 + # 6 +) # 7 + + +if ( + something + # trailing own line on value +).first_method("some string"): + ... + +variable = ( + ( + something + # 1 + ) # 2 + .first_method("some string") +) ```