From 7146658dca7acf1cce76fbf6b794ebacb2c6a583 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 17 Jan 2025 08:44:28 +0100 Subject: [PATCH] Fix instable f-string formatting for expressions containing a trailing comma --- .../test/fixtures/ruff/expression/fstring.py | 5 +++++ crates/ruff_python_formatter/src/builders.rs | 14 ++++++++++++++ crates/ruff_python_formatter/src/context.rs | 11 +++++++++++ .../snapshots/format@expression__fstring.py.snap | 15 +++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py index b90da551b0d3ee..d3a19d24e82389 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py @@ -719,6 +719,7 @@ print(f"{1, 2, {3} }") print(f"{(1, 2, {3})}") + # Regression tests for https://github.com/astral-sh/ruff/issues/15535 print(f"{ {}, }") # A single item tuple gets parenthesized print(f"{ {}.values(), }") @@ -726,3 +727,7 @@ print(f"{ # Tuple with multiple elements that doesn't fit on a single line gets parenthesized {}, 1, }") + + +# Regression tests for https://github.com/astral-sh/ruff/issues/15536 +print(f"{ {}, 1, }") diff --git a/crates/ruff_python_formatter/src/builders.rs b/crates/ruff_python_formatter/src/builders.rs index 45b74ed8b5b637..8c50d70c22cb90 100644 --- a/crates/ruff_python_formatter/src/builders.rs +++ b/crates/ruff_python_formatter/src/builders.rs @@ -206,6 +206,20 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> { pub(crate) fn finish(&mut self) -> FormatResult<()> { self.result.and_then(|()| { + // Don't add a magic trailing comma when formatting an f-string expression + // that always must be flat because the `expand_parent` forces enclosing + // groups to expand, e.g. `print(f"{(a,)} ")` would format the f-string in + // flat mode but the `print` call gets expanded because of the `expand_parent`. + if self + .fmt + .context() + .f_string_state() + .can_contain_line_breaks() + == Some(false) + { + return Ok(()); + } + if let Some(last_end) = self.entries.position() { let magic_trailing_comma = has_magic_trailing_comma( TextRange::new(last_end, self.sequence_end), diff --git a/crates/ruff_python_formatter/src/context.rs b/crates/ruff_python_formatter/src/context.rs index e6b7830cb712d5..c1293758019cd2 100644 --- a/crates/ruff_python_formatter/src/context.rs +++ b/crates/ruff_python_formatter/src/context.rs @@ -147,6 +147,17 @@ pub(crate) enum FStringState { Outside, } +impl FStringState { + pub(crate) fn can_contain_line_breaks(self) -> Option { + match self { + FStringState::InsideExpressionElement(context) => { + Some(context.can_contain_line_breaks()) + } + FStringState::Outside => None, + } + } +} + /// The position of a top-level statement in the module. #[derive(Copy, Clone, Debug, Eq, PartialEq, Default)] pub(crate) enum TopLevelStatementPosition { diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index e2d1a899a11ae6..fea634ff693ff9 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -725,6 +725,7 @@ print(f"{({1, 2, 3}) - ({2})}") print(f"{1, 2, {3} }") print(f"{(1, 2, {3})}") + # Regression tests for https://github.com/astral-sh/ruff/issues/15535 print(f"{ {}, }") # A single item tuple gets parenthesized print(f"{ {}.values(), }") @@ -732,6 +733,10 @@ print(f"{ {}, 1 }") # A tuple with multiple elements doesn't get parenthesized print(f"{ # Tuple with multiple elements that doesn't fit on a single line gets parenthesized {}, 1, }") + + +# Regression tests for https://github.com/astral-sh/ruff/issues/15536 +print(f"{ {}, 1, }") ``` ## Outputs @@ -1515,6 +1520,7 @@ print(f"{({1, 2, 3}) - ({2})}") print(f"{1, 2, {3}}") print(f"{(1, 2, {3})}") + # Regression tests for https://github.com/astral-sh/ruff/issues/15535 print(f"{({},)}") # A single item tuple gets parenthesized print(f"{({}.values(),)}") @@ -1527,6 +1533,10 @@ print( ) }" ) + + +# Regression tests for https://github.com/astral-sh/ruff/issues/15536 +print(f"{ {}, 1 }") ``` @@ -2310,6 +2320,7 @@ print(f"{({1, 2, 3}) - ({2})}") print(f"{1, 2, {3}}") print(f"{(1, 2, {3})}") + # Regression tests for https://github.com/astral-sh/ruff/issues/15535 print(f"{({},)}") # A single item tuple gets parenthesized print(f"{({}.values(),)}") @@ -2322,4 +2333,8 @@ print( ) }" ) + + +# Regression tests for https://github.com/astral-sh/ruff/issues/15536 +print(f"{ {}, 1 }") ```