Skip to content

Commit

Permalink
Only add spaces if the left-most sub-expression uses curly parentheses
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Jan 15, 2025
1 parent 9ab3221 commit 7315f62
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 106 deletions.
90 changes: 1 addition & 89 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ 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, SimpleTokenKind, SimpleTokenizer};
use ruff_python_trivia::CommentRanges;
use ruff_text_size::Ranged;

use crate::builders::parenthesize_if_expands;
Expand Down Expand Up @@ -1311,91 +1311,3 @@ pub(crate) fn left_most<'expr>(
current = left;
}
}

/// Returns the sub-expression to which the right-most character in expression belongs.
///
/// For example, in the expression `a + b * c`, the right-most subexpression is `c`. But for
/// the expression `{ "a": 1 }`, the right-most subexpression is the dictionary, and not `"1"` because
/// the `{` belongs to the dictionary.
///
/// Parenthesized expressions are treated as belonging to the enclosing expression. Therefore, the right-most
/// sub expression for `c * (a + b)` is `a + b` and not `b`.
pub(crate) fn right_most<'expr>(
expression: &'expr Expr,
comment_ranges: &CommentRanges,
source: &str,
) -> &'expr Expr {
let mut current = expression;
loop {
let right = match current {
Expr::BoolOp(expr_bool_op) => expr_bool_op.values.last(),
Expr::Compare(compare) => compare.comparators.last(),

Expr::Generator(generator) if !generator.parenthesized => {
generator.generators.last().map(|last_comprehension| {
last_comprehension
.ifs
.last()
.unwrap_or(&last_comprehension.iter)
})
}
Expr::Tuple(tuple) if !tuple.parenthesized => tuple.elts.last(),
Expr::Slice(slice) => {
if SimpleTokenizer::new(source, slice.range())
.skip_trivia()
.last()
.is_some_and(|last| last.kind() == SimpleTokenKind::Colon)
{
None
} else {
slice
.step
.as_deref()
.or(slice.upper.as_deref())
.or(slice.lower.as_deref())
}
}
Expr::Starred(ast::ExprStarred { value: right, .. })
| Expr::UnaryOp(ast::ExprUnaryOp { operand: right, .. })
| Expr::Lambda(ast::ExprLambda { body: right, .. })
| Expr::Await(ast::ExprAwait { value: right, .. })
| Expr::YieldFrom(ast::ExprYieldFrom { value: right, .. })
| Expr::If(ast::ExprIf { orelse: right, .. })
| Expr::BinOp(ast::ExprBinOp { right, .. }) => Some(&**right),

Expr::Yield(ast::ExprYield { value, .. }) => value.as_deref(),

Expr::Attribute(_)
| Expr::Call(_)
| Expr::List(_)
| Expr::Tuple(_)
| Expr::Name(_)
| Expr::FString(_)
| Expr::StringLiteral(_)
| Expr::BytesLiteral(_)
| Expr::NumberLiteral(_)
| Expr::BooleanLiteral(_)
| Expr::NoneLiteral(_)
| Expr::EllipsisLiteral(_)
| Expr::DictComp(_)
| Expr::SetComp(_)
| Expr::ListComp(_)
| Expr::Set(_)
| Expr::Dict(_)
| Expr::Named(_)
| Expr::IpyEscapeCommand(_)
| Expr::Generator(_)
| Expr::Subscript(_) => None,
};

let Some(right) = right else {
break current;
};

if is_expression_parenthesized(right.into(), comment_ranges, source) {
break current;
}

current = right;
}
}
26 changes: 11 additions & 15 deletions crates/ruff_python_formatter/src/other/f_string_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +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, right_most};
use crate::expression::left_most;
use crate::prelude::*;
use crate::string::normalize_string;
use crate::verbatim::verbatim_text;
Expand Down Expand Up @@ -203,20 +203,16 @@ impl Format<PyFormatContext<'_>> for FormatFStringExpressionElement<'_> {
// 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 = if has_curly_parentheses(left_most(
expression,
comments.ranges(),
f.context().source(),
)) || has_curly_parentheses(right_most(
expression,
comments.ranges(),
f.context().source(),
)) {
Some(format_with(|f| {
if self.context.can_contain_line_breaks() {
soft_line_break_or_space().fmt(f)
} else {
space().fmt(f)
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() {
soft_line_break_or_space().fmt(f)
} else {
space().fmt(f)
}
}))
}
_ => None,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1504,7 +1504,7 @@ f'{1=: {'ab"cd"'}}' # It's okay if the quotes are in an expression part.
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}}")
print(f"{(1, 2, {3})}")
```
Expand Down Expand Up @@ -2286,6 +2286,6 @@ f'{1=: {'ab"cd"'}}' # It's okay if the quotes are in an expression part.
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}}")
print(f"{(1, 2, {3})}")
```

0 comments on commit 7315f62

Please sign in to comment.