diff --git a/crates/ruff/src/rules/pylint/mod.rs b/crates/ruff/src/rules/pylint/mod.rs index 523eb62e8c9425..c3ac58c2fd7ce5 100644 --- a/crates/ruff/src/rules/pylint/mod.rs +++ b/crates/ruff/src/rules/pylint/mod.rs @@ -113,6 +113,19 @@ mod tests { Ok(()) } + #[test] + fn repeated_isinstance_calls() -> Result<()> { + let diagnostics = test_path( + Path::new("pylint/repeated_isinstance_calls.py"), + &Settings { + target_version: PythonVersion::Py39, + ..Settings::for_rules(vec![Rule::RepeatedIsinstanceCalls]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } + #[test] fn continue_in_finally() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff/src/rules/pylint/rules/repeated_isinstance_calls.rs b/crates/ruff/src/rules/pylint/rules/repeated_isinstance_calls.rs index 002d31a0718157..031eee89d25a32 100644 --- a/crates/ruff/src/rules/pylint/rules/repeated_isinstance_calls.rs +++ b/crates/ruff/src/rules/pylint/rules/repeated_isinstance_calls.rs @@ -2,11 +2,13 @@ use itertools::Itertools; use rustc_hash::{FxHashMap, FxHashSet}; use rustpython_parser::ast::{self, Boolop, Expr, Ranged}; -use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::hashable::HashableExpr; use crate::checkers::ast::Checker; +use crate::registry::AsRule; +use crate::settings::types::PythonVersion; /// ## What it does /// Checks for repeated `isinstance` calls on the same object. @@ -35,19 +37,22 @@ use crate::checkers::ast::Checker; /// ``` /// /// ## References -/// - [Python documentation](https://docs.python.org/3/library/functions.html#isinstance) +/// - [Python documentation: `isinstance`](https://docs.python.org/3/library/functions.html#isinstance) #[violation] pub struct RepeatedIsinstanceCalls { - obj: String, - types: Vec, + expr: String, } -impl Violation for RepeatedIsinstanceCalls { +impl AlwaysAutofixableViolation for RepeatedIsinstanceCalls { #[derive_message_formats] fn message(&self) -> String { - let RepeatedIsinstanceCalls { obj, types } = self; - let types = types.join(", "); - format!("Merge these isinstance calls: `isinstance({obj}, ({types}))`") + let RepeatedIsinstanceCalls { expr } = self; + format!("Merge `isinstance` calls: `{expr}`") + } + + fn autofix_title(&self) -> String { + let RepeatedIsinstanceCalls { expr } = self; + format!("Replace with `{expr}`") } } @@ -58,7 +63,7 @@ pub(crate) fn repeated_isinstance_calls( op: Boolop, values: &[Expr], ) { - if !matches!(op, Boolop::Or) || !checker.semantic_model().is_builtin("isinstance") { + if !op.is_or() { return; } @@ -74,6 +79,9 @@ pub(crate) fn repeated_isinstance_calls( let [obj, types] = &args[..] else { continue; }; + if !checker.semantic_model().is_builtin("isinstance") { + return; + } let (num_calls, matches) = obj_to_types .entry(obj.into()) .or_insert_with(|| (0, FxHashSet::default())); @@ -91,18 +99,33 @@ pub(crate) fn repeated_isinstance_calls( for (obj, (num_calls, types)) in obj_to_types { if num_calls > 1 && types.len() > 1 { - checker.diagnostics.push(Diagnostic::new( - RepeatedIsinstanceCalls { - obj: checker.generator().expr(obj.as_expr()), - types: types - .iter() - .map(HashableExpr::as_expr) - .map(|expr| checker.generator().expr(expr)) - .sorted() - .collect(), - }, - expr.range(), - )); + let call = merged_isinstance_call( + &checker.generator().expr(obj.as_expr()), + types + .iter() + .map(HashableExpr::as_expr) + .map(|expr| checker.generator().expr(expr)) + .sorted(), + checker.settings.target_version, + ); + let mut diagnostic = + Diagnostic::new(RepeatedIsinstanceCalls { expr: call.clone() }, expr.range()); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.set_fix(Fix::automatic(Edit::range_replacement(call, expr.range()))); + } + checker.diagnostics.push(diagnostic); } } } + +fn merged_isinstance_call( + obj: &str, + types: impl IntoIterator, + target_version: PythonVersion, +) -> String { + if target_version >= PythonVersion::Py310 { + format!("isinstance({}, {})", obj, types.into_iter().join(" | ")) + } else { + format!("isinstance({}, ({}))", obj, types.into_iter().join(", ")) + } +} diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1701_repeated_isinstance_calls.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1701_repeated_isinstance_calls.py.snap index 725ccd6cfe7dc3..ca7bf98cb97257 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1701_repeated_isinstance_calls.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1701_repeated_isinstance_calls.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff/src/rules/pylint/mod.rs --- -repeated_isinstance_calls.py:15:8: PLR1701 Merge these isinstance calls: `isinstance(var[3], (float, int))` +repeated_isinstance_calls.py:15:8: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[3], float | int)` | 15 | # not merged 16 | if isinstance(var[3], int) or isinstance(var[3], float) or isinstance(var[3], list) and True: # [consider-merging-isinstance] @@ -9,8 +9,19 @@ repeated_isinstance_calls.py:15:8: PLR1701 Merge these isinstance calls: `isinst 17 | pass 18 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] | + = help: Replace with `isinstance(var[3], float | int)` -repeated_isinstance_calls.py:17:14: PLR1701 Merge these isinstance calls: `isinstance(var[4], (float, int))` +ℹ Fix +12 12 | result = isinstance(var[2], (int, float)) +13 13 | +14 14 | # not merged +15 |- if isinstance(var[3], int) or isinstance(var[3], float) or isinstance(var[3], list) and True: # [consider-merging-isinstance] + 15 |+ if isinstance(var[3], float | int): # [consider-merging-isinstance] +16 16 | pass +17 17 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] +18 18 | + +repeated_isinstance_calls.py:17:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[4], float | int)` | 17 | if isinstance(var[3], int) or isinstance(var[3], float) or isinstance(var[3], list) and True: # [consider-merging-isinstance] 18 | pass @@ -19,8 +30,19 @@ repeated_isinstance_calls.py:17:14: PLR1701 Merge these isinstance calls: `isins 20 | 21 | result = isinstance(var[5], int) or True or isinstance(var[5], float) # [consider-merging-isinstance] | + = help: Replace with `isinstance(var[4], float | int)` + +ℹ Fix +14 14 | # not merged +15 15 | if isinstance(var[3], int) or isinstance(var[3], float) or isinstance(var[3], list) and True: # [consider-merging-isinstance] +16 16 | pass +17 |- result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] + 17 |+ result = isinstance(var[4], float | int) # [consider-merging-isinstance] +18 18 | +19 19 | result = isinstance(var[5], int) or True or isinstance(var[5], float) # [consider-merging-isinstance] +20 20 | -repeated_isinstance_calls.py:19:14: PLR1701 Merge these isinstance calls: `isinstance(var[5], (float, int))` +repeated_isinstance_calls.py:19:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[5], float | int)` | 19 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] 20 | @@ -29,8 +51,19 @@ repeated_isinstance_calls.py:19:14: PLR1701 Merge these isinstance calls: `isins 22 | 23 | inferred_isinstance = isinstance | + = help: Replace with `isinstance(var[5], float | int)` + +ℹ Fix +16 16 | pass +17 17 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] +18 18 | +19 |- result = isinstance(var[5], int) or True or isinstance(var[5], float) # [consider-merging-isinstance] + 19 |+ result = isinstance(var[5], float | int) # [consider-merging-isinstance] +20 20 | +21 21 | inferred_isinstance = isinstance +22 22 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] -repeated_isinstance_calls.py:23:14: PLR1701 Merge these isinstance calls: `isinstance(var[10], (list, str))` +repeated_isinstance_calls.py:23:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[10], list | str)` | 23 | inferred_isinstance = isinstance 24 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] @@ -38,8 +71,19 @@ repeated_isinstance_calls.py:23:14: PLR1701 Merge these isinstance calls: `isins | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701 26 | result = isinstance(var[11], int) or isinstance(var[11], int) or isinstance(var[11], float) # [consider-merging-isinstance] | + = help: Replace with `isinstance(var[10], list | str)` -repeated_isinstance_calls.py:24:14: PLR1701 Merge these isinstance calls: `isinstance(var[11], (float, int))` +ℹ Fix +20 20 | +21 21 | inferred_isinstance = isinstance +22 22 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] +23 |- result = isinstance(var[10], str) or isinstance(var[10], int) and var[8] * 14 or isinstance(var[10], float) and var[5] * 14.4 or isinstance(var[10], list) # [consider-merging-isinstance] + 23 |+ result = isinstance(var[10], list | str) # [consider-merging-isinstance] +24 24 | result = isinstance(var[11], int) or isinstance(var[11], int) or isinstance(var[11], float) # [consider-merging-isinstance] +25 25 | +26 26 | result = isinstance(var[20]) + +repeated_isinstance_calls.py:24:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[11], float | int)` | 24 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] 25 | result = isinstance(var[10], str) or isinstance(var[10], int) and var[8] * 14 or isinstance(var[10], float) and var[5] * 14.4 or isinstance(var[10], list) # [consider-merging-isinstance] @@ -48,8 +92,19 @@ repeated_isinstance_calls.py:24:14: PLR1701 Merge these isinstance calls: `isins 27 | 28 | result = isinstance(var[20]) | + = help: Replace with `isinstance(var[11], float | int)` + +ℹ Fix +21 21 | inferred_isinstance = isinstance +22 22 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] +23 23 | result = isinstance(var[10], str) or isinstance(var[10], int) and var[8] * 14 or isinstance(var[10], float) and var[5] * 14.4 or isinstance(var[10], list) # [consider-merging-isinstance] +24 |- result = isinstance(var[11], int) or isinstance(var[11], int) or isinstance(var[11], float) # [consider-merging-isinstance] + 24 |+ result = isinstance(var[11], float | int) # [consider-merging-isinstance] +25 25 | +26 26 | result = isinstance(var[20]) +27 27 | result = isinstance() -repeated_isinstance_calls.py:30:14: PLR1701 Merge these isinstance calls: `isinstance(var[12], (float, int, list))` +repeated_isinstance_calls.py:30:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[12], float | int | list)` | 30 | # Combination merged and not merged 31 | result = isinstance(var[12], (int, float)) or isinstance(var[12], list) # [consider-merging-isinstance] @@ -57,5 +112,16 @@ repeated_isinstance_calls.py:30:14: PLR1701 Merge these isinstance calls: `isins 32 | 33 | # not merged but valid | + = help: Replace with `isinstance(var[12], float | int | list)` + +ℹ Fix +27 27 | result = isinstance() +28 28 | +29 29 | # Combination merged and not merged +30 |- result = isinstance(var[12], (int, float)) or isinstance(var[12], list) # [consider-merging-isinstance] + 30 |+ result = isinstance(var[12], float | int | list) # [consider-merging-isinstance] +31 31 | +32 32 | # not merged but valid +33 33 | result = isinstance(var[5], int) and var[5] * 14 or isinstance(var[5], float) and var[5] * 14.4 diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__repeated_isinstance_calls.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__repeated_isinstance_calls.snap new file mode 100644 index 00000000000000..22051533bff25d --- /dev/null +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__repeated_isinstance_calls.snap @@ -0,0 +1,127 @@ +--- +source: crates/ruff/src/rules/pylint/mod.rs +--- +repeated_isinstance_calls.py:15:8: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[3], (float, int))` + | +15 | # not merged +16 | if isinstance(var[3], int) or isinstance(var[3], float) or isinstance(var[3], list) and True: # [consider-merging-isinstance] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701 +17 | pass +18 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] + | + = help: Replace with `isinstance(var[3], (float, int))` + +ℹ Fix +12 12 | result = isinstance(var[2], (int, float)) +13 13 | +14 14 | # not merged +15 |- if isinstance(var[3], int) or isinstance(var[3], float) or isinstance(var[3], list) and True: # [consider-merging-isinstance] + 15 |+ if isinstance(var[3], (float, int)): # [consider-merging-isinstance] +16 16 | pass +17 17 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] +18 18 | + +repeated_isinstance_calls.py:17:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[4], (float, int))` + | +17 | if isinstance(var[3], int) or isinstance(var[3], float) or isinstance(var[3], list) and True: # [consider-merging-isinstance] +18 | pass +19 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701 +20 | +21 | result = isinstance(var[5], int) or True or isinstance(var[5], float) # [consider-merging-isinstance] + | + = help: Replace with `isinstance(var[4], (float, int))` + +ℹ Fix +14 14 | # not merged +15 15 | if isinstance(var[3], int) or isinstance(var[3], float) or isinstance(var[3], list) and True: # [consider-merging-isinstance] +16 16 | pass +17 |- result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] + 17 |+ result = isinstance(var[4], (float, int)) # [consider-merging-isinstance] +18 18 | +19 19 | result = isinstance(var[5], int) or True or isinstance(var[5], float) # [consider-merging-isinstance] +20 20 | + +repeated_isinstance_calls.py:19:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[5], (float, int))` + | +19 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] +20 | +21 | result = isinstance(var[5], int) or True or isinstance(var[5], float) # [consider-merging-isinstance] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701 +22 | +23 | inferred_isinstance = isinstance + | + = help: Replace with `isinstance(var[5], (float, int))` + +ℹ Fix +16 16 | pass +17 17 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance] +18 18 | +19 |- result = isinstance(var[5], int) or True or isinstance(var[5], float) # [consider-merging-isinstance] + 19 |+ result = isinstance(var[5], (float, int)) # [consider-merging-isinstance] +20 20 | +21 21 | inferred_isinstance = isinstance +22 22 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] + +repeated_isinstance_calls.py:23:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[10], (list, str))` + | +23 | inferred_isinstance = isinstance +24 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] +25 | result = isinstance(var[10], str) or isinstance(var[10], int) and var[8] * 14 or isinstance(var[10], float) and var[5] * 14.4 or isinstance(var[10], list) # [consider-merging-isinstance] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701 +26 | result = isinstance(var[11], int) or isinstance(var[11], int) or isinstance(var[11], float) # [consider-merging-isinstance] + | + = help: Replace with `isinstance(var[10], (list, str))` + +ℹ Fix +20 20 | +21 21 | inferred_isinstance = isinstance +22 22 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] +23 |- result = isinstance(var[10], str) or isinstance(var[10], int) and var[8] * 14 or isinstance(var[10], float) and var[5] * 14.4 or isinstance(var[10], list) # [consider-merging-isinstance] + 23 |+ result = isinstance(var[10], (list, str)) # [consider-merging-isinstance] +24 24 | result = isinstance(var[11], int) or isinstance(var[11], int) or isinstance(var[11], float) # [consider-merging-isinstance] +25 25 | +26 26 | result = isinstance(var[20]) + +repeated_isinstance_calls.py:24:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[11], (float, int))` + | +24 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] +25 | result = isinstance(var[10], str) or isinstance(var[10], int) and var[8] * 14 or isinstance(var[10], float) and var[5] * 14.4 or isinstance(var[10], list) # [consider-merging-isinstance] +26 | result = isinstance(var[11], int) or isinstance(var[11], int) or isinstance(var[11], float) # [consider-merging-isinstance] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701 +27 | +28 | result = isinstance(var[20]) + | + = help: Replace with `isinstance(var[11], (float, int))` + +ℹ Fix +21 21 | inferred_isinstance = isinstance +22 22 | result = inferred_isinstance(var[6], int) or inferred_isinstance(var[6], float) or inferred_isinstance(var[6], list) and False # [consider-merging-isinstance] +23 23 | result = isinstance(var[10], str) or isinstance(var[10], int) and var[8] * 14 or isinstance(var[10], float) and var[5] * 14.4 or isinstance(var[10], list) # [consider-merging-isinstance] +24 |- result = isinstance(var[11], int) or isinstance(var[11], int) or isinstance(var[11], float) # [consider-merging-isinstance] + 24 |+ result = isinstance(var[11], (float, int)) # [consider-merging-isinstance] +25 25 | +26 26 | result = isinstance(var[20]) +27 27 | result = isinstance() + +repeated_isinstance_calls.py:30:14: PLR1701 [*] Merge `isinstance` calls: `isinstance(var[12], (float, int, list))` + | +30 | # Combination merged and not merged +31 | result = isinstance(var[12], (int, float)) or isinstance(var[12], list) # [consider-merging-isinstance] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1701 +32 | +33 | # not merged but valid + | + = help: Replace with `isinstance(var[12], (float, int, list))` + +ℹ Fix +27 27 | result = isinstance() +28 28 | +29 29 | # Combination merged and not merged +30 |- result = isinstance(var[12], (int, float)) or isinstance(var[12], list) # [consider-merging-isinstance] + 30 |+ result = isinstance(var[12], (float, int, list)) # [consider-merging-isinstance] +31 31 | +32 32 | # not merged but valid +33 33 | result = isinstance(var[5], int) and var[5] * 14 or isinstance(var[5], float) and var[5] * 14.4 + +