Skip to content

Commit

Permalink
Minor tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Apr 2, 2024
1 parent 3e756e1 commit ea537d1
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 54 deletions.
96 changes: 56 additions & 40 deletions crates/ruff_linter/src/rules/refurb/rules/int_on_sliced_str.rs
View file
@@ -1,16 +1,21 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprCall, Identifier};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for uses of converting a string starting with `0b`, `0o`, or `0x` to an int, by removing
/// two first symbols and explicitly set the base.
/// Checks for uses of `int` with an explicit base in which a string expression
/// is stripped of its leading prefix (i.e., `0b`, `0o`, or `0x`).
///
/// ## Why is this bad?
/// Rather than set the base explicitly, call the `int` with the base of zero, and let automatically
/// deduce the base by the prefix.
/// Given an integer string with a prefix (e.g., `0xABC`), Python can automatically
/// determine the base of the integer by the prefix without needing to specify
/// it explicitly.
///
/// Instead of `int(num[2:], 16)`, use `int(num, 0)`, which will automatically
/// deduce the base based on the prefix.
///
/// ## Example
/// ```python
Expand All @@ -36,55 +41,69 @@ use crate::checkers::ast::Checker;
/// ```
///
/// ## Fix safety
/// The rule's fix is marked as unsafe, because there is no way for `ruff` to detect whether
/// the stripped prefix was a valid Python `int` prefix.
/// The rule's fix is marked as unsafe, as Ruff cannot guarantee that the
/// argument to `int` will remain valid when its base is included in the
/// function call.
///
/// ## References
/// - [Python documentation: `int`](https://docs.python.org/3/library/functions.html#int)
#[violation]
pub struct IntOnSlicedStr;
pub struct IntOnSlicedStr {
base: u8,
}

impl AlwaysFixableViolation for IntOnSlicedStr {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use of `int` with the explicit `base` after removing the prefix")
let IntOnSlicedStr { base } = self;
format!("Use of `int` with explicit `base={base}` after removing prefix")
}

fn fix_title(&self) -> String {
format!("Use `int` with `base` 0 instead")
format!("Replace with `base=0`")
}
}

