diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB116.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB116.py new file mode 100644 index 0000000000..9c968e32aa --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB116.py @@ -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:]) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 931166653b..36a9b21e8f 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -128,6 +128,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::SortedMinMax) { 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); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index c54e8cf732..b4c7ce0184 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1050,6 +1050,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Refurb, "110") => (RuleGroup::Preview, rules::refurb::rules::IfExpInsteadOfOrOperator), #[allow(deprecated)] (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, "129") => (RuleGroup::Preview, rules::refurb::rules::ReadlinesInFor), #[allow(deprecated)] diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index 46445912db..19b17dfaa4 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -42,6 +42,7 @@ mod tests { #[test_case(Rule::HashlibDigestHex, Path::new("FURB181.py"))] #[test_case(Rule::ListReverseCopy, Path::new("FURB187.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"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/refurb/rules/fstring_number_format.rs b/crates/ruff_linter/src/rules/refurb/rules/fstring_number_format.rs new file mode 100644 index 0000000000..11f0573969 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/fstring_number_format.rs @@ -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, + 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 { + 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 { + match s { + "hex" => Some(Base::Hex), + "bin" => Some(Base::Bin), + "oct" => Some(Base::Oct), + _ => None, + } + } +} diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index 91083264d9..9cfd2c9ff9 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -2,6 +2,7 @@ pub(crate) use bit_count::*; pub(crate) use check_and_remove_from_set::*; pub(crate) use delete_full_slice::*; pub(crate) use for_loop_set_mutations::*; +pub(crate) use fstring_number_format::*; pub(crate) use hashlib_digest_hex::*; pub(crate) use if_exp_instead_of_or_operator::*; pub(crate) use if_expr_min_max::*; @@ -32,6 +33,7 @@ mod bit_count; mod check_and_remove_from_set; mod delete_full_slice; mod for_loop_set_mutations; +mod fstring_number_format; mod hashlib_digest_hex; mod if_exp_instead_of_or_operator; mod if_expr_min_max; diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB116_FURB116.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB116_FURB116.py.snap new file mode 100644 index 0000000000..02e102a7ba --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB116_FURB116.py.snap @@ -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 diff --git a/ruff.schema.json b/ruff.schema.json index ac21bdf992..343ace0f2a 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3047,6 +3047,7 @@ "FURB11", "FURB110", "FURB113", + "FURB116", "FURB118", "FURB12", "FURB129",