Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refurb] Implement int-on-sliced-str (FURB166) #10650

Merged
merged 3 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 29 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB166.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Errors

_ = int("0b1010"[2:], 2)
_ = int("0o777"[2:], 8)
_ = int("0xFFFF"[2:], 16)

b = "0b11"
_ = int(b[2:], 2)

_ = int("0xFFFF"[2:], base=16)

_ = int(b"0xFFFF"[2:], 16)


def get_str():
return "0xFFF"


_ = int(get_str()[2:], 16)

# Ok

_ = int("0b1100", 0)
_ = int("123", 3)
_ = int("123", 10)
_ = int("0b1010"[3:], 2)
_ = int("0b1010"[:2], 2)
_ = int("12345"[2:])
_ = int("12345"[2:], xyz=1) # type: ignore
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 @@ -974,6 +974,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::UnnecessaryIterableAllocationForFirstElement) {
ruff::rules::unnecessary_iterable_allocation_for_first_element(checker, expr);
}
if checker.enabled(Rule::IntOnSlicedStr) {
refurb::rules::int_on_sliced_str(checker, call);
}
}
Expr::Dict(dict) => {
if checker.any_enabled(&[
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 @@ -1051,6 +1051,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Refurb, "157") => (RuleGroup::Preview, rules::refurb::rules::VerboseDecimalConstructor),
(Refurb, "161") => (RuleGroup::Preview, rules::refurb::rules::BitCount),
(Refurb, "163") => (RuleGroup::Preview, rules::refurb::rules::RedundantLogBase),
(Refurb, "166") => (RuleGroup::Preview, rules::refurb::rules::IntOnSlicedStr),
(Refurb, "167") => (RuleGroup::Preview, rules::refurb::rules::RegexFlagAlias),
(Refurb, "168") => (RuleGroup::Preview, rules::refurb::rules::IsinstanceTypeNone),
(Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison),
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 @@ -31,6 +31,7 @@ mod tests {
#[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))]
#[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))]
#[test_case(Rule::BitCount, Path::new("FURB161.py"))]
#[test_case(Rule::IntOnSlicedStr, Path::new("FURB166.py"))]
#[test_case(Rule::RegexFlagAlias, Path::new("FURB167.py"))]
#[test_case(Rule::IsinstanceTypeNone, Path::new("FURB168.py"))]
#[test_case(Rule::TypeNoneComparison, Path::new("FURB169.py"))]
Expand Down
118 changes: 118 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/int_on_sliced_str.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprCall, Identifier};

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.
///
/// ## 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.
///
/// ## Example
/// ```python
/// num = "0xABC"
///
/// if num.startswith("0b"):
/// i = int(num[2:], 2)
/// elif num.startswith("0o"):
/// i = int(num[2:], 8)
/// elif num.startswith("0x"):
/// i = int(num[2:], 16)
///
/// print(i)
/// ```
///
/// Use instead:
/// ```python
/// num = "0xABC"
///
/// i = int(num, 0)
///
/// print(i)
/// ```
///
/// ## 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.
/// ## References
/// - [Python documentation: `int`](https://docs.python.org/3/library/functions.html#int)
#[violation]
pub struct IntOnSlicedStr;

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

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

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"]))
{
return;
}
let (arg, base_keyword, 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)
}
([arg, base_arg], []) => (arg, "", base_arg),
_ => {
return;
}
};
if !base
.as_number_literal_expr()
.and_then(|base| base.value.as_int())
.is_some_and(|base| matches!(base.as_u8(), Some(2 | 8 | 16)))
{
return;
};

let Expr::Subscript(expr_subscript) = arg else {
return;
};
let Expr::Slice(expr_slice) = expr_subscript.slice.as_ref() else {
return;
};
if expr_slice.upper.is_some() || expr_slice.step.is_some() {
return;
}
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))
{
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,
),
)),
);
}
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 @@ -5,6 +5,7 @@ pub(crate) use for_loop_set_mutations::*;
pub(crate) use hashlib_digest_hex::*;
pub(crate) use if_expr_min_max::*;
pub(crate) use implicit_cwd::*;
pub(crate) use int_on_sliced_str::*;
pub(crate) use isinstance_type_none::*;
pub(crate) use list_reverse_copy::*;
pub(crate) use math_constant::*;
Expand All @@ -30,6 +31,7 @@ mod for_loop_set_mutations;
mod hashlib_digest_hex;
mod if_expr_min_max;
mod implicit_cwd;
mod int_on_sliced_str;
mod isinstance_type_none;
mod list_reverse_copy;
mod math_constant;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
---
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
|
1 | # Errors
2 |
3 | _ = int("0b1010"[2:], 2)
| ^^^^^^^^^^^^^^^^^^^^ FURB166
4 | _ = int("0o777"[2:], 8)
5 | _ = int("0xFFFF"[2:], 16)
|
= help: Use `int` with `base` 0 instead

ℹ Unsafe fix
1 1 | # Errors
2 2 |
3 |-_ = int("0b1010"[2:], 2)
3 |+_ = int("0b1010", 0)
4 4 | _ = int("0o777"[2:], 8)
5 5 | _ = int("0xFFFF"[2:], 16)
6 6 |

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

ℹ Unsafe fix
1 1 | # Errors
2 2 |
3 3 | _ = int("0b1010"[2:], 2)
4 |-_ = int("0o777"[2:], 8)
4 |+_ = int("0o777", 0)
5 5 | _ = int("0xFFFF"[2:], 16)
6 6 |
7 7 | b = "0b11"

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

ℹ Unsafe fix
2 2 |
3 3 | _ = int("0b1010"[2:], 2)
4 4 | _ = int("0o777"[2:], 8)
5 |-_ = int("0xFFFF"[2:], 16)
5 |+_ = int("0xFFFF", 0)
6 6 |
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
|
7 | b = "0b11"
8 | _ = int(b[2:], 2)
| ^^^^^^^^^^^^^ FURB166
9 |
10 | _ = int("0xFFFF"[2:], base=16)
|
= help: Use `int` with `base` 0 instead

ℹ Unsafe fix
5 5 | _ = int("0xFFFF"[2:], 16)
6 6 |
7 7 | b = "0b11"
8 |-_ = int(b[2:], 2)
8 |+_ = int(b, 0)
9 9 |
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
|
8 | _ = int(b[2:], 2)
9 |
10 | _ = int("0xFFFF"[2:], base=16)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB166
11 |
12 | _ = int(b"0xFFFF"[2:], 16)
|
= help: Use `int` with `base` 0 instead

ℹ Unsafe fix
7 7 | b = "0b11"
8 8 | _ = int(b[2:], 2)
9 9 |
10 |-_ = int("0xFFFF"[2:], base=16)
10 |+_ = int("0xFFFF", base=0)
11 11 |
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
|
10 | _ = int("0xFFFF"[2:], base=16)
11 |
12 | _ = int(b"0xFFFF"[2:], 16)
| ^^^^^^^^^^^^^^^^^^^^^^ FURB166
|
= help: Use `int` with `base` 0 instead

ℹ Unsafe fix
9 9 |
10 10 | _ = int("0xFFFF"[2:], base=16)
11 11 |
12 |-_ = int(b"0xFFFF"[2:], 16)
12 |+_ = int(b"0xFFFF", 0)
13 13 |
14 14 |
15 15 | def get_str():

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

ℹ Unsafe fix
16 16 | return "0xFFF"
17 17 |
18 18 |
19 |-_ = int(get_str()[2:], 16)
19 |+_ = int(get_str(), 0)
20 20 |
21 21 | # Ok
22 22 |
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.