diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs index 5c7613357cfba..1b51c920ade14 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_isinstance_calls.rs @@ -4,7 +4,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use crate::fix::edits::pad; use crate::fix::snippet::SourceCodeSnippet; -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::hashable::HashableExpr; use ruff_text_size::Ranged; @@ -20,6 +20,15 @@ use crate::settings::types::PythonVersion; /// Repeated `isinstance` calls on the same object can be merged into a /// single call. /// +/// ## Fix safety +/// This rule's fix is marked as unsafe on Python 3.10 and later, as combining +/// multiple `isinstance` calls with a binary operator (`|`) will fail at +/// runtime if any of the operands are themselves tuples. +/// +/// For example, given `TYPES = (dict, list)`, then +/// `isinstance(None, TYPES | set | float)` will raise a `TypeError` at runtime, +/// while `isinstance(None, set | float)` will not. +/// /// ## Example /// ```python /// def is_number(x): @@ -130,10 +139,14 @@ pub(crate) fn repeated_isinstance_calls( }, expr.range(), ); - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - pad(call, expr.range(), checker.locator()), - expr.range(), - ))); + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement(pad(call, expr.range(), checker.locator()), expr.range()), + if checker.settings.target_version >= PythonVersion::Py310 { + Applicability::Unsafe + } else { + Applicability::Safe + }, + )); checker.diagnostics.push(diagnostic); } } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1701_repeated_isinstance_calls.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1701_repeated_isinstance_calls.py.snap index 9dd57ab80ebd2..e4f1bdf2066af 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1701_repeated_isinstance_calls.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1701_repeated_isinstance_calls.py.snap @@ -11,7 +11,7 @@ repeated_isinstance_calls.py:15:8: PLR1701 [*] Merge `isinstance` calls: `isinst | = help: Replace with `isinstance(var[3], float | int)` -ℹ Safe fix +ℹ Unsafe fix 12 12 | result = isinstance(var[2], (int, float)) 13 13 | 14 14 | # not merged @@ -32,7 +32,7 @@ repeated_isinstance_calls.py:17:14: PLR1701 [*] Merge `isinstance` calls: `isins | = help: Replace with `isinstance(var[4], float | int)` -ℹ Safe fix +ℹ Unsafe 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 @@ -53,7 +53,7 @@ repeated_isinstance_calls.py:19:14: PLR1701 [*] Merge `isinstance` calls: `isins | = help: Replace with `isinstance(var[5], float | int)` -ℹ Safe fix +ℹ Unsafe 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 | @@ -73,7 +73,7 @@ repeated_isinstance_calls.py:23:14: PLR1701 [*] Merge `isinstance` calls: `isins | = help: Replace with `isinstance(var[10], list | str)` -ℹ Safe fix +ℹ Unsafe 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] @@ -94,7 +94,7 @@ repeated_isinstance_calls.py:24:14: PLR1701 [*] Merge `isinstance` calls: `isins | = help: Replace with `isinstance(var[11], float | int)` -ℹ Safe fix +ℹ Unsafe 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] @@ -114,7 +114,7 @@ repeated_isinstance_calls.py:30:14: PLR1701 [*] Merge `isinstance` calls: `isins | = help: Replace with `isinstance(var[12], float | int | list)` -ℹ Safe fix +ℹ Unsafe fix 27 27 | result = isinstance() 28 28 | 29 29 | # Combination merged and not merged @@ -133,12 +133,10 @@ repeated_isinstance_calls.py:42:3: PLR1701 [*] Merge `isinstance` calls: `isinst | = help: Replace with `isinstance(self.k, float | int)` -ℹ Safe fix +ℹ Unsafe fix 39 39 | 40 40 | 41 41 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722460483 42 |-if(isinstance(self.k, int)) or (isinstance(self.k, float)): 42 |+if isinstance(self.k, float | int): 43 43 | ... - - diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__repeated_isinstance_calls.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__repeated_isinstance_calls.snap index 0d9e88910b492..38edd15cdbbf1 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__repeated_isinstance_calls.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__repeated_isinstance_calls.snap @@ -140,5 +140,3 @@ repeated_isinstance_calls.py:42:3: PLR1701 [*] Merge `isinstance` calls: `isinst 42 |-if(isinstance(self.k, int)) or (isinstance(self.k, float)): 42 |+if isinstance(self.k, (float, int)): 43 43 | ... - -