Skip to content

Commit

Permalink
Consider FString expression element format-spec when choosing quotes
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Nov 20, 2024
1 parent 3c52d2d commit f681076
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -319,3 +319,10 @@ def implicit_with_comment():
assert False, +"Implicit concatenated string" "uses {} layout on {} format".format(
"Multiline", "first"
)


# https://github.com/astral-sh/ruff/issues/13935
"aaa" f'{1: hy "user"}'
"aaa 'b' " f'{1:hy "user"}'
"aaa " f'{1: abcd "{1}" }'
"aaa " f'{1: abcd "{'aa'}" }'
84 changes: 53 additions & 31 deletions crates/ruff_python_formatter/src/string/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use std::iter::FusedIterator;
use ruff_formatter::FormatContext;
use ruff_python_ast::visitor::source_order::SourceOrderVisitor;
use ruff_python_ast::{
str::Quote, AnyStringFlags, BytesLiteral, FString, StringFlags, StringLikePart, StringLiteral,
str::Quote, AnyStringFlags, BytesLiteral, FString, FStringElement, FStringElements,
FStringFlags, StringFlags, StringLikePart, StringLiteral,
};
use ruff_text_size::{Ranged, TextRange, TextSlice};

Expand Down Expand Up @@ -262,37 +263,14 @@ impl QuoteMetadata {
}
StringLikePart::FString(fstring) => {
if is_f_string_formatting_enabled(context) {
// For f-strings, only consider the quotes inside string-literals but ignore
// quotes inside expressions. This allows both the outer and the nested literals
// to make the optimal local-choice to reduce the total number of quotes necessary.
// This doesn't require any pre 312 special handling because an expression
// can never contain the outer quote character, not even escaped:
// ```python
// f"{'escaping a quote like this \" is a syntax error pre 312'}"
// ```
let mut literals = fstring.elements.literals();

let Some(first) = literals.next() else {
return QuoteMetadata::from_str("", part.flags(), preferred_quote);
};

let mut metadata = QuoteMetadata::from_str(
context.source().slice(first),
fstring.flags.into(),
preferred_quote,
);

for literal in literals {
metadata = metadata
.merge(&QuoteMetadata::from_str(
context.source().slice(literal),
fstring.flags.into(),
preferred_quote,
))
.expect("Merge to succeed because all parts have the same flags");
}
let metadata = QuoteMetadata::from_str("", part.flags(), preferred_quote);

metadata
metadata.merge_fstring_elements(
&fstring.elements,
fstring.flags,
context,
preferred_quote,
)
} else {
let text = &context.source()[part.content_range()];

Expand Down Expand Up @@ -398,6 +376,50 @@ impl QuoteMetadata {
source_style: self.source_style,
})
}

/// For f-strings, only consider the quotes inside string-literals but ignore
/// quotes inside expressions (except inside the format spec). This allows both the outer and the nested literals
/// to make the optimal local-choice to reduce the total number of quotes necessary.
/// This doesn't require any pre 312 special handling because an expression
/// can never contain the outer quote character, not even escaped:
/// ```python
/// f"{'escaping a quote like this \" is a syntax error pre 312'}"
/// ```
fn merge_fstring_elements(
self,
elements: &FStringElements,
flags: FStringFlags,
context: &PyFormatContext,
preferred_quote: Quote,
) -> Self {
let mut merged = self;

for element in elements {
match element {
FStringElement::Literal(literal) => {
merged = merged
.merge(&QuoteMetadata::from_str(
context.source().slice(literal),
flags.into(),
preferred_quote,
))
.expect("Merge to succeed because all parts have the same flags");
}
FStringElement::Expression(expression) => {
if let Some(spec) = expression.format_spec.as_deref() {
merged = merged.merge_fstring_elements(
&spec.elements,
flags,
context,
preferred_quote,
);
}
}
}
}

merged
}
}

#[derive(Copy, Clone, Debug)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,13 @@ assert False, "Implicit concatenated stringuses {} layout on {} format"[
assert False, +"Implicit concatenated string" "uses {} layout on {} format".format(
"Multiline", "first"
)
# https://github.com/astral-sh/ruff/issues/13935
"aaa" f'{1: hy "user"}'
"aaa 'b' " f'{1:hy "user"}'
"aaa " f'{1: abcd "{1}" }'
"aaa " f'{1: abcd "{'aa'}" }'
```
## Outputs
Expand Down Expand Up @@ -733,4 +740,11 @@ assert False, "Implicit concatenated stringuses {} layout on {} format"[
assert False, +"Implicit concatenated stringuses {} layout on {} format".format(
"Multiline", "first"
)
# https://github.com/astral-sh/ruff/issues/13935
f'aaa{1: hy "user"}'
f"aaa 'b' {1:hy \"user\"}"
f'aaa {1: abcd "{1}" }'
f'aaa {1: abcd "{"aa"}" }'
```

0 comments on commit f681076

Please sign in to comment.