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

Formatter quoting for f-strings with triple quotes #7826

Merged
merged 5 commits into from
Oct 11, 2023

Conversation

konstin
Copy link
Member

@konstin konstin commented Oct 5, 2023

Summary Quoting of f-strings can change if they are triple quoted and only contain single quotes inside.

Fixes #6841

Test Plan New fixtures

@konstin
Copy link
Member Author

konstin commented Oct 5, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

Comment on lines 50 to 65
Self::FString(f_string) => {
let unprefixed = locator
.slice(f_string.range)
.trim_start_matches(|c| c != '"' && c != '\'');
let triple_quotes =
unprefixed.starts_with(r#"""""#) || unprefixed.starts_with(r#"'''"#);
if f_string.values.iter().any(|value| match value {
Expr::FormattedValue(ast::ExprFormattedValue { range, .. }) => {
let string_content = locator.slice(*range);
string_content.contains(['"', '\''])
if triple_quotes {
string_content.contains(r#"""""#) || string_content.contains(r#"'''"#)
} else {
string_content.contains(['"', '\''])
}
}
_ => false,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this logic belongs here (it should be part of normalize), but i'll wait for dhruv's f-string changes first

@konstin konstin marked this pull request as ready for review October 6, 2023 09:00
Base automatically changed from ruff_python_formatter-serde to main October 8, 2023 13:47
@konstin konstin requested a review from dhruvmanila October 10, 2023 08:52
Comment on lines 51 to 55
let unprefixed = locator
.slice(f_string.range)
.trim_start_matches(|c| c != '"' && c != '\'');
let triple_quotes =
unprefixed.starts_with(r#"""""#) || unprefixed.starts_with(r#"'''"#);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, we need to figure out if this f-string is triple-quoted or not. If so, could we use ruff_python_ast::str::is_triple_quote here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That method only takes a prefix, so i'd need to extract the prefix here, which is surprisingly complex (iterators don't work because we don't want to allocate a new String when collecting)

crates/ruff_python_formatter/src/expression/string.rs Outdated Show resolved Hide resolved
crates/ruff_python_formatter/src/expression/string.rs Outdated Show resolved Hide resolved
konstin and others added 5 commits October 11, 2023 13:21
**Summary** Quoting of f-strings can change if they are triple quoted and only contain single quotes inside.

Fixes #6841

**Test Plan** New fixtures
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
@konstin konstin force-pushed the formatter_f-strings_with_triple_quotes branch from d88f317 to c5fe59a Compare October 11, 2023 11:21
@konstin konstin enabled auto-merge (squash) October 11, 2023 11:22
@konstin konstin merged commit 644011f into main Oct 11, 2023
15 checks passed
@konstin konstin deleted the formatter_f-strings_with_triple_quotes branch October 11, 2023 11:30
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 11, 2023

CodSpeed Performance Report

Merging #7826 will improve performances by 4.06%

Comparing formatter_f-strings_with_triple_quotes (c5fe59a) with main (a1ee6d2)

Summary

⚡ 1 improvements
✅ 24 untouched benchmarks

Benchmarks breakdown

Benchmark main formatter_f-strings_with_triple_quotes Change
linter/default-rules[large/dataset.py] 95.4 ms 91.7 ms +4.06%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

formatter bug: f-strings with triple quotes
2 participants