Add autofix for PLR1701 (repeated-isinstance-calls) (#4792)

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(

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,
+    expr: String,
-    types: Vec<String>,
 }
 
-impl Violation for RepeatedIsinstanceCalls {
+impl AlwaysAutofixableViolation for RepeatedIsinstanceCalls {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let RepeatedIsinstanceCalls { obj, types } = self;
+        let RepeatedIsinstanceCalls { expr } = self;
-        let types = types.join(", ");
+        format!("Merge `isinstance` calls: `{expr}`")
-        format!("Merge these isinstance calls: `isinstance({obj}, ({types}))`")
+    }
+
+    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(
+            let call = merged_isinstance_call(
-                RepeatedIsinstanceCalls {
+                &checker.generator().expr(obj.as_expr()),
-                    obj: checker.generator().expr(obj.as_expr()),
+                types
-                    types: types
+                    .iter()
-                        .iter()
+                    .map(HashableExpr::as_expr)
-                        .map(HashableExpr::as_expr)
+                    .map(|expr| checker.generator().expr(expr))
-                        .map(|expr| checker.generator().expr(expr))
+                    .sorted(),
-                        .sorted()
+                checker.settings.target_version,
-                        .collect(),
+            );
-                },
+            let mut diagnostic =
-                expr.range(),
+                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<Item = String>,
+    target_version: PythonVersion,
+) -> String {
+    if target_version >= PythonVersion::Py310 {
+        format!("isinstance({}, {})", obj, types.into_iter().join(" | "))
+    } else {
+        format!("isinstance({}, ({}))", obj, types.into_iter().join(", "))
+    }
+}

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)`
 
-repeated_isinstance_calls.py:19:14: PLR1701 Merge these isinstance calls: `isinstance(var[5], (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 | 
@@ -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)`
 
-repeated_isinstance_calls.py:23:14: PLR1701 Merge these isinstance calls: `isinstance(var[10], (list, str))`
+ℹ 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]
@@ -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)`
 
-repeated_isinstance_calls.py:30:14: PLR1701 Merge these isinstance calls: `isinstance(var[12], (float, int, list))`
+ℹ 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]
@@ -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
 
 

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
+
+