Mark repeated-isinstance-calls as unsafe on Python 3.10 and later (#11622)

## Summary

Closes https://github.com/astral-sh/ruff/issues/11616.
This commit is contained in:
Charlie Marsh 2024-05-30 14:05:24 -04:00 committed by GitHub
parent dcabd04caf
commit 685d11a909
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 25 additions and 16 deletions

View file

@ -4,7 +4,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
use crate::fix::edits::pad; use crate::fix::edits::pad;
use crate::fix::snippet::SourceCodeSnippet; 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_macros::{derive_message_formats, violation};
use ruff_python_ast::hashable::HashableExpr; use ruff_python_ast::hashable::HashableExpr;
use ruff_text_size::Ranged; 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 /// Repeated `isinstance` calls on the same object can be merged into a
/// single call. /// 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 /// ## Example
/// ```python /// ```python
/// def is_number(x): /// def is_number(x):
@ -130,10 +139,14 @@ pub(crate) fn repeated_isinstance_calls(
}, },
expr.range(), expr.range(),
); );
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( diagnostic.set_fix(Fix::applicable_edit(
pad(call, expr.range(), checker.locator()), Edit::range_replacement(pad(call, expr.range(), checker.locator()), expr.range()),
expr.range(), if checker.settings.target_version >= PythonVersion::Py310 {
))); Applicability::Unsafe
} else {
Applicability::Safe
},
));
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }
} }

View file

@ -11,7 +11,7 @@ repeated_isinstance_calls.py:15:8: PLR1701 [*] Merge `isinstance` calls: `isinst
| |
= help: Replace with `isinstance(var[3], float | int)` = help: Replace with `isinstance(var[3], float | int)`
Safe fix Unsafe fix
12 12 | result = isinstance(var[2], (int, float)) 12 12 | result = isinstance(var[2], (int, float))
13 13 | 13 13 |
14 14 | # not merged 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)` = help: Replace with `isinstance(var[4], float | int)`
Safe fix Unsafe fix
14 14 | # not merged 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] 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 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)` = help: Replace with `isinstance(var[5], float | int)`
Safe fix Unsafe fix
16 16 | pass 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] 17 17 | result = isinstance(var[4], int) or isinstance(var[4], float) or isinstance(var[5], list) and False # [consider-merging-isinstance]
18 18 | 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)` = help: Replace with `isinstance(var[10], list | str)`
Safe fix Unsafe fix
20 20 | 20 20 |
21 21 | inferred_isinstance = isinstance 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] 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)` = help: Replace with `isinstance(var[11], float | int)`
Safe fix Unsafe fix
21 21 | inferred_isinstance = isinstance 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] 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] 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)` = help: Replace with `isinstance(var[12], float | int | list)`
Safe fix Unsafe fix
27 27 | result = isinstance() 27 27 | result = isinstance()
28 28 | 28 28 |
29 29 | # Combination merged and not merged 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)` = help: Replace with `isinstance(self.k, float | int)`
Safe fix Unsafe fix
39 39 | 39 39 |
40 40 | 40 40 |
41 41 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722460483 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, int)) or (isinstance(self.k, float)):
42 |+if isinstance(self.k, float | int): 42 |+if isinstance(self.k, float | int):
43 43 | ... 43 43 | ...

View file

@ -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, int)) or (isinstance(self.k, float)):
42 |+if isinstance(self.k, (float, int)): 42 |+if isinstance(self.k, (float, int)):
43 43 | ... 43 43 | ...