From 96c2d0996db49179f0caac2806fb87b5f37869c3 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 15 Jan 2025 09:22:47 +0100 Subject: [PATCH] Fix curly bracket spacing around curly f-string expressions (#15471) --- .../test/fixtures/ruff/expression/fstring.py | 8 +++ .../src/expression/mod.rs | 70 ++++++++++++++++++- .../src/other/f_string_element.rs | 30 ++++---- .../format@expression__fstring.py.snap | 24 +++++++ 4 files changed, 117 insertions(+), 15 deletions(-) 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 1b0d11ce33..4ef0024380 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 @@ -710,3 +710,11 @@ f'{x:a{y=:{z:hy "user"}}} \'\'\'' f"""{1=: "this" is fine}""" f'''{1=: "this" is fine}''' # Change quotes to double quotes because they're preferred f'{1=: {'ab"cd"'}}' # It's okay if the quotes are in an expression part. + + +# Regression tests for https://github.com/astral-sh/ruff/issues/15459 +print(f"{ {1, 2, 3} - {2} }") +print(f"{ {1: 2}.keys() }") +print(f"{({1, 2, 3}) - ({2})}") +print(f"{1, 2, {3} }") +print(f"{(1, 2, {3})}") diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index 574aee804f..6d302401e4 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -4,9 +4,9 @@ use std::slice; use ruff_formatter::{ write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions, }; -use ruff_python_ast as ast; use ruff_python_ast::parenthesize::parentheses_iterator; use ruff_python_ast::visitor::source_order::{walk_expr, SourceOrderVisitor}; +use ruff_python_ast::{self as ast}; use ruff_python_ast::{AnyNodeRef, Expr, ExpressionRef, Operator}; use ruff_python_trivia::CommentRanges; use ruff_text_size::Ranged; @@ -1243,3 +1243,71 @@ pub(crate) fn is_splittable_expression(expr: &Expr, context: &PyFormatContext) - } } } + +/// Returns the sub-expression to which the left-most character in expression belongs. +/// +/// For example, in the expression `a + b * c`, the left-most subexpression is `a`. But for +/// the expression `{ "a": 1 }`, the left-most subexpression is the dictionary, and not `"a"` because +/// the `{` belongs to the dictionary. +/// +/// Parenthesized expressions are treated as belonging to the enclosing expression. Therefore, the left +/// most expression for `(a + b) * c` is `a + b` and not `a`. +pub(crate) fn left_most<'expr>( + expression: &'expr Expr, + comment_ranges: &CommentRanges, + source: &str, +) -> &'expr Expr { + let mut current = expression; + loop { + let left = match current { + Expr::BinOp(ast::ExprBinOp { left, .. }) + | Expr::If(ast::ExprIf { body: left, .. }) + | Expr::Call(ast::ExprCall { func: left, .. }) + | Expr::Attribute(ast::ExprAttribute { value: left, .. }) + | Expr::Subscript(ast::ExprSubscript { value: left, .. }) => Some(&**left), + + Expr::BoolOp(expr_bool_op) => expr_bool_op.values.first(), + Expr::Compare(compare) => Some(&*compare.left), + + Expr::Generator(generator) if !generator.parenthesized => Some(&*generator.elt), + + Expr::Tuple(tuple) if !tuple.parenthesized => tuple.elts.first(), + Expr::Slice(slice) => slice.lower.as_deref(), + + Expr::List(_) + | Expr::Tuple(_) + | Expr::Name(_) + | Expr::Starred(_) + | Expr::FString(_) + | Expr::StringLiteral(_) + | Expr::BytesLiteral(_) + | Expr::NumberLiteral(_) + | Expr::BooleanLiteral(_) + | Expr::NoneLiteral(_) + | Expr::EllipsisLiteral(_) + | Expr::Yield(_) + | Expr::YieldFrom(_) + | Expr::Await(_) + | Expr::DictComp(_) + | Expr::SetComp(_) + | Expr::ListComp(_) + | Expr::Set(_) + | Expr::Dict(_) + | Expr::UnaryOp(_) + | Expr::Lambda(_) + | Expr::Named(_) + | Expr::IpyEscapeCommand(_) + | Expr::Generator(_) => None, + }; + + let Some(left) = left else { + break current; + }; + + if is_expression_parenthesized(left.into(), comment_ranges, source) { + break current; + } + + current = left; + } +} diff --git a/crates/ruff_python_formatter/src/other/f_string_element.rs b/crates/ruff_python_formatter/src/other/f_string_element.rs index 6fe59c0d12..073aad2160 100644 --- a/crates/ruff_python_formatter/src/other/f_string_element.rs +++ b/crates/ruff_python_formatter/src/other/f_string_element.rs @@ -9,6 +9,7 @@ use ruff_text_size::{Ranged, TextSlice}; use crate::comments::{dangling_open_parenthesis_comments, trailing_comments}; use crate::context::{FStringState, NodeLevel, WithFStringState, WithNodeLevel}; +use crate::expression::left_most; use crate::prelude::*; use crate::string::normalize_string; use crate::verbatim::verbatim_text; @@ -190,20 +191,20 @@ impl Format> for FormatFStringExpressionElement<'_> { let comments = f.context().comments().clone(); let dangling_item_comments = comments.dangling(self.element); - let item = format_with(|f| { - let bracket_spacing = match expression.as_ref() { - // If an expression starts with a `{`, we need to add a space before the - // curly brace to avoid turning it into a literal curly with `{{`. - // - // For example, - // ```python - // f"{ {'x': 1, 'y': 2} }" - // # ^ ^ - // ``` - // - // We need to preserve the space highlighted by `^`. The whitespace - // before the closing curly brace is not strictly necessary, but it's - // added to maintain consistency. + // If an expression starts with a `{`, we need to add a space before the + // curly brace to avoid turning it into a literal curly with `{{`. + // + // For example, + // ```python + // f"{ {'x': 1, 'y': 2} }" + // # ^ ^ + // ``` + // + // We need to preserve the space highlighted by `^`. The whitespace + // before the closing curly brace is not strictly necessary, but it's + // added to maintain consistency. + let bracket_spacing = + match left_most(expression, comments.ranges(), f.context().source()) { Expr::Dict(_) | Expr::DictComp(_) | Expr::Set(_) | Expr::SetComp(_) => { Some(format_with(|f| { if self.context.can_contain_line_breaks() { @@ -216,6 +217,7 @@ impl Format> for FormatFStringExpressionElement<'_> { _ => None, }; + let item = format_with(|f: &mut PyFormatter| { // Update the context to be inside the f-string expression element. let f = &mut WithFStringState::new( FStringState::InsideExpressionElement(self.context), 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 f8020fc193..cdcc2fba8e 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 @@ -716,6 +716,14 @@ f'{x:a{y=:{z:hy "user"}}} \'\'\'' f"""{1=: "this" is fine}""" f'''{1=: "this" is fine}''' # Change quotes to double quotes because they're preferred f'{1=: {'ab"cd"'}}' # It's okay if the quotes are in an expression part. + + +# Regression tests for https://github.com/astral-sh/ruff/issues/15459 +print(f"{ {1, 2, 3} - {2} }") +print(f"{ {1: 2}.keys() }") +print(f"{({1, 2, 3}) - ({2})}") +print(f"{1, 2, {3} }") +print(f"{(1, 2, {3})}") ``` ## Outputs @@ -1490,6 +1498,14 @@ f'{x:a{y=:{z:hy "user"}}} \'\'\'' f"""{1=: "this" is fine}""" f"""{1=: "this" is fine}""" # Change quotes to double quotes because they're preferred f'{1=: {'ab"cd"'}}' # It's okay if the quotes are in an expression part. + + +# Regression tests for https://github.com/astral-sh/ruff/issues/15459 +print(f"{ {1, 2, 3} - {2} }") +print(f"{ {1: 2}.keys() }") +print(f"{({1, 2, 3}) - ({2})}") +print(f"{1, 2, {3}}") +print(f"{(1, 2, {3})}") ``` @@ -2264,4 +2280,12 @@ f'{x:a{y=:{z:hy "user"}}} \'\'\'' f"""{1=: "this" is fine}""" f"""{1=: "this" is fine}""" # Change quotes to double quotes because they're preferred f'{1=: {'ab"cd"'}}' # It's okay if the quotes are in an expression part. + + +# Regression tests for https://github.com/astral-sh/ruff/issues/15459 +print(f"{ {1, 2, 3} - {2} }") +print(f"{ {1: 2}.keys() }") +print(f"{({1, 2, 3}) - ({2})}") +print(f"{1, 2, {3}}") +print(f"{(1, 2, {3})}") ```