mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-19 03:48:29 +00:00
[ty] Add diagnostics for isinstance() and issubclass() calls that use invalid PEP-604 unions for their second argument (#21343)
## Summary
This PR adds extra validation for `isinstance()` and `issubclass()`
calls that use `UnionType` instances for their second argument.
According to typeshed's annotations, any `UnionType` is accepted for the
second argument, but this isn't true at runtime: at runtime, all
elements in the `UnionType` must either be class objects or be `None` in
order for the `isinstance()` or `issubclass()` call to reliably succeed:
```pycon
% uvx python3.14
Python 3.14.0 (main, Oct 10 2025, 12:54:13) [Clang 20.1.4 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing import LiteralString
>>> import types
>>> type(LiteralString | int) is types.UnionType
True
>>> isinstance(42, LiteralString | int)
Traceback (most recent call last):
File "<python-input-5>", line 1, in <module>
isinstance(42, LiteralString | int)
~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/alexw/Library/Application Support/uv/python/cpython-3.14.0-macos-aarch64-none/lib/python3.14/typing.py", line 559, in __instancecheck__
raise TypeError(f"{self} cannot be used with isinstance()")
TypeError: typing.LiteralString cannot be used with isinstance()
```
## Test Plan
Added mdtests/snapshots
This commit is contained in:
Alex Waygood
GitHub
parent
52bd22003b
commit
73b1fce74a
5 changed files with 213 additions and 8 deletions
|
|
@ -104,16 +104,25 @@ Except for the `None` special case mentioned above, narrowing can only take plac
|
|||
the PEP-604 union are class literals. If any elements are generic aliases or other types, the
|
||||
`isinstance()` call may fail at runtime, so no narrowing can take place:
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
```toml
|
||||
[environment]
|
||||
python-version = "3.10"
|
||||
```
|
||||
|
||||
```py
|
||||
from typing import Any, Literal, NamedTuple
|
||||
|
||||
def _(x: int | list[int] | bytes):
|
||||
# TODO: this fails at runtime; we should emit a diagnostic
|
||||
# (requires special-casing of the `isinstance()` signature)
|
||||
if isinstance(x, int | list[int]):
|
||||
# error: [invalid-argument-type]
|
||||
if isinstance(x, list[int] | int):
|
||||
reveal_type(x) # revealed: int | list[int] | bytes
|
||||
# error: [invalid-argument-type]
|
||||
elif isinstance(x, Literal[42] | list[int] | bytes):
|
||||
reveal_type(x) # revealed: int | list[int] | bytes
|
||||
# error: [invalid-argument-type]
|
||||
elif isinstance(x, Any | NamedTuple | list[int]):
|
||||
reveal_type(x) # revealed: int | list[int] | bytes
|
||||
else:
|
||||
reveal_type(x) # revealed: int | list[int] | bytes
|
||||
|
|
|
|||
|
|
@ -165,6 +165,8 @@ Except for the `None` special case mentioned above, narrowing can only take plac
|
|||
the PEP-604 union are class literals. If any elements are generic aliases or other types, the
|
||||
`issubclass()` call may fail at runtime, so no narrowing can take place:
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
```toml
|
||||
[environment]
|
||||
python-version = "3.10"
|
||||
|
|
@ -172,8 +174,7 @@ python-version = "3.10"
|
|||
|
||||
```py
|
||||
def _(x: type[int | list | bytes]):
|
||||
# TODO: this fails at runtime; we should emit a diagnostic
|
||||
# (requires special-casing of the `issubclass()` signature)
|
||||
# error: [invalid-argument-type]
|
||||
if issubclass(x, int | list[int]):
|
||||
reveal_type(x) # revealed: type[int] | type[list[Unknown]] | type[bytes]
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,88 @@
|
|||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
---
|
||||
mdtest name: isinstance.md - Narrowing for `isinstance` checks - `classinfo` is an invalid PEP-604 union of types
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/narrow/isinstance.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | from typing import Any, Literal, NamedTuple
|
||||
2 |
|
||||
3 | def _(x: int | list[int] | bytes):
|
||||
4 | # error: [invalid-argument-type]
|
||||
5 | if isinstance(x, list[int] | int):
|
||||
6 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
7 | # error: [invalid-argument-type]
|
||||
8 | elif isinstance(x, Literal[42] | list[int] | bytes):
|
||||
9 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
10 | # error: [invalid-argument-type]
|
||||
11 | elif isinstance(x, Any | NamedTuple | list[int]):
|
||||
12 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
13 | else:
|
||||
14 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error[invalid-argument-type]: Invalid second argument to `isinstance`
|
||||
--> src/mdtest_snippet.py:5:8
|
||||
|
|
||||
3 | def _(x: int | list[int] | bytes):
|
||||
4 | # error: [invalid-argument-type]
|
||||
5 | if isinstance(x, list[int] | int):
|
||||
| ^^^^^^^^^^^^^^---------------^
|
||||
| |
|
||||
| This `UnionType` instance contains non-class elements
|
||||
6 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
7 | # error: [invalid-argument-type]
|
||||
|
|
||||
info: A `UnionType` instance can only be used as the second argument to `isinstance` if all elements are class objects
|
||||
info: Element `<class 'list[int]'>` in the union is not a class object
|
||||
info: rule `invalid-argument-type` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error[invalid-argument-type]: Invalid second argument to `isinstance`
|
||||
--> src/mdtest_snippet.py:8:10
|
||||
|
|
||||
6 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
7 | # error: [invalid-argument-type]
|
||||
8 | elif isinstance(x, Literal[42] | list[int] | bytes):
|
||||
| ^^^^^^^^^^^^^^-------------------------------^
|
||||
| |
|
||||
| This `UnionType` instance contains non-class elements
|
||||
9 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
10 | # error: [invalid-argument-type]
|
||||
|
|
||||
info: A `UnionType` instance can only be used as the second argument to `isinstance` if all elements are class objects
|
||||
info: Elements `typing.Literal` and `<class 'list[int]'>` in the union are not class objects
|
||||
info: rule `invalid-argument-type` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error[invalid-argument-type]: Invalid second argument to `isinstance`
|
||||
--> src/mdtest_snippet.py:11:10
|
||||
|
|
||||
9 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
10 | # error: [invalid-argument-type]
|
||||
11 | elif isinstance(x, Any | NamedTuple | list[int]):
|
||||
| ^^^^^^^^^^^^^^----------------------------^
|
||||
| |
|
||||
| This `UnionType` instance contains non-class elements
|
||||
12 | reveal_type(x) # revealed: int | list[int] | bytes
|
||||
13 | else:
|
||||
|
|
||||
info: A `UnionType` instance can only be used as the second argument to `isinstance` if all elements are class objects
|
||||
info: Element `typing.Any` in the union, and 2 more elements, are not class objects
|
||||
info: rule `invalid-argument-type` is enabled by default
|
||||
|
||||
```
|
||||
|
|
@ -0,0 +1,42 @@
|
|||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
---
|
||||
mdtest name: issubclass.md - Narrowing for `issubclass` checks - `classinfo` is an invalid PEP-604 union of types
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/narrow/issubclass.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | def _(x: type[int | list | bytes]):
|
||||
2 | # error: [invalid-argument-type]
|
||||
3 | if issubclass(x, int | list[int]):
|
||||
4 | reveal_type(x) # revealed: type[int] | type[list[Unknown]] | type[bytes]
|
||||
5 | else:
|
||||
6 | reveal_type(x) # revealed: type[int] | type[list[Unknown]] | type[bytes]
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error[invalid-argument-type]: Invalid second argument to `issubclass`
|
||||
--> src/mdtest_snippet.py:3:8
|
||||
|
|
||||
1 | def _(x: type[int | list | bytes]):
|
||||
2 | # error: [invalid-argument-type]
|
||||
3 | if issubclass(x, int | list[int]):
|
||||
| ^^^^^^^^^^^^^^---------------^
|
||||
| |
|
||||
| This `UnionType` instance contains non-class elements
|
||||
4 | reveal_type(x) # revealed: type[int] | type[list[Unknown]] | type[bytes]
|
||||
5 | else:
|
||||
|
|
||||
info: A `UnionType` instance can only be used as the second argument to `issubclass` if all elements are class objects
|
||||
info: Element `<class 'list[int]'>` in the union is not a class object
|
||||
info: rule `invalid-argument-type` is enabled by default
|
||||
|
||||
```
|
||||
|
|
@ -81,9 +81,9 @@ use crate::types::visitor::any_over_type;
|
|||
use crate::types::{
|
||||
ApplyTypeMappingVisitor, BoundMethodType, BoundTypeVarInstance, CallableType, ClassBase,
|
||||
ClassLiteral, ClassType, DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor,
|
||||
HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor, KnownClass, NormalizedVisitor,
|
||||
SpecialFormType, Truthiness, Type, TypeContext, TypeMapping, TypeRelation, UnionBuilder,
|
||||
binding_type, todo_type, walk_signature,
|
||||
HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor, KnownClass, KnownInstanceType,
|
||||
NormalizedVisitor, SpecialFormType, Truthiness, Type, TypeContext, TypeMapping, TypeRelation,
|
||||
UnionBuilder, binding_type, todo_type, walk_signature,
|
||||
};
|
||||
use crate::{Db, FxOrderSet, ModuleName, resolve_module};
|
||||
|
||||
|
|
@ -1755,6 +1755,71 @@ impl KnownFunction {
|
|||
diagnostic
|
||||
.set_primary_message("This call will raise `TypeError` at runtime");
|
||||
}
|
||||
|
||||
Type::KnownInstance(KnownInstanceType::UnionType(_)) => {
|
||||
fn find_invalid_elements<'db>(
|
||||
db: &'db dyn Db,
|
||||
ty: Type<'db>,
|
||||
invalid_elements: &mut Vec<Type<'db>>,
|
||||
) {
|
||||
match ty {
|
||||
Type::ClassLiteral(_) => {}
|
||||
Type::NominalInstance(instance)
|
||||
if instance.has_known_class(db, KnownClass::NoneType) => {}
|
||||
Type::KnownInstance(KnownInstanceType::UnionType(union)) => {
|
||||
for element in union.elements(db) {
|
||||
find_invalid_elements(db, *element, invalid_elements);
|
||||
}
|
||||
}
|
||||
_ => invalid_elements.push(ty),
|
||||
}
|
||||
}
|
||||
|
||||
let mut invalid_elements = vec![];
|
||||
find_invalid_elements(db, *second_argument, &mut invalid_elements);
|
||||
|
||||
let Some((first_invalid_element, other_invalid_elements)) =
|
||||
invalid_elements.split_first()
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
||||
let Some(builder) =
|
||||
context.report_lint(&INVALID_ARGUMENT_TYPE, call_expression)
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
||||
let function_name: &str = self.into();
|
||||
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Invalid second argument to `{function_name}`"
|
||||
));
|
||||
diagnostic.info(format_args!(
|
||||
"A `UnionType` instance can only be used as the second argument to \
|
||||
`{function_name}` if all elements are class objects"
|
||||
));
|
||||
diagnostic.annotate(
|
||||
Annotation::secondary(context.span(&call_expression.arguments.args[1]))
|
||||
.message("This `UnionType` instance contains non-class elements"),
|
||||
);
|
||||
match other_invalid_elements {
|
||||
[] => diagnostic.info(format_args!(
|
||||
"Element `{}` in the union is not a class object",
|
||||
first_invalid_element.display(db)
|
||||
)),
|
||||
[single] => diagnostic.info(format_args!(
|
||||
"Elements `{}` and `{}` in the union are not class objects",
|
||||
first_invalid_element.display(db),
|
||||
single.display(db),
|
||||
)),
|
||||
_ => diagnostic.info(format_args!(
|
||||
"Element `{}` in the union, and {} more elements, are not class objects",
|
||||
first_invalid_element.display(db),
|
||||
other_invalid_elements.len(),
|
||||
))
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue