Skip to content

Commit

Permalink
[ruff] Detect more strict-integer expressions (RUF046) (#14833)
Browse files Browse the repository at this point in the history 
Co-authored-by: Micha Reiser <[email protected]>
  • Loading branch information
@InSyncWithFoo @MichaReiser
InSyncWithFoo and MichaReiser authored Dec 21, 2024
1 parent 000948a commit bd023c4
Show file tree
Hide file tree
Showing 4 changed files with 719 additions and 203 deletions.
86 changes: 86 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF046.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,33 @@
int(round(1.))
int(round(1., None))

int(1)
int(v := 1)
int(~1)
int(-1)
int(+1)

int(1 + 1)
int(1 - 1)
int(1 * 1)
int(1 % 1)
int(1 ** 1)
int(1 << 1)
int(1 >> 1)
int(1 | 1)
int(1 ^ 1)
int(1 & 1)
int(1 // 1)

int(1 if ... else 2)

int(1 and 0)
int(0 or -1)


if int(1 + 2) * 3:
...


### Unsafe

Expand Down Expand Up @@ -68,3 +95,62 @@

int(round(0, 0), base)
int(round(0, 0, extra=keyword))

int(foo if ... else 4)

int(3.14)
int(2.8j)

async def f():
int(await f())

int(foo.bar)
int(bar([1][False]))

int(1 == 1)
int(1 != 1)
int(1 < 1)
int(1 <= 1)
int(1 > 1)
int(1 >= 1)
int(1 in 1)
int(1 not in 1)
int(1 is 1)
int(2 is not 3)
int(foo in 1)
int(foo not in 1)
int(foo is 1)
int(foo is not 1)

int(1 == 2 == 3)
int(foo == 1)
int(foo != 1)
int(foo < 1)
int(foo <= 1)
int(foo > 1)
int(foo >= 1)

int(v := {}[{}['']])

int(foo + 1)
int(foo - 1)
int(foo * 1)
int(foo @ 1)
int(foo / 1)
int(foo % 1)
int(foo ** 1)
int(foo << 1)
int(foo >> 1)
int(foo | 1)
int(foo ^ 1)
int(foo & 1)
int(foo // 1)

int(v := 3.7)

int(not 109)

int(1 / 1)
int(1 @ 1)

int(1. if ... else .2)
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ pub(crate) use pytest_raises_ambiguous_pattern::*;
pub(crate) use quadratic_list_summation::*;
pub(crate) use redirected_noqa::*;
pub(crate) use redundant_bool_literal::*;
pub(crate) use round_applicability::*;
pub(crate) use sort_dunder_all::*;
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 @@ -76,14 +76,14 @@ mod pytest_raises_ambiguous_pattern;
mod quadratic_list_summation;
mod redirected_noqa;
mod redundant_bool_literal;
mod round_applicability;
mod sequence_sorting;
mod sort_dunder_all;
mod sort_dunder_slots;
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
107 changes: 63 additions & 44 deletions ...les/ruff/rules/unnecessary_cast_to_int.rs → ...c/rules/ruff/rules/round_applicability.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ 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::analyze::type_inference::{NumberLike, PythonType, ResolvedPythonType};
use ruff_python_semantic::analyze::typing;
use ruff_python_semantic::SemanticModel;
use ruff_text_size::TextRange;
use ruff_text_size::Ranged;

/// ## What it does
/// Checks for `int` conversions of values that are already integers.
Expand Down Expand Up @@ -54,44 +55,74 @@ impl AlwaysFixableViolation for UnnecessaryCastToInt {
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 {
let Some(argument) = 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 applicability = if matches!(
ResolvedPythonType::from(argument),
ResolvedPythonType::Atom(PythonType::Number(NumberLike::Integer))
) {
Some(Applicability::Safe)
} else if let Expr::Call(inner_call) = argument {
call_applicability(checker, inner_call)
} else {
None
};

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

let fix = match qualified_name.segments() {
let fix = unwrap_int_expression(checker, call, argument, applicability);
let diagnostic = Diagnostic::new(UnnecessaryCastToInt, call.range);
checker.diagnostics.push(diagnostic.with_fix(fix));
}

/// Creates a fix that replaces `int(expression)` with `expression`.
fn unwrap_int_expression(
checker: &mut Checker,
call: &ExprCall,
argument: &Expr,
applicability: Applicability,
) -> Fix {
let (locator, semantic) = (checker.locator(), checker.semantic());

let argument_expr = locator.slice(argument.range());

let has_parent_expr = semantic.current_expression_parent().is_some();
let new_content = if has_parent_expr || argument.is_named_expr() {
format!("({argument_expr})")
} else {
argument_expr.to_string()
};

let edit = Edit::range_replacement(new_content, call.range);
Fix::applicable_edit(edit, applicability)
}

/// Returns `Some` if `call` in `int(call(..))` is a method that returns an `int` and `None`
/// otherwise.
fn call_applicability(checker: &mut Checker, inner_call: &ExprCall) -> Option<Applicability> {
let (func, arguments) = (&inner_call.func, &inner_call.arguments);

let qualified_name = checker.semantic().resolve_qualified_name(func)?;

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

// Depends on `ndigits` and `number.__round__`
["" | "builtins", "round"] => {
if let Some(fix) = replace_with_round(checker, outer_range, inner_range, arguments) {
fix
} else {
return;
}
}
["" | "builtins", "round"] => replace_with_round(checker, arguments),

// Depends on `__ceil__`/`__floor__`/`__trunc__`
["math", "ceil" | "floor" | "trunc"] => {
Fix::unsafe_edit(replace_with_inner(checker, outer_range, inner_range))
}

_ => return,
};

let diagnostic = Diagnostic::new(UnnecessaryCastToInt, call.range);
["math", "ceil" | "floor" | "trunc"] => Some(Applicability::Unsafe),

checker.diagnostics.push(diagnostic.with_fix(fix));
_ => None,
}
}

fn single_argument_to_int_call<'a>(
Expand Down Expand Up @@ -136,12 +167,10 @@ enum Ndigits {
Other,
}

fn replace_with_round(
checker: &Checker,
outer_range: TextRange,
inner_range: TextRange,
arguments: &Arguments,
) -> Option<Fix> {
/// Determines the [`Applicability`] for a `round(..)` call.
///
/// The Applicability depends on the `ndigits` and the number argument.
fn replace_with_round(checker: &Checker, arguments: &Arguments) -> Option<Applicability> {
if arguments.len() > 2 {
return None;
}
Expand Down Expand Up @@ -181,28 +210,18 @@ fn replace_with_round(
_ => Ndigits::Other,
};

let applicability = match (number_kind, ndigits_kind) {
match (number_kind, ndigits_kind) {
(Rounded::LiteralInt, Ndigits::LiteralInt)
| (Rounded::LiteralInt | Rounded::LiteralFloat, Ndigits::NotGiven | Ndigits::LiteralNone) => {
Applicability::Safe
Some(Applicability::Safe)
}

(Rounded::InferredInt, Ndigits::LiteralInt)
| (
Rounded::InferredInt | Rounded::InferredFloat | Rounded::Other,
Ndigits::NotGiven | Ndigits::LiteralNone,
) => Applicability::Unsafe,
) => Some(Applicability::Unsafe),

_ => return None,
};

let edit = replace_with_inner(checker, outer_range, inner_range);

Some(Fix::applicable_edit(edit, applicability))
}

fn replace_with_inner(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)
_ => None,
}
}
Loading

0 comments on commit bd023c4

Please sign in to comment.