pub(crate) fn int_on_sliced_str(checker: &mut Checker, call: &ExprCall) {
if !checker
.semantic()
.resolve_qualified_name(call.func.as_ref())
.is_some_and(|name| matches!(name.segments(), ["" | "builtins", "int"]))
{
// Verify that the function is `int`.
let Expr::Name(name) = call.func.as_ref() else {
return;
};
if name.id.as_str() != "int" {
return;
}
if !checker.semantic().is_builtin("int") {
return;
}
let (arg, base_keyword, base) = match (

// There must be exactly two arguments (e.g., `int(num[2:], 16)`).
let (expression, base) = match (
call.arguments.args.as_ref(),
call.arguments.keywords.as_ref(),
) {
([arg], [base_kw_arg])
if base_kw_arg.arg.as_ref().map(Identifier::as_str) == Some("base") =>
{
(arg, "base=", &base_kw_arg.value)
([expression], [base]) if base.arg.as_ref().map(Identifier::as_str) == Some("base") => {
(expression, &base.value)
}
([arg, base_arg], []) => (arg, "", base_arg),
([expression, base], []) => (expression, base),
_ => {
return;
}
};
if !base

// The base must be a number literal with a value of 2, 8, or 16.
let Some(base_u8) = base
.as_number_literal_expr()
.and_then(|base| base.value.as_int())
.is_some_and(|base| matches!(base.as_u8(), Some(2 | 8 | 16)))
{
.and_then(|base| base.as_u8())
else {
return;
};
if !matches!(base_u8, 2 | 8 | 16) {
return;
}

let Expr::Subscript(expr_subscript) = arg else {
// Determine whether the expression is a slice of a string (e.g., `num[2:]`).
let Expr::Subscript(expr_subscript) = expression else {
return;
};
let Expr::Slice(expr_slice) = expr_subscript.slice.as_ref() else {
Expand All @@ -96,23 +115,20 @@ pub(crate) fn int_on_sliced_str(checker: &mut Checker, call: &ExprCall) {
if !expr_slice
.lower
.as_ref()
.and_then(|x| x.as_number_literal_expr())
.and_then(|x| x.value.as_int())
.is_some_and(|x| x.as_u8() == Some(2))
.and_then(|expr| expr.as_number_literal_expr())
.and_then(|expr| expr.value.as_int())
.is_some_and(|expr| expr.as_u8() == Some(2))
{
return;
}
checker
.diagnostics
.push(
Diagnostic::new(IntOnSlicedStr, call.range).with_fix(Fix::unsafe_edit(
Edit::range_replacement(
format!(
"int({}, {base_keyword}0)",
checker.locator().slice(expr_subscript.value.as_ref())
),
call.range,
),
)),
);

let mut diagnostic = Diagnostic::new(IntOnSlicedStr { base: base_u8 }, call.range());
diagnostic.set_fix(Fix::unsafe_edits(
Edit::range_replacement(
checker.locator().slice(&*expr_subscript.value).to_string(),
expression.range(),
),
[Edit::range_replacement("0".to_string(), base.range())],
));
checker.diagnostics.push(diagnostic);
}
28 changes: 14 additions & 14 deletions ...ter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB166_FURB166.py.snap
View file
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
---
FURB166.py:3:5: FURB166 [*] Use of `int` with the explicit `base` after removing the prefix
FURB166.py:3:5: FURB166 [*] Use of `int` with explicit `base=2` after removing prefix
|
1 | # Errors
2 |
Expand All @@ -10,7 +10,7 @@ FURB166.py:3:5: FURB166 [*] Use of `int` with the explicit `base` after removing
4 | _ = int("0o777"[2:], 8)
5 | _ = int("0xFFFF"[2:], 16)
|
= help: Use `int` with `base` 0 instead
= help: Replace with `base=0`

ℹ Unsafe fix
1 1 | # Errors
Expand All @@ -21,14 +21,14 @@ FURB166.py:3:5: FURB166 [*] Use of `int` with the explicit `base` after removing
5 5 | _ = int("0xFFFF"[2:], 16)
6 6 |

FURB166.py:4:5: FURB166 [*] Use of `int` with the explicit `base` after removing the prefix
FURB166.py:4:5: FURB166 [*] Use of `int` with explicit `base=8` after removing prefix
|
3 | _ = int("0b1010"[2:], 2)
4 | _ = int("0o777"[2:], 8)
| ^^^^^^^^^^^^^^^^^^^ FURB166
5 | _ = int("0xFFFF"[2:], 16)
|
= help: Use `int` with `base` 0 instead
= help: Replace with `base=0`

ℹ Unsafe fix
1 1 | # Errors
Expand All @@ -40,7 +40,7 @@ FURB166.py:4:5: FURB166 [*] Use of `int` with the explicit `base` after removing
6 6 |
7 7 | b = "0b11"

FURB166.py:5:5: FURB166 [*] Use of `int` with the explicit `base` after removing the prefix
FURB166.py:5:5: FURB166 [*] Use of `int` with explicit `base=16` after removing prefix
|
3 | _ = int("0b1010"[2:], 2)
4 | _ = int("0o777"[2:], 8)
Expand All @@ -49,7 +49,7 @@ FURB166.py:5:5: FURB166 [*] Use of `int` with the explicit `base` after removing
6 |
7 | b = "0b11"
|
= help: Use `int` with `base` 0 instead
= help: Replace with `base=0`

ℹ Unsafe fix
2 2 |
Expand All @@ -61,15 +61,15 @@ FURB166.py:5:5: FURB166 [*] Use of `int` with the explicit `base` after removing
7 7 | b = "0b11"
8 8 | _ = int(b[2:], 2)

FURB166.py:8:5: FURB166 [*] Use of `int` with the explicit `base` after removing the prefix
FURB166.py:8:5: FURB166 [*] Use of `int` with explicit `base=2` after removing prefix
|
7 | b = "0b11"
8 | _ = int(b[2:], 2)
| ^^^^^^^^^^^^^ FURB166
9 |
10 | _ = int("0xFFFF"[2:], base=16)
|
= help: Use `int` with `base` 0 instead
= help: Replace with `base=0`

ℹ Unsafe fix
5 5 | _ = int("0xFFFF"[2:], 16)
Expand All @@ -81,7 +81,7 @@ FURB166.py:8:5: FURB166 [*] Use of `int` with the explicit `base` after removing
10 10 | _ = int("0xFFFF"[2:], base=16)
11 11 |

FURB166.py:10:5: FURB166 [*] Use of `int` with the explicit `base` after removing the prefix
FURB166.py:10:5: FURB166 [*] Use of `int` with explicit `base=16` after removing prefix
|
8 | _ = int(b[2:], 2)
9 |
Expand All @@ -90,7 +90,7 @@ FURB166.py:10:5: FURB166 [*] Use of `int` with the explicit `base` after removin
11 |
12 | _ = int(b"0xFFFF"[2:], 16)
|
= help: Use `int` with `base` 0 instead
= help: Replace with `base=0`

ℹ Unsafe fix
7 7 | b = "0b11"
Expand All @@ -102,14 +102,14 @@ FURB166.py:10:5: FURB166 [*] Use of `int` with the explicit `base` after removin
12 12 | _ = int(b"0xFFFF"[2:], 16)
13 13 |

FURB166.py:12:5: FURB166 [*] Use of `int` with the explicit `base` after removing the prefix
FURB166.py:12:5: FURB166 [*] Use of `int` with explicit `base=16` after removing prefix
|
10 | _ = int("0xFFFF"[2:], base=16)
11 |
12 | _ = int(b"0xFFFF"[2:], 16)
| ^^^^^^^^^^^^^^^^^^^^^^ FURB166
|
= help: Use `int` with `base` 0 instead
= help: Replace with `base=0`

ℹ Unsafe fix
9 9 |
Expand All @@ -121,14 +121,14 @@ FURB166.py:12:5: FURB166 [*] Use of `int` with the explicit `base` after removin
14 14 |
15 15 | def get_str():

FURB166.py:19:5: FURB166 [*] Use of `int` with the explicit `base` after removing the prefix
FURB166.py:19:5: FURB166 [*] Use of `int` with explicit `base=16` after removing prefix
|
19 | _ = int(get_str()[2:], 16)
| ^^^^^^^^^^^^^^^^^^^^^^ FURB166
20 |
21 | # Ok
|
= help: Use `int` with `base` 0 instead
= help: Replace with `base=0`

ℹ Unsafe fix
16 16 | return "0xFFF"
Expand Down

0 comments on commit ea537d1

Please sign in to comment.