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.

Sign up for GitHub

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

Fix UP032 auto-fix with integers #4525

Merged
merged 3 commits into from
May 19, 2023
Merged

Fix UP032 auto-fix with integers #4525

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25e0538
Fix UP032 auto-fix with integers
JonathanPlasse May 19, 2023
d563abc
Merge branch 'main' into fix-up032-auto-fix-with-integers
charliermarsh May 19, 2023
4fea6a4
Extend test
charliermarsh May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/ruff/resources/test/fixtures/pyupgrade/UP032_2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"{.real}".format(1)

"{0.real}".format(1)

"{a.real}".format(a=1)
Copy link
Member

Choose a reason for hiding this comment

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

Can you add some more test cases here, for floats, octal integers, complex, etc.?

1 change: 1 addition & 0 deletions crates/ruff/src/rules/pyupgrade/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ mod tests {
#[test_case(Rule::PrintfStringFormatting, Path::new("UP031_1.py"); "UP031_1")]
#[test_case(Rule::FString, Path::new("UP032_0.py"); "UP032_0")]
#[test_case(Rule::FString, Path::new("UP032_1.py"); "UP032_1")]
#[test_case(Rule::FString, Path::new("UP032_2.py"); "UP032_2")]
#[test_case(Rule::LRUCacheWithMaxsizeNone, Path::new("UP033_0.py"); "UP033_0")]
#[test_case(Rule::LRUCacheWithMaxsizeNone, Path::new("UP033_1.py"); "UP033_1")]
#[test_case(Rule::ExtraneousParentheses, Path::new("UP034.py"); "UP034")]
Expand Down
19 changes: 16 additions & 3 deletions crates/ruff/src/rules/pyupgrade/rules/f_strings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,24 +171,37 @@ fn try_convert_to_f_string(checker: &Checker, expr: &Expr) -> Option<String> {
converted.push('{');

let field = FieldName::parse(&field_name).ok()?;
let has_field_parts = !field.parts.is_empty();
match field.field_type {
FieldType::Auto => {
let Some(arg) = summary.consume_next() else {
return None;
};
converted.push_str(&arg);
if has_field_parts && arg.chars().all(|c| c.is_ascii_digit()) {
converted.push_str(&format!("({arg})"));
} else {
converted.push_str(&arg);
}
}
FieldType::Index(index) => {
let Some(arg) = summary.consume_arg(index) else {
return None;
};
converted.push_str(&arg);
if has_field_parts && arg.chars().all(|c| c.is_ascii_digit()) {
converted.push_str(&format!("({arg})"));
} else {
converted.push_str(&arg);
}
}
FieldType::Keyword(name) => {
let Some(arg) = summary.consume_kwarg(&name) else {
return None;
};
converted.push_str(&arg);
if has_field_parts && arg.chars().all(|c| c.is_ascii_digit()) {
converted.push_str(&format!("({arg})"));
} else {
converted.push_str(&arg);
}
}
}

Expand Down
55 changes: 55 additions & 0 deletions crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP032_2.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
source: crates/ruff/src/rules/pyupgrade/mod.rs
---
UP032_2.py:1:1: UP032 [*] Use f-string instead of `format` call
|
1 | "{.real}".format(1)
| ^^^^^^^^^^^^^^^^^^^ UP032
2 |
3 | "{0.real}".format(1)
Copy link
Member

Choose a reason for hiding this comment

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

Happens happens with floats? E.g. .format(1.0)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested it before.
The problem is only present with integers. floats and complex numbers work.

f"{0.real}"  # SyntaxError
f"{0.0.real}"  # OK
f"{0j.real}"  # OK

Copy link
Member

Choose a reason for hiding this comment

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

Great, thank you 🙏

Copy link
Member

Choose a reason for hiding this comment

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

It looks like these work too?

>>> f"{0b101.real}"
'5'
>>> f"{0o101.real}"
'65'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed

|
= help: Convert to f-string

ℹ Suggested fix
1 |-"{.real}".format(1)
1 |+f"{(1).real}"
2 2 |
3 3 | "{0.real}".format(1)
4 4 |

UP032_2.py:3:1: UP032 [*] Use f-string instead of `format` call
|
3 | "{.real}".format(1)
4 |
5 | "{0.real}".format(1)
| ^^^^^^^^^^^^^^^^^^^^ UP032
6 |
7 | "{a.real}".format(a=1)
|
= help: Convert to f-string

ℹ Suggested fix
1 1 | "{.real}".format(1)
2 2 |
3 |-"{0.real}".format(1)
3 |+f"{(1).real}"
4 4 |
5 5 | "{a.real}".format(a=1)

UP032_2.py:5:1: UP032 [*] Use f-string instead of `format` call
|
5 | "{0.real}".format(1)
6 |
7 | "{a.real}".format(a=1)
| ^^^^^^^^^^^^^^^^^^^^^^ UP032
|
= help: Convert to f-string

ℹ Suggested fix
2 2 |
3 3 | "{0.real}".format(1)
4 4 |
5 |-"{a.real}".format(a=1)
5 |+f"{(1).real}"