Skip to content

Commit

Permalink
Fix assert formatting instability
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Oct 16, 2024
1 parent ae9c6a9 commit cf7b995
Show file tree
Hide file tree
Showing 15 changed files with 258 additions and 299 deletions.
3 changes: 2 additions & 1 deletion crates/ruff_python_formatter/src/other/f_string_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ impl<'a> FormatFStringLiteralElement<'a> {
impl Format<PyFormatContext<'_>> for FormatFStringLiteralElement<'_> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
let literal_content = f.context().locator().slice(self.element.range());
let normalized = normalize_string(literal_content, 0, self.context.flags(), true, false);
let normalized =
normalize_string(literal_content, 0, self.context.flags(), false, false, true);
match &normalized {
Cow::Borrowed(_) => source_text_slice(self.element.range()).fmt(f),
Cow::Owned(normalized) => text(normalized).fmt(f),
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_formatter/src/statement/stmt_assert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl FormatNodeRule<StmtAssert> for FormatStmtAssert {
[
token(","),
space(),
maybe_parenthesize_expression(msg, item, Parenthesize::IfBreaks),
maybe_parenthesize_expression(msg, item, Parenthesize::IfBreaksParenthesized),
]
)?;
}
Expand Down
Empty file.
4 changes: 3 additions & 1 deletion crates/ruff_python_formatter/src/string/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,17 @@ impl Format<PyFormatContext<'_>> for FormatImplicitConcatenatedString<'_> {
write!(f, [flags.prefix(), quotes])?;

// TODO: strings in expression statements aren't joined correctly because they aren't wrap in a group :(
// TODO: FStrings when the f-string preview style is enabled???

for part in self.string.parts() {
let content = f.context().locator().slice(part.content_range());
let normalized = normalize_string(
content,
0,
flags,
is_f_string_formatting_enabled(f.context()),
flags.is_f_string() && !part.flags().is_f_string(),
true,
false,
);
match normalized {
Cow::Borrowed(_) => source_text_slice(part.content_range()).fmt(f)?,
Expand Down
44 changes: 41 additions & 3 deletions crates/ruff_python_formatter/src/string/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,9 @@ impl<'a, 'src> StringNormalizer<'a, 'src> {
quote_selection.flags,
// TODO: Remove the `b'{'` in `choose_quotes` when promoting the
// `format_fstring` preview style
is_f_string_formatting_enabled(self.context),
false,
false,
is_f_string_formatting_enabled(self.context),
)
} else {
Cow::Borrowed(raw_content)
Expand Down Expand Up @@ -517,8 +518,9 @@ pub(crate) fn normalize_string(
input: &str,
start_offset: usize,
new_flags: AnyStringFlags,
format_f_string: bool,
escape_braces: bool,
flip_nested_fstring_quotes: bool,
format_f_string: bool,
) -> Cow<str> {
// The normalized string if `input` is not yet normalized.
// `output` must remain empty if `input` is already normalized.
Expand Down Expand Up @@ -626,6 +628,16 @@ pub(crate) fn normalize_string(
output.push(c);
last_index = index + preferred_quote.len_utf8();
}
// TODO: Don't normalize inner quotes if f-string formatting is enabled.
else if c == preferred_quote
&& flip_nested_fstring_quotes
&& formatted_value_nesting > 0
{
// Flip the quotes
output.push_str(&input[last_index..index]);
output.push(opposite_quote);
last_index = index + preferred_quote.len_utf8();
}
}
}

Expand Down Expand Up @@ -789,6 +801,7 @@ mod tests {

use super::UnicodeEscape;
use crate::string::normalize_string;
use ruff_python_ast::str_prefix::FStringPrefix;
use ruff_python_ast::{
str::Quote,
str_prefix::{AnyStringPrefix, ByteStringPrefix},
Expand Down Expand Up @@ -817,10 +830,35 @@ mod tests {
Quote::Double,
false,
),
true,
false,
false,
true,
);

assert_eq!(r"\x89\x50\x4e\x47\x0d\x0a\x1a\x0a", &normalized);
}

#[test]
fn normalize_nested_fstring() {
let input =
r#"With single quote: ' {my_dict['foo']} With double quote: " {my_dict["bar"]}"#;

let normalized = normalize_string(
input,
0,
AnyStringFlags::new(
AnyStringPrefix::Format(FStringPrefix::Regular),
Quote::Double,
false,
),
false,
true,
false,
);

assert_eq!(
"With single quote: ' {my_dict['foo']} With double quote: \\\" {my_dict['bar']}",
&normalized
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -193,41 +193,7 @@ class C:
```diff
--- Black
+++ Ruff
@@ -110,19 +110,20 @@
value, is_going_to_be="too long to fit in a single line", srsly=True
), "Not what we expected"
- assert {
- key1: value1,
- key2: value2,
- key3: value3,
- key4: value4,
- key5: value5,
- key6: value6,
- key7: value7,
- key8: value8,
- key9: value9,
- } == expected, (
- "Not what we expected and the message is too long to fit in one line"
- )
+ assert (
+ {
+ key1: value1,
+ key2: value2,
+ key3: value3,
+ key4: value4,
+ key5: value5,
+ key6: value6,
+ key7: value7,
+ key8: value8,
+ key9: value9,
+ }
+ == expected
+ ), "Not what we expected and the message is too long to fit in one line"
assert expected(
value, is_going_to_be="too long to fit in a single line", srsly=True
@@ -161,9 +162,7 @@
@@ -161,9 +161,7 @@
8 STORE_ATTR 0 (x)
10 LOAD_CONST 0 (None)
12 RETURN_VALUE
Expand Down Expand Up @@ -355,20 +321,19 @@ class C:
value, is_going_to_be="too long to fit in a single line", srsly=True
), "Not what we expected"
assert (
{
key1: value1,
key2: value2,
key3: value3,
key4: value4,
key5: value5,
key6: value6,
key7: value7,
key8: value8,
key9: value9,
}
== expected
), "Not what we expected and the message is too long to fit in one line"
assert {
key1: value1,
key2: value2,
key3: value3,
key4: value4,
key5: value5,
key6: value6,
key7: value7,
key8: value8,
key9: value9,
} == expected, (
"Not what we expected and the message is too long to fit in one line"
)
assert expected(
value, is_going_to_be="too long to fit in a single line", srsly=True
Expand Down Expand Up @@ -610,5 +575,3 @@ class C:
}
)
```


Original file line number Diff line number Diff line change
Expand Up @@ -193,41 +193,7 @@ class C:
```diff
--- Black
+++ Ruff
@@ -110,19 +110,20 @@
value, is_going_to_be="too long to fit in a single line", srsly=True
), "Not what we expected"
- assert {
- key1: value1,
- key2: value2,
- key3: value3,
- key4: value4,
- key5: value5,
- key6: value6,
- key7: value7,
- key8: value8,
- key9: value9,
- } == expected, (
- "Not what we expected and the message is too long to fit in one line"
- )
+ assert (
+ {
+ key1: value1,
+ key2: value2,
+ key3: value3,
+ key4: value4,
+ key5: value5,
+ key6: value6,
+ key7: value7,
+ key8: value8,
+ key9: value9,
+ }
+ == expected
+ ), "Not what we expected and the message is too long to fit in one line"
assert expected(
value, is_going_to_be="too long to fit in a single line", srsly=True
@@ -161,9 +162,7 @@
@@ -161,9 +161,7 @@
8 STORE_ATTR 0 (x)
10 LOAD_CONST 0 (None)
12 RETURN_VALUE
Expand Down Expand Up @@ -355,20 +321,19 @@ class C:
value, is_going_to_be="too long to fit in a single line", srsly=True
), "Not what we expected"
assert (
{
key1: value1,
key2: value2,
key3: value3,
key4: value4,
key5: value5,
key6: value6,
key7: value7,
key8: value8,
key9: value9,
}
== expected
), "Not what we expected and the message is too long to fit in one line"
assert {
key1: value1,
key2: value2,
key3: value3,
key4: value4,
key5: value5,
key6: value6,
key7: value7,
key8: value8,
key9: value9,
} == expected, (
"Not what we expected and the message is too long to fit in one line"
)
assert expected(
value, is_going_to_be="too long to fit in a single line", srsly=True
Expand Down Expand Up @@ -610,5 +575,3 @@ class C:
}
)
```


Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,19 @@ last_call()
) # note: no trailing comma pre-3.6
call(*gidgets[:2])
call(a, *gidgets[:2])
@@ -251,9 +251,9 @@
print(**{1: 3} if False else {x: x for x in range(3)})
print(*lambda x: x)
assert not Test, "Short message"
-assert this is ComplexTest and not requirements.fit_in_a_single_line(
- force=False
-), "Short message"
+assert this is ComplexTest and not requirements.fit_in_a_single_line(force=False), (
+ "Short message"
+)
assert parens is TooMany
for (x,) in (1,), (2,), (3,):
...
```
## Ruff Output
Expand Down Expand Up @@ -533,9 +546,9 @@ print(*[] or [1])
print(**{1: 3} if False else {x: x for x in range(3)})
print(*lambda x: x)
assert not Test, "Short message"
assert this is ComplexTest and not requirements.fit_in_a_single_line(
force=False
), "Short message"
assert this is ComplexTest and not requirements.fit_in_a_single_line(force=False), (
"Short message"
)
assert parens is TooMany
for (x,) in (1,), (2,), (3,):
...
Expand Down Expand Up @@ -1028,5 +1041,3 @@ bbbb >> bbbb * bbbb
last_call()
# standalone comment at ENDMARKER
```
Original file line number Diff line number Diff line change
Expand Up @@ -308,23 +308,29 @@ long_unmergable_string_with_pragma = (
```diff
--- Black
+++ Ruff
@@ -167,13 +167,9 @@
@@ -167,14 +167,14 @@

triple_quote_string = """This is a really really really long triple quote string assignment and it should not be touched."""

-assert (
- some_type_of_boolean_expression
-), "Followed by a really really really long string that is used to provide context to the AssertionError exception."
+assert some_type_of_boolean_expression, "Followed by a really really really long string that is used to provide context to the AssertionError exception."
+assert some_type_of_boolean_expression, (
+ "Followed by a really really really long string that is used to provide context to the AssertionError exception."
+)

-assert (
- some_type_of_boolean_expression
-), "Followed by a really really really long string that is used to provide context to the AssertionError exception, which uses dynamic string {}.".format(
+assert some_type_of_boolean_expression, "Followed by a really really really long string that is used to provide context to the AssertionError exception, which uses dynamic string {}.".format(
"formatting"
- "formatting"
+assert some_type_of_boolean_expression, (
+ "Followed by a really really really long string that is used to provide context to the AssertionError exception, which uses dynamic string {}.".format(
+ "formatting"
+ )
)

@@ -256,9 +252,7 @@
assert some_type_of_boolean_expression, (
@@ -256,9 +256,7 @@
+ CONCATENATED
+ "using the '+' operator."
)
Expand Down Expand Up @@ -509,10 +515,14 @@ pragma_comment_string2 = "Lines which end with an inline pragma comment of the f

triple_quote_string = """This is a really really really long triple quote string assignment and it should not be touched."""

assert some_type_of_boolean_expression, "Followed by a really really really long string that is used to provide context to the AssertionError exception."
assert some_type_of_boolean_expression, (
"Followed by a really really really long string that is used to provide context to the AssertionError exception."
)

assert some_type_of_boolean_expression, "Followed by a really really really long string that is used to provide context to the AssertionError exception, which uses dynamic string {}.".format(
"formatting"
assert some_type_of_boolean_expression, (
"Followed by a really really really long string that is used to provide context to the AssertionError exception, which uses dynamic string {}.".format(
"formatting"
)
)

assert some_type_of_boolean_expression, (
Expand Down
Loading

0 comments on commit cf7b995

Please sign in to comment.