Skip to content

Commit

Permalink
[ruff] Unnecessary cast to int (RUF046)
Browse files Browse the repository at this point in the history
  • Loading branch information
InSyncWithFoo committed Dec 2, 2024
1 parent 83651de commit 5a270e4
Show file tree
Hide file tree
Showing 9 changed files with 672 additions and 1 deletion.
50 changes: 50 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import math


### Safely fixable

# Arguments are not checked
int(id())
int(len([]))
int(ord(foo))
int(hash(foo, bar))
int(int(''))

int(math.comb())
int(math.factorial())
int(math.gcd())
int(math.lcm())
int(math.isqrt())
int(math.perm())


### Unsafe

int(math.ceil())
int(math.floor())
int(math.trunc())


### `round()`

## Errors
int(round(0))
int(round(0, 0))
int(round(0, None))

int(round(0.1))
int(round(0.1, 0))
int(round(0.1, None))

# Argument type is not checked
foo = type("Foo", (), {"__round__": lambda self: 4.2})()

int(round(foo))
int(round(foo, 0))
int(round(foo, None))

## No errors
int(round(0, 3.14))
int(round(0, non_literal))
int(round(0, 0), base)
int(round(0, 0, extra=keyword))
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 @@ -1093,6 +1093,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::Airflow3Removal) {
airflow::rules::removed_in_3(checker, expr);
}
if checker.enabled(Rule::UnnecessaryCastToInt) {
ruff::rules::unnecessary_cast_to_int(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 @@ -983,6 +983,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "039") => (RuleGroup::Preview, rules::ruff::rules::UnrawRePattern),
(Ruff, "040") => (RuleGroup::Preview, rules::ruff::rules::InvalidAssertMessageLiteralArgument),
(Ruff, "041") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryNestedLiteral),
(Ruff, "046") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryCastToInt),
(Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing),
(Ruff, "055") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRegularExpression),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ mod tests {
#[test_case(Rule::UnrawRePattern, Path::new("RUF039.py"))]
#[test_case(Rule::UnrawRePattern, Path::new("RUF039_concat.py"))]
#[test_case(Rule::UnnecessaryRegularExpression, Path::new("RUF055.py"))]
#[test_case(Rule::UnnecessaryCastToInt, Path::new("RUF046.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub(crate) use sort_dunder_slots::*;
pub(crate) use static_key_dict_comprehension::*;
#[cfg(any(feature = "test-rules", test))]
pub(crate) use test_rules::*;
pub(crate) use unnecessary_cast_to_int::*;
pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
pub(crate) use unnecessary_key_check::*;
pub(crate) use unnecessary_nested_literal::*;
Expand Down Expand Up @@ -77,6 +78,7 @@ mod static_key_dict_comprehension;
mod suppression_comment_visitor;
#[cfg(any(feature = "test-rules", test))]
pub(crate) mod test_rules;
mod unnecessary_cast_to_int;
mod unnecessary_iterable_allocation_for_first_element;
mod unnecessary_key_check;
mod unnecessary_nested_literal;
Expand Down
143 changes: 143 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/unnecessary_cast_to_int.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
use crate::checkers::ast::Checker;
use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::{Arguments, Expr, ExprCall, ExprNumberLiteral, Number};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::TextRange;

/// ## What it does
/// Checks for `int` conversions of values that are already integers.
///
/// ## Why is this bad?
/// Such a conversion is unnecessary.
///
/// ## Known problems
/// This rule is prone to false positives due to type inference limitations.
///
/// ## Example
///
/// ```python
/// int(len([]))
/// int(round(foo, 0))
/// ```
///
/// Use instead:
///
/// ```python
/// len([])
/// round(foo)
/// ```
#[derive(ViolationMetadata)]
pub(crate) struct UnnecessaryCastToInt;

impl AlwaysFixableViolation for UnnecessaryCastToInt {
#[derive_message_formats]
fn message(&self) -> String {
"Value being casted is already an integer".to_string()
}

fn fix_title(&self) -> String {
"Remove `int()` wrapper call".to_string()
}
}

/// RUF046
pub(crate) fn unnecessary_cast_to_int(checker: &mut Checker, call: &ExprCall) {
let semantic = checker.semantic();

let Some(Expr::Call(inner_call)) = single_argument_to_int_call(semantic, call) else {
return;
};

let (func, arguments) = (&inner_call.func, &inner_call.arguments);
let (outer_range, inner_range) = (call.range, inner_call.range);

let Some(qualified_name) = checker.semantic().resolve_qualified_name(func) else {
return;
};

let (edit, applicability) = match qualified_name.segments() {
// Always returns a strict instance of `int`
["" | "builtins", "len" | "id" | "hash" | "ord" | "int"]
| ["math", "comb" | "factorial" | "gcd" | "lcm" | "isqrt" | "perm"] => (
handle_other(checker, outer_range, inner_range),
Applicability::Safe,
),

// Depends on `ndigits` and `number.__round__`
["" | "builtins", "round"] => match handle_round(checker, outer_range, arguments) {
None => return,
Some(edit) => (edit, Applicability::Unsafe),
},

// Depends on `__ceil__`/`__floor__`/`__trunc__`
["math", "ceil" | "floor" | "trunc"] => (
handle_other(checker, outer_range, inner_range),
Applicability::Unsafe,
),

_ => return,
};

let diagnostic = Diagnostic::new(UnnecessaryCastToInt {}, call.range);
let fix = Fix::applicable_edit(edit, applicability);

checker.diagnostics.push(diagnostic.with_fix(fix));
}

fn single_argument_to_int_call<'a>(
semantic: &SemanticModel,
call: &'a ExprCall,
) -> Option<&'a Expr> {
let ExprCall {
func, arguments, ..
} = call;

if !semantic.match_builtin_expr(func, "int") {
return None;
}

if !arguments.keywords.is_empty() {
return None;
}

let [argument] = &*arguments.args else {
return None;
};

Some(argument)
}

fn handle_round(checker: &Checker, outer_range: TextRange, arguments: &Arguments) -> Option<Edit> {
if arguments.len() > 2 {
return None;
}

let number = arguments.find_argument("number", 0)?;
let ndigits = arguments.find_argument("ndigits", 1);

let number_expr = checker.locator().slice(number);
let new_content = match ndigits {
Some(Expr::NumberLiteral(ExprNumberLiteral { value, .. })) if is_literal_zero(value) => {
format!("round({})", number_expr)
}
Some(Expr::NoneLiteral(_)) | None => format!("round({})", number_expr),
_ => return None,
};

Some(Edit::range_replacement(new_content, outer_range))
}

fn is_literal_zero(value: &Number) -> bool {
let Number::Int(int) = value else {
return false;
};

matches!(int.as_u8(), Some(0))
}

fn handle_other(checker: &Checker, outer_range: TextRange, inner_range: TextRange) -> Edit {
let inner_expr = checker.locator().slice(inner_range);

Edit::range_replacement(inner_expr.to_string(), outer_range)
}
Loading

0 comments on commit 5a270e4

Please sign in to comment.