Skip to content

Commit c9031ce

Browse files
authored
[refurb] Mark autofix as safe only for number literals in FURB116 (astral-sh#17692)
<!-- Thank you for contributing to Ruff! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary We can only guarantee the safety of the autofix for number literals, all other cases may change the runtime behaviour of the program or introduce a syntax error. For the cases reported in the issue that would result in a syntax error, I disabled the autofix. Follow-up of astral-sh#17661. Fixes astral-sh#16472. <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan Snapshot tests. <!-- How was it tested? -->
1 parent 138ab91 commit c9031ce

File tree

5 files changed

+608
-129
lines changed

5 files changed

+608
-129
lines changed

crates/ruff_linter/resources/test/fixtures/refurb/FURB116.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import datetime
2+
import sys
3+
14
num = 1337
25

36
def return_num() -> int:
@@ -10,6 +13,7 @@ def return_num() -> int:
1013
print(oct(1337)[2:]) # FURB116
1114
print(hex(1337)[2:]) # FURB116
1215
print(bin(1337)[2:]) # FURB116
16+
print(bin(+1337)[2:]) # FURB116
1317

1418
print(bin(return_num())[2:]) # FURB116 (no autofix)
1519
print(bin(int(f"{num}"))[2:]) # FURB116 (no autofix)
@@ -22,3 +26,19 @@ def return_num() -> int:
2226
# float and complex numbers should be ignored
2327
print(bin(1.0)[2:])
2428
print(bin(3.14j)[2:])
29+
30+
d = datetime.datetime.now(tz=datetime.UTC)
31+
# autofix is display-only
32+
print(bin(d)[2:])
33+
# no autofix for Python 3.11 and earlier, as it introduces a syntax error
34+
print(bin(len("xyz").numerator)[2:])
35+
36+
# autofix is display-only
37+
print(bin({0: 1}[0].numerator)[2:])
38+
# no autofix for Python 3.11 and earlier, as it introduces a syntax error
39+
print(bin(ord("\\").numerator)[2:])
40+
print(hex(sys
41+
.maxunicode)[2:])
42+
43+
# for negatives numbers autofix is display-only
44+
print(bin(-1)[2:])

crates/ruff_linter/src/rules/refurb/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,15 @@ mod tests {
8989
assert_messages!(diagnostics);
9090
Ok(())
9191
}
92+
93+
#[test]
94+
fn fstring_number_format_python_311() -> Result<()> {
95+
let diagnostics = test_path(
96+
Path::new("refurb/FURB116.py"),
97+
&settings::LinterSettings::for_rule(Rule::FStringNumberFormat)
98+
.with_target_version(PythonVersion::PY311),
99+
)?;
100+
assert_messages!(diagnostics);
101+
Ok(())
102+
}
92103
}

crates/ruff_linter/src/rules/refurb/rules/fstring_number_format.rs

Lines changed: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
1+
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
22
use ruff_macros::{derive_message_formats, ViolationMetadata};
3-
use ruff_python_ast::{self as ast, Expr, ExprCall, Number};
3+
use ruff_python_ast::{self as ast, Expr, ExprCall, Number, PythonVersion, UnaryOp};
4+
use ruff_source_file::find_newline;
45
use ruff_text_size::Ranged;
56

67
use crate::checkers::ast::Checker;
@@ -24,6 +25,11 @@ use crate::fix::snippet::SourceCodeSnippet;
2425
/// ```python
2526
/// print(f"{1337:b}")
2627
/// ```
28+
///
29+
/// ## Fix safety
30+
/// The fix is only marked as safe for integer literals, all other cases
31+
/// are display-only, as they may change the runtime behaviour of the program
32+
/// or introduce syntax errors.
2733
#[derive(ViolationMetadata)]
2834
pub(crate) struct FStringNumberFormat {
2935
replacement: Option<SourceCodeSnippet>,
@@ -121,21 +127,24 @@ pub(crate) fn fstring_number_format(checker: &Checker, subscript: &ast::ExprSubs
121127
return;
122128
}
123129

124-
// Generate a replacement, if possible.
125-
let replacement = if matches!(
126-
arg,
127-
Expr::NumberLiteral(_) | Expr::Name(_) | Expr::Attribute(_)
128-
) {
129-
let inner_source = checker.locator().slice(arg);
130-
131-
let quote = checker.stylist().quote();
132-
let shorthand = base.shorthand();
130+
let maybe_number = if let Some(maybe_number) = arg
131+
.as_unary_op_expr()
132+
.filter(|unary_expr| unary_expr.op == UnaryOp::UAdd)
133+
.map(|unary_expr| &unary_expr.operand)
134+
{
135+
maybe_number
136+
} else {
137+
arg
138+
};
133139

134-
Some(format!("f{quote}{{{inner_source}:{shorthand}}}{quote}"))
140+
let applicability = if matches!(maybe_number, Expr::NumberLiteral(_)) {
141+
Applicability::Safe
135142
} else {
136-
None
143+
Applicability::DisplayOnly
137144
};
138145

146+
let replacement = try_create_replacement(checker, arg, base);
147+
139148
let mut diagnostic = Diagnostic::new(
140149
FStringNumberFormat {
141150
replacement: replacement.as_deref().map(SourceCodeSnippet::from_str),
@@ -145,15 +154,54 @@ pub(crate) fn fstring_number_format(checker: &Checker, subscript: &ast::ExprSubs
145154
);
146155

147156
if let Some(replacement) = replacement {
148-
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
149-
replacement,
150-
subscript.range(),
151-
)));
157+
let edit = Edit::range_replacement(replacement, subscript.range());
158+
diagnostic.set_fix(Fix::applicable_edit(edit, applicability));
152159
}
153160

154161
checker.report_diagnostic(diagnostic);
155162
}
156163

164+
/// Generate a replacement, if possible.
165+
fn try_create_replacement(checker: &Checker, arg: &Expr, base: Base) -> Option<String> {
166+
if !matches!(
167+
arg,
168+
Expr::NumberLiteral(_) | Expr::Name(_) | Expr::Attribute(_) | Expr::UnaryOp(_)
169+
) {
170+
return None;
171+
}
172+
173+
let inner_source = checker.locator().slice(arg);
174+
175+
// On Python 3.11 and earlier, trying to replace an `arg` that contains a backslash
176+
// would create a `SyntaxError` in the f-string.
177+
if checker.target_version() <= PythonVersion::PY311 && inner_source.contains('\\') {
178+
return None;
179+
}
180+
181+
// On Python 3.11 and earlier, trying to replace an `arg` that spans multiple lines
182+
// would create a `SyntaxError` in the f-string.
183+
if checker.target_version() <= PythonVersion::PY311 && find_newline(inner_source).is_some() {
184+
return None;
185+
}
186+
187+
let quote = checker.stylist().quote();
188+
let shorthand = base.shorthand();
189+
190+
// If the `arg` contains double quotes we need to create the f-string with single quotes
191+
// to avoid a `SyntaxError` in Python 3.11 and earlier.
192+
if checker.target_version() <= PythonVersion::PY311 && inner_source.contains(quote.as_str()) {
193+
return None;
194+
}
195+
196+
// If the `arg` contains a brace add an space before it to avoid a `SyntaxError`
197+
// in the f-string.
198+
if inner_source.starts_with('{') {
199+
Some(format!("f{quote}{{ {inner_source}:{shorthand}}}{quote}"))
200+
} else {
201+
Some(format!("f{quote}{{{inner_source}:{shorthand}}}{quote}"))
202+
}
203+
}
204+
157205
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
158206
enum Base {
159207
Hex,

0 commit comments

Comments
 (0)