Skip to content

Commit

Permalink
skip normalisation if there are quotes inside FormattedValues
Browse files Browse the repository at this point in the history
  • Loading branch information
davidszotten committed Jul 30, 2023
1 parent 07a08ba commit ef0aaec
Show file tree
Hide file tree
Showing 14 changed files with 125 additions and 623 deletions.
36 changes: 27 additions & 9 deletions crates/ruff_python_formatter/src/expression/expr_joined_str.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use super::string::{normalize_string, preferred_quotes, StringPrefix, StringQuotes};
use crate::context::PyFormatContext;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::expression::Expr;
use crate::prelude::*;
use crate::{FormatError, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprJoinedStr;
use ruff_python_ast::{self as ast, ExprJoinedStr};
use ruff_text_size::{TextLen, TextRange};
use std::borrow::Cow;

Expand All @@ -14,7 +15,16 @@ pub struct FormatExprJoinedStr;

impl FormatNodeRule<ExprJoinedStr> for FormatExprJoinedStr {
fn fmt_fields(&self, item: &ExprJoinedStr, f: &mut PyFormatter) -> FormatResult<()> {
let ExprJoinedStr { range, .. } = item;
let ExprJoinedStr { range, values } = item;

let contains_quotes = values.iter().any(|value| match value {
Expr::FormattedValue(ast::ExprFormattedValue { range, .. }) => {
let string_content = f.context().locator().slice(*range);
string_content.contains('"') || string_content.contains('\'')
}
_ => false,
});

let string_content = f.context().locator().slice(*range);

let prefix = StringPrefix::parse(string_content);
Expand All @@ -30,18 +40,26 @@ impl FormatNodeRule<ExprJoinedStr> for FormatExprJoinedStr {
let raw_content_range = relative_raw_content_range + range.start();

let raw_content = &string_content[relative_raw_content_range];
let preferred_quotes = preferred_quotes(raw_content, quotes, f.options().quote_style());
let preferred_quotes = if contains_quotes {
quotes
} else {
preferred_quotes(raw_content, quotes, f.options().quote_style())
};

write!(f, [prefix, preferred_quotes])?;

let (normalized, contains_newlines) = normalize_string(raw_content, preferred_quotes);

match normalized {
Cow::Borrowed(_) => {
source_text_slice(raw_content_range, contains_newlines).fmt(f)?;
}
Cow::Owned(normalized) => {
dynamic_text(&normalized, Some(raw_content_range.start())).fmt(f)?;
if contains_quotes {
source_text_slice(raw_content_range, contains_newlines).fmt(f)?;
} else {
match normalized {
Cow::Borrowed(_) => {
source_text_slice(raw_content_range, contains_newlines).fmt(f)?;
}
Cow::Owned(normalized) => {
dynamic_text(&normalized, Some(raw_content_range.start())).fmt(f)?;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class DebugVisitor(Visitor[T]):
```diff
--- Black
+++ Ruff
@@ -3,24 +3,29 @@
@@ -3,24 +3,24 @@
tree_depth: int = 0
def visit_default(self, node: LN) -> Iterator[T]:
Expand All @@ -53,30 +53,25 @@ class DebugVisitor(Visitor[T]):
if isinstance(node, Node):
_type = type_repr(node.type)
- out(f'{indent}{_type}', fg='yellow')
+ out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow")
+ out(f"{indent}{_type}", fg="yellow")
self.tree_depth += 1
for child in node.children:
yield from self.visit(child)
self.tree_depth -= 1
- out(f'{indent}/{_type}', fg='yellow', bold=False)
+ out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow", bold=False)
+ out(f"{indent}/{_type}", fg="yellow", bold=False)
else:
_type = token.tok_name.get(node.type, str(node.type))
- out(f'{indent}{_type}', fg='blue', nl=False)
+ out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", nl=False)
+ out(f"{indent}{_type}", fg="blue", nl=False)
if node.prefix:
# We don't have to handle prefixes for `Node` objects since
# that delegates to the first child anyway.
- out(f' {node.prefix!r}', fg='green', bold=False, nl=False)
- out(f' {node.value!r}', fg='blue', bold=False)
+ out(
+ f"NOT_YET_IMPLEMENTED_ExprJoinedStr",
+ fg="green",
+ bold=False,
+ nl=False,
+ )
+ out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", bold=False)
+ out(f" {node.prefix!r}", fg="green", bold=False, nl=False)
+ out(f" {node.value!r}", fg="blue", bold=False)
@classmethod
def show(cls, code: str) -> None:
Expand All @@ -93,26 +88,21 @@ class DebugVisitor(Visitor[T]):
indent = " " * (2 * self.tree_depth)
if isinstance(node, Node):
_type = type_repr(node.type)
out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow")
out(f"{indent}{_type}", fg="yellow")
self.tree_depth += 1
for child in node.children:
yield from self.visit(child)
self.tree_depth -= 1
out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow", bold=False)
out(f"{indent}/{_type}", fg="yellow", bold=False)
else:
_type = token.tok_name.get(node.type, str(node.type))
out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", nl=False)
out(f"{indent}{_type}", fg="blue", nl=False)
if node.prefix:
# We don't have to handle prefixes for `Node` objects since
# that delegates to the first child anyway.
out(
f"NOT_YET_IMPLEMENTED_ExprJoinedStr",
fg="green",
bold=False,
nl=False,
)
out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", bold=False)
out(f" {node.prefix!r}", fg="green", bold=False, nl=False)
out(f" {node.value!r}", fg="blue", bold=False)
@classmethod
def show(cls, code: str) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def do_not_touch_this_prefix3():
def do_not_touch_this_prefix2():
- FR'There was a bug where docstring prefixes would be normalized even with -S.'
+ f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+ Rf"There was a bug where docstring prefixes would be normalized even with -S."
def do_not_touch_this_prefix3():
Expand All @@ -43,7 +43,7 @@ def do_not_touch_this_prefix():
def do_not_touch_this_prefix2():
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
Rf"There was a bug where docstring prefixes would be normalized even with -S."
def do_not_touch_this_prefix3():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,19 +304,23 @@ long_unmergable_string_with_pragma = (
```diff
--- Black
+++ Ruff
@@ -143,9 +143,9 @@
@@ -143,9 +143,13 @@
)
)

-fstring = f"f-strings definitely make things more {difficult} than they need to be for {{black}}. But boy they sure are handy. The problem is that some lines will need to have the 'f' whereas others do not. This {line}, for example, needs one."
+fstring = f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+fstring = (
+ f"f-strings definitely make things more {difficult} than they need to be for {{black}}. But boy they sure are handy. The problem is that some lines will need to have the 'f' whereas others do not. This {line}, for example, needs one."
+)

-fstring_with_no_fexprs = f"Some regular string that needs to get split certainly but is NOT an fstring by any means whatsoever."
+fstring_with_no_fexprs = f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+fstring_with_no_fexprs = (
+ f"Some regular string that needs to get split certainly but is NOT an fstring by any means whatsoever."
+)

comment_string = "Long lines with inline comments should have their comments appended to the reformatted string's enclosing right parentheses." # This comment gets thrown to the top.

@@ -165,13 +165,9 @@
@@ -165,13 +169,9 @@

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

Expand All @@ -332,7 +336,7 @@ long_unmergable_string_with_pragma = (
"formatting"
)

@@ -221,8 +217,8 @@
@@ -221,8 +221,8 @@
func_with_bad_comma(
(
"This is a really long string argument to a function that has a trailing comma"
Expand All @@ -343,12 +347,14 @@ long_unmergable_string_with_pragma = (
)

func_with_bad_parens_that_wont_fit_in_one_line(
@@ -274,7 +270,7 @@
@@ -274,7 +274,9 @@
yield "This is a really long string that can't possibly be expected to fit all together on one line. In fact it may even take up three or more lines... like four or five... but probably just three."


-x = f"This is a {{really}} long string that needs to be split without a doubt (i.e. most definitely). In short, this {string} that can't possibly be {{expected}} to fit all together on one line. In {fact} it may even take up three or more lines... like four or five... but probably just four."
+x = f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+x = (
+ f"This is a {{really}} long string that needs to be split without a doubt (i.e. most definitely). In short, this {string} that can't possibly be {{expected}} to fit all together on one line. In {fact} it may even take up three or more lines... like four or five... but probably just four."
+)

long_unmergable_string_with_pragma = (
"This is a really long string that can't be merged because it has a likely pragma at the end" # type: ignore
Expand Down Expand Up @@ -502,9 +508,13 @@ old_fmt_string3 = (
)
)

fstring = f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
fstring = (
f"f-strings definitely make things more {difficult} than they need to be for {{black}}. But boy they sure are handy. The problem is that some lines will need to have the 'f' whereas others do not. This {line}, for example, needs one."
)

fstring_with_no_fexprs = f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
fstring_with_no_fexprs = (
f"Some regular string that needs to get split certainly but is NOT an fstring by any means whatsoever."
)

comment_string = "Long lines with inline comments should have their comments appended to the reformatted string's enclosing right parentheses." # This comment gets thrown to the top.

Expand Down Expand Up @@ -629,7 +639,9 @@ def foo():
yield "This is a really long string that can't possibly be expected to fit all together on one line. In fact it may even take up three or more lines... like four or five... but probably just three."


x = f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
x = (
f"This is a {{really}} long string that needs to be split without a doubt (i.e. most definitely). In short, this {string} that can't possibly be {{expected}} to fit all together on one line. In {fact} it may even take up three or more lines... like four or five... but probably just four."
)

long_unmergable_string_with_pragma = (
"This is a really long string that can't be merged because it has a likely pragma at the end" # type: ignore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,27 +69,17 @@ f"\"{a}\"{'hello' * b}\"{c}\""
```diff
--- Black
+++ Ruff
@@ -15,16 +15,21 @@
"""Here's a " """
"""Just a normal triple
quote"""
-f"just a normal {f} string"
-f"""This is a triple-quoted {f}-string"""
-f'MOAR {" ".join([])}'
-f"MOAR {' '.join([])}"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
@@ -20,11 +20,16 @@
f'MOAR {" ".join([])}'
f"MOAR {' '.join([])}"
r"raw string ftw"
-r"Date d\'expiration:(.*)"
+r"Date d'expiration:(.*)"
r'Tricky "quote'
-r"Not-so-tricky \"quote"
-rf"{yay}"
-"\nThe \"quick\"\nbrown fox\njumps over\nthe 'lazy' dog.\n"
+r'Not-so-tricky "quote'
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
rf"{yay}"
-"\nThe \"quick\"\nbrown fox\njumps over\nthe 'lazy' dog.\n"
+"\n\
+The \"quick\"\n\
+brown fox\n\
Expand All @@ -99,27 +89,6 @@ f"\"{a}\"{'hello' * b}\"{c}\""
re.compile(r'[\\"]')
"x = ''; y = \"\""
"x = '''; y = \"\""
@@ -39,14 +44,14 @@
'\\""'
"\\''"
"Lots of \\\\\\\\'quotes'"
-f'{y * " "} \'{z}\''
-f"{{y * \" \"}} '{z}'"
-f'\'{z}\' {y * " "}'
-f"{y * x} '{z}'"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
"'{z}' {y * \" \"}"
"{y * x} '{z}'"
# We must bail out if changing the quotes would introduce backslashes in f-string
# expressions. xref: https://github.com/psf/black/issues/2348
-f"\"{b}\"{' ' * (long-len(b)+1)}: \"{sts}\",\n"
-f"\"{a}\"{'hello' * b}\"{c}\""
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
```

## Ruff Output
Expand All @@ -142,15 +111,15 @@ f"\"{a}\"{'hello' * b}\"{c}\""
"""Here's a " """
"""Just a normal triple
quote"""
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"just a normal {f} string"
f"""This is a triple-quoted {f}-string"""
f'MOAR {" ".join([])}'
f"MOAR {' '.join([])}"
r"raw string ftw"
r"Date d'expiration:(.*)"
r'Tricky "quote'
r'Not-so-tricky "quote'
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
rf"{yay}"
"\n\
The \"quick\"\n\
brown fox\n\
Expand All @@ -171,17 +140,17 @@ re.compile(r'[\\"]')
'\\""'
"\\''"
"Lots of \\\\\\\\'quotes'"
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f'{y * " "} \'{z}\''
f"{{y * \" \"}} '{z}'"
f'\'{z}\' {y * " "}'
f"{y * x} '{z}'"
"'{z}' {y * \" \"}"
"{y * x} '{z}'"
# We must bail out if changing the quotes would introduce backslashes in f-string
# expressions. xref: https://github.com/psf/black/issues/2348
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
f"\"{b}\"{' ' * (long-len(b)+1)}: \"{sts}\",\n"
f"\"{a}\"{'hello' * b}\"{c}\""
```
## Black Output
Expand Down
Loading

0 comments on commit ef0aaec

Please sign in to comment.