Skip to content

Remove empty strings when converting to f-string (UP032) - #11524

Merged
dhruvmanila merged 2 commits into
mainfrom
dhruv/remove-empty-string
May 27, 2024
Merged

dhruvmanila merged 2 commits into
mainfrom
dhruv/remove-empty-string

Conversation

@dhruvmanila

Copy link
Copy Markdown
Member

Summary

This PR brings back the functionality to remove empty strings when converting to an f-string in UP032.

For context, #8712 added this functionality to remove trailing empty strings but it got removed in #8697 possibly unexpectedly so.

There's one difference which is that this PR will remove any empty strings and not just trailing ones. For example,

--- /Users/dhruv/playground/ruff/src/UP032.py
+++ /Users/dhruv/playground/ruff/src/UP032.py
@@ -1,7 +1,5 @@
 (
-    "{a}"
-    ""
-    "{b}"
-    ""
-).format(a=1, b=1)
+    f"{1}"
+    f"{1}"
+)

Test Plan

Run cargo insta test and update the snapshots.

Comment on lines -470 to -478
if !lexer::lex_starts_at(rest, Mode::Expression, prev_end)
.flatten()
.all(|(token, _)| match token {
Tok::Comment(_) | Tok::Newline | Tok::NonLogicalNewline | Tok::Indent | Tok::Dedent => {
true
for (range, conversion) in patches {
let fstring = match conversion {
FStringConversion::Convert(fstring) => Some(fstring),
FStringConversion::EmptyLiteral => None,
FStringConversion::Literal => {
// Convert escaped curly brackets e.g. `{{` to `{` in literal string parts
Some(curly_unescape(checker.locator().slice(range)).to_string())
}
Tok::String { value, .. } => value.is_empty(),
_ => false,
})

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The main reason for this PR is to remove this little lexer usage which uses the owned data from String token.

@dhruvmanila
dhruvmanila requested a review from charliermarsh May 24, 2024 04:32
@dhruvmanila dhruvmanila added the rule Implementing or modifying a lint rule label May 24, 2024
@github-actions

github-actions Bot commented May 24, 2024 •

Copy link
Copy Markdown
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Comment thread crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs Outdated
@dhruvmanila
dhruvmanila enabled auto-merge (squash) May 27, 2024 04:59
@dhruvmanila
dhruvmanila disabled auto-merge May 27, 2024 05:00
@dhruvmanila dhruvmanila changed the title Remove empty strings when converting to f-string Remove empty strings when converting to f-string (UP032) May 27, 2024
@dhruvmanila
dhruvmanila enabled auto-merge (squash) May 27, 2024 05:00
@dhruvmanila
dhruvmanila merged commit 9200dfc into main May 27, 2024
@dhruvmanila
dhruvmanila deleted the dhruv/remove-empty-string branch May 27, 2024 05:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants