[refurb] Implement fstring-number-format (FURB116) (#10921)

## Summary

Adds `FURB116`

See #1348 

## Test Plan

`cargo test`
This commit is contained in:
Steve C 2024-04-25 21:15:33 -04:00 committed by GitHub
parent b15e9e6e05
commit c8c227dd5d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 352 additions and 0 deletions

View file

@ -0,0 +1,19 @@
num = 1337
def return_num() -> int:
return num
print(oct(num)[2:]) # FURB116
print(hex(num)[2:]) # FURB116
print(bin(num)[2:]) # FURB116
print(oct(1337)[2:]) # FURB116
print(hex(1337)[2:]) # FURB116
print(bin(1337)[2:]) # FURB116
print(bin(return_num())[2:]) # FURB116 (no autofix)
print(bin(int(f"{num}"))[2:]) # FURB116 (no autofix)
## invalid
print(oct(0o1337)[1:])
print(hex(0x1337)[3:])

View file

@ -128,6 +128,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::SortedMinMax) { if checker.enabled(Rule::SortedMinMax) {
refurb::rules::sorted_min_max(checker, subscript); refurb::rules::sorted_min_max(checker, subscript);
} }
if checker.enabled(Rule::FStringNumberFormat) {
refurb::rules::fstring_number_format(checker, subscript);
}
pandas_vet::rules::subscript(checker, value, expr); pandas_vet::rules::subscript(checker, value, expr);
} }

View file

@ -1050,6 +1050,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Refurb, "110") => (RuleGroup::Preview, rules::refurb::rules::IfExpInsteadOfOrOperator), (Refurb, "110") => (RuleGroup::Preview, rules::refurb::rules::IfExpInsteadOfOrOperator),
#[allow(deprecated)] #[allow(deprecated)]
(Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend), (Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),
(Refurb, "116") => (RuleGroup::Preview, rules::refurb::rules::FStringNumberFormat),
(Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator), (Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator),
(Refurb, "129") => (RuleGroup::Preview, rules::refurb::rules::ReadlinesInFor), (Refurb, "129") => (RuleGroup::Preview, rules::refurb::rules::ReadlinesInFor),
#[allow(deprecated)] #[allow(deprecated)]

View file

@ -42,6 +42,7 @@ mod tests {
#[test_case(Rule::HashlibDigestHex, Path::new("FURB181.py"))] #[test_case(Rule::HashlibDigestHex, Path::new("FURB181.py"))]
#[test_case(Rule::ListReverseCopy, Path::new("FURB187.py"))] #[test_case(Rule::ListReverseCopy, Path::new("FURB187.py"))]
#[test_case(Rule::WriteWholeFile, Path::new("FURB103.py"))] #[test_case(Rule::WriteWholeFile, Path::new("FURB103.py"))]
#[test_case(Rule::FStringNumberFormat, Path::new("FURB116.py"))]
#[test_case(Rule::SortedMinMax, Path::new("FURB192.py"))] #[test_case(Rule::SortedMinMax, Path::new("FURB192.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> { fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());

View file

@ -0,0 +1,181 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, ExprCall};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;
/// ## What it does
/// Checks for uses of `bin(...)[2:]` (or `hex`, or `oct`) to convert
/// an integer into a string.
///
/// ## Why is this bad?
/// When converting an integer to a baseless binary, hexadecimal, or octal
/// string, using f-strings is more concise and readable than using the
/// `bin`, `hex`, or `oct` functions followed by a slice.
///
/// ## Example
/// ```python
/// print(bin(1337)[2:])
/// ```
///
/// Use instead:
/// ```python
/// print(f"{1337:b}")
/// ```
#[violation]
pub struct FStringNumberFormat {
replacement: Option<SourceCodeSnippet>,
base: Base,
}
impl Violation for FStringNumberFormat {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
let FStringNumberFormat { replacement, base } = self;
let function_name = base.function_name();
if let Some(display) = replacement
.as_ref()
.and_then(SourceCodeSnippet::full_display)
{
format!("Replace `{function_name}` call with `{display}`")
} else {
format!("Replace `{function_name}` call with f-string")
}
}
fn fix_title(&self) -> Option<String> {
let FStringNumberFormat { replacement, .. } = self;
if let Some(display) = replacement
.as_ref()
.and_then(SourceCodeSnippet::full_display)
{
Some(format!("Replace with `{display}`"))
} else {
Some(format!("Replace with f-string"))
}
}
}
/// FURB116
pub(crate) fn fstring_number_format(checker: &mut Checker, subscript: &ast::ExprSubscript) {
// The slice must be exactly `[2:]`.
let Expr::Slice(ast::ExprSlice {
lower: Some(lower),
upper: None,
step: None,
..
}) = subscript.slice.as_ref()
else {
return;
};
let Expr::NumberLiteral(ast::ExprNumberLiteral {
value: ast::Number::Int(int),
..
}) = lower.as_ref()
else {
return;
};
if *int != 2 {
return;
}
// The call must be exactly `hex(...)`, `bin(...)`, or `oct(...)`.
let Expr::Call(ExprCall {
func, arguments, ..
}) = subscript.value.as_ref()
else {
return;
};
if !arguments.keywords.is_empty() {
return;
}
let [arg] = &*arguments.args else {
return;
};
let Some(id) = checker.semantic().resolve_builtin_symbol(func) else {
return;
};
let Some(base) = Base::from_str(id) else {
return;
};
// Generate a replacement, if possible.
let replacement = if matches!(
arg,
Expr::NumberLiteral(_) | Expr::Name(_) | Expr::Attribute(_)
) {
let inner_source = checker.locator().slice(arg);
let quote = checker.stylist().quote();
let shorthand = base.shorthand();
Some(format!("f{quote}{{{inner_source}:{shorthand}}}{quote}"))
} else {
None
};
let mut diagnostic = Diagnostic::new(
FStringNumberFormat {
replacement: replacement.as_deref().map(SourceCodeSnippet::from_str),
base,
},
subscript.range(),
);
if let Some(replacement) = replacement {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
replacement,
subscript.range(),
)));
}
checker.diagnostics.push(diagnostic);
}
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum Base {
Hex,
Bin,
Oct,
}
impl Base {
/// Returns the shorthand for the base.
fn shorthand(self) -> &'static str {
match self {
Base::Hex => "x",
Base::Bin => "b",
Base::Oct => "o",
}
}
/// Returns the builtin function name for the base.
fn function_name(self) -> &'static str {
match self {
Base::Hex => "hex",
Base::Bin => "bin",
Base::Oct => "oct",
}
}
/// Parses the base from a string.
fn from_str(s: &str) -> Option<Self> {
match s {
"hex" => Some(Base::Hex),
"bin" => Some(Base::Bin),
"oct" => Some(Base::Oct),
_ => None,
}
}
}

View file

@ -2,6 +2,7 @@ pub(crate) use bit_count::*;
pub(crate) use check_and_remove_from_set::*; pub(crate) use check_and_remove_from_set::*;
pub(crate) use delete_full_slice::*; pub(crate) use delete_full_slice::*;
pub(crate) use for_loop_set_mutations::*; pub(crate) use for_loop_set_mutations::*;
pub(crate) use fstring_number_format::*;
pub(crate) use hashlib_digest_hex::*; pub(crate) use hashlib_digest_hex::*;
pub(crate) use if_exp_instead_of_or_operator::*; pub(crate) use if_exp_instead_of_or_operator::*;
pub(crate) use if_expr_min_max::*; pub(crate) use if_expr_min_max::*;
@ -32,6 +33,7 @@ mod bit_count;
mod check_and_remove_from_set; mod check_and_remove_from_set;
mod delete_full_slice; mod delete_full_slice;
mod for_loop_set_mutations; mod for_loop_set_mutations;
mod fstring_number_format;
mod hashlib_digest_hex; mod hashlib_digest_hex;
mod if_exp_instead_of_or_operator; mod if_exp_instead_of_or_operator;
mod if_expr_min_max; mod if_expr_min_max;

View file

@ -0,0 +1,144 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
---
FURB116.py:6:7: FURB116 [*] Replace `oct` call with `f"{num:o}"`
|
4 | return num
5 |
6 | print(oct(num)[2:]) # FURB116
| ^^^^^^^^^^^^ FURB116
7 | print(hex(num)[2:]) # FURB116
8 | print(bin(num)[2:]) # FURB116
|
= help: Replace with `f"{num:o}"`
Safe fix
3 3 | def return_num() -> int:
4 4 | return num
5 5 |
6 |-print(oct(num)[2:]) # FURB116
6 |+print(f"{num:o}") # FURB116
7 7 | print(hex(num)[2:]) # FURB116
8 8 | print(bin(num)[2:]) # FURB116
9 9 |
FURB116.py:7:7: FURB116 [*] Replace `hex` call with `f"{num:x}"`
|
6 | print(oct(num)[2:]) # FURB116
7 | print(hex(num)[2:]) # FURB116
| ^^^^^^^^^^^^ FURB116
8 | print(bin(num)[2:]) # FURB116
|
= help: Replace with `f"{num:x}"`
Safe fix
4 4 | return num
5 5 |
6 6 | print(oct(num)[2:]) # FURB116
7 |-print(hex(num)[2:]) # FURB116
7 |+print(f"{num:x}") # FURB116
8 8 | print(bin(num)[2:]) # FURB116
9 9 |
10 10 | print(oct(1337)[2:]) # FURB116
FURB116.py:8:7: FURB116 [*] Replace `bin` call with `f"{num:b}"`
|
6 | print(oct(num)[2:]) # FURB116
7 | print(hex(num)[2:]) # FURB116
8 | print(bin(num)[2:]) # FURB116
| ^^^^^^^^^^^^ FURB116
9 |
10 | print(oct(1337)[2:]) # FURB116
|
= help: Replace with `f"{num:b}"`
Safe fix
5 5 |
6 6 | print(oct(num)[2:]) # FURB116
7 7 | print(hex(num)[2:]) # FURB116
8 |-print(bin(num)[2:]) # FURB116
8 |+print(f"{num:b}") # FURB116
9 9 |
10 10 | print(oct(1337)[2:]) # FURB116
11 11 | print(hex(1337)[2:]) # FURB116
FURB116.py:10:7: FURB116 [*] Replace `oct` call with `f"{1337:o}"`
|
8 | print(bin(num)[2:]) # FURB116
9 |
10 | print(oct(1337)[2:]) # FURB116
| ^^^^^^^^^^^^^ FURB116
11 | print(hex(1337)[2:]) # FURB116
12 | print(bin(1337)[2:]) # FURB116
|
= help: Replace with `f"{1337:o}"`
Safe fix
7 7 | print(hex(num)[2:]) # FURB116
8 8 | print(bin(num)[2:]) # FURB116
9 9 |
10 |-print(oct(1337)[2:]) # FURB116
10 |+print(f"{1337:o}") # FURB116
11 11 | print(hex(1337)[2:]) # FURB116
12 12 | print(bin(1337)[2:]) # FURB116
13 13 |
FURB116.py:11:7: FURB116 [*] Replace `hex` call with `f"{1337:x}"`
|
10 | print(oct(1337)[2:]) # FURB116
11 | print(hex(1337)[2:]) # FURB116
| ^^^^^^^^^^^^^ FURB116
12 | print(bin(1337)[2:]) # FURB116
|
= help: Replace with `f"{1337:x}"`
Safe fix
8 8 | print(bin(num)[2:]) # FURB116
9 9 |
10 10 | print(oct(1337)[2:]) # FURB116
11 |-print(hex(1337)[2:]) # FURB116
11 |+print(f"{1337:x}") # FURB116
12 12 | print(bin(1337)[2:]) # FURB116
13 13 |
14 14 | print(bin(return_num())[2:]) # FURB116 (no autofix)
FURB116.py:12:7: FURB116 [*] Replace `bin` call with `f"{1337:b}"`
|
10 | print(oct(1337)[2:]) # FURB116
11 | print(hex(1337)[2:]) # FURB116
12 | print(bin(1337)[2:]) # FURB116
| ^^^^^^^^^^^^^ FURB116
13 |
14 | print(bin(return_num())[2:]) # FURB116 (no autofix)
|
= help: Replace with `f"{1337:b}"`
Safe fix
9 9 |
10 10 | print(oct(1337)[2:]) # FURB116
11 11 | print(hex(1337)[2:]) # FURB116
12 |-print(bin(1337)[2:]) # FURB116
12 |+print(f"{1337:b}") # FURB116
13 13 |
14 14 | print(bin(return_num())[2:]) # FURB116 (no autofix)
15 15 | print(bin(int(f"{num}"))[2:]) # FURB116 (no autofix)
FURB116.py:14:7: FURB116 Replace `bin` call with f-string
|
12 | print(bin(1337)[2:]) # FURB116
13 |
14 | print(bin(return_num())[2:]) # FURB116 (no autofix)
| ^^^^^^^^^^^^^^^^^^^^^ FURB116
15 | print(bin(int(f"{num}"))[2:]) # FURB116 (no autofix)
|
= help: Replace with f-string
FURB116.py:15:7: FURB116 Replace `bin` call with f-string
|
14 | print(bin(return_num())[2:]) # FURB116 (no autofix)
15 | print(bin(int(f"{num}"))[2:]) # FURB116 (no autofix)
| ^^^^^^^^^^^^^^^^^^^^^^ FURB116
16 |
17 | ## invalid
|
= help: Replace with f-string

1
ruff.schema.json generated
View file

@ -3047,6 +3047,7 @@
"FURB11", "FURB11",
"FURB110", "FURB110",
"FURB113", "FURB113",
"FURB116",
"FURB118", "FURB118",
"FURB12", "FURB12",
"FURB129", "FURB129",