Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore quote escapes in expression part of f-string #7597

Merged
merged 3 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 35 additions & 13 deletions crates/ruff_python_formatter/src/expression/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,8 @@ impl<'a> FormatString<'a> {

impl<'a> Format<PyFormatContext<'_>> for FormatString<'a> {
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
// TODO(dhruvmanila): With PEP 701, comments can be inside f-strings.
// This is to mark all of those comments as formatted but we need to
// figure out how to handle them.
if matches!(self.string, AnyString::FString(_)) {
f.context()
.comments()
.mark_verbatim_node_comments_formatted(self.string.into());
}
let locator = f.context().locator();
match self.layout {
let result = match self.layout {
StringLayout::Default => {
if self.string.is_implicit_concatenated() {
in_parentheses_only_group(&FormatStringContinuation::new(self.string)).fmt(f)
Expand All @@ -170,7 +162,18 @@ impl<'a> Format<PyFormatContext<'_>> for FormatString<'a> {
StringLayout::ImplicitConcatenatedStringInBinaryLike => {
FormatStringContinuation::new(self.string).fmt(f)
}
};
// TODO(dhruvmanila): With PEP 701, comments can be inside f-strings.
// This is to mark all of those comments as formatted but we need to
// figure out how to handle them. Note that this needs to be done only
// after the f-string is formatted, so only for all the non-formatted
// comments.
if matches!(self.string, AnyString::FString(_)) {
f.context()
.comments()
.mark_verbatim_node_comments_formatted(self.string.into());
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved
}
result
}
}

Expand Down Expand Up @@ -430,7 +433,7 @@ impl StringPart {
let normalized = normalize_string(
locator.slice(self.content_range),
preferred_quotes,
self.prefix.is_raw_string(),
self.prefix,
);

NormalizedString {
Expand Down Expand Up @@ -523,6 +526,10 @@ impl StringPrefix {
pub(super) const fn is_raw_string(self) -> bool {
self.contains(StringPrefix::RAW) || self.contains(StringPrefix::RAW_UPPER)
}

pub(super) const fn is_fstring(self) -> bool {
self.contains(StringPrefix::F_STRING)
}
}

impl Format<PyFormatContext<'_>> for StringPrefix {
Expand Down Expand Up @@ -760,7 +767,7 @@ impl Format<PyFormatContext<'_>> for StringQuotes {
/// with the provided `style`.
///
/// Returns the normalized string and whether it contains new lines.
fn normalize_string(input: &str, quotes: StringQuotes, is_raw: bool) -> Cow<str> {
fn normalize_string(input: &str, quotes: StringQuotes, prefix: StringPrefix) -> Cow<str> {
// The normalized string if `input` is not yet normalized.
// `output` must remain empty if `input` is already normalized.
let mut output = String::new();
Expand All @@ -774,7 +781,22 @@ fn normalize_string(input: &str, quotes: StringQuotes, is_raw: bool) -> Cow<str>

let mut chars = input.char_indices();

let is_raw = prefix.is_raw_string();
let is_fstring = prefix.is_fstring();
let mut in_formatted_value = false;
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved

while let Some((index, c)) = chars.next() {
if is_fstring && matches!(c, '{' | '}') {
if let Some(next) = input.as_bytes().get(index + 1).copied().map(char::from) {
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved
if next == c {
// Skip over the second character of the double braces
chars.next();
} else {
in_formatted_value = c == '{';
}
}
continue;
}
if c == '\r' {
output.push_str(&input[last_index..index]);

Expand All @@ -792,7 +814,7 @@ fn normalize_string(input: &str, quotes: StringQuotes, is_raw: bool) -> Cow<str>
if c == '\\' {
if let Some(next) = input.as_bytes().get(index + 1).copied().map(char::from) {
#[allow(clippy::if_same_then_else)]
if next == opposite_quote {
if next == opposite_quote && !in_formatted_value {
// Remove the escape by ending before the backslash and starting again with the quote
chars.next();
output.push_str(&input[last_index..index]);
Expand All @@ -805,7 +827,7 @@ fn normalize_string(input: &str, quotes: StringQuotes, is_raw: bool) -> Cow<str>
chars.next();
}
}
} else if c == preferred_quote {
} else if c == preferred_quote && !in_formatted_value {
// Escape the quote
output.push_str(&input[last_index..index]);
output.push('\\');
Expand Down
2 changes: 1 addition & 1 deletion scripts/formatter_ecosystem_checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ git -C "$dir/cpython" checkout 1a1bfc28912a39b500c578e9f10a8a222638d411

time cargo run --bin ruff_dev -- format-dev --stability-check \
--error-file "$target/progress_projects_errors.txt" --log-file "$target/progress_projects_log.txt" --stats-file "$target/progress_projects_stats.txt" \
--files-with-errors 16 --multi-project "$dir" || (
--files-with-errors 15 --multi-project "$dir" || (
echo "Ecosystem check failed"
cat "$target/progress_projects_log.txt"
exit 1
Expand Down