Skip to content

Commit

Permalink
[refurb] Implement fstring-number-format (FURB116) (#10921)
Browse files Browse the repository at this point in the history
## Summary

Adds `FURB116`

See #1348 

## Test Plan

`cargo test`
  • Loading branch information
diceroll123 authored Apr 26, 2024
1 parent b15e9e6 commit c8c227d
Show file tree
Hide file tree
Showing 8 changed files with 352 additions and 0 deletions.
19 changes: 19 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB116.py
Original file line number Diff line number Diff line change
@@ -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:])
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
181 changes: 181 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/fstring_number_format.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c8c227d

Please sign in to comment.