mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 10:48:32 +00:00
[red-knot] Emit diagnostics for isinstance() and issubclass() calls where a non-runtime-checkable protocol is the second argument (#17561)
This commit is contained in:
parent
00e73dc331
commit
e897f37911
6 changed files with 336 additions and 16 deletions
|
@ -1199,23 +1199,25 @@ static_assert(is_assignable_to(HasGetAttrAndSetAttr, XAsymmetricProperty)) # er
|
|||
|
||||
## Narrowing of protocols
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
By default, a protocol class cannot be used as the second argument to `isinstance()` or
|
||||
`issubclass()`, and a type checker must emit an error on such calls. However, we still narrow the
|
||||
type inside these branches (this matches the behaviour of other type checkers):
|
||||
|
||||
```py
|
||||
from typing import Protocol
|
||||
from typing_extensions import Protocol, reveal_type
|
||||
|
||||
class HasX(Protocol):
|
||||
x: int
|
||||
|
||||
def f(arg: object, arg2: type):
|
||||
if isinstance(arg, HasX): # error
|
||||
if isinstance(arg, HasX): # error: [invalid-argument-type]
|
||||
reveal_type(arg) # revealed: HasX
|
||||
else:
|
||||
reveal_type(arg) # revealed: ~HasX
|
||||
|
||||
if issubclass(arg2, HasX): # error
|
||||
if issubclass(arg2, HasX): # error: [invalid-argument-type]
|
||||
reveal_type(arg2) # revealed: type[HasX]
|
||||
else:
|
||||
reveal_type(arg2) # revealed: type & ~type[HasX]
|
||||
|
@ -1250,10 +1252,10 @@ class OnlyMethodMembers(Protocol):
|
|||
def method(self) -> None: ...
|
||||
|
||||
def f(arg1: type, arg2: type):
|
||||
if issubclass(arg1, OnlyMethodMembers): # error
|
||||
reveal_type(arg1) # revealed: type[OnlyMethodMembers]
|
||||
if issubclass(arg1, RuntimeCheckableHasX): # TODO: should emit an error here (has non-method members)
|
||||
reveal_type(arg1) # revealed: type[RuntimeCheckableHasX]
|
||||
else:
|
||||
reveal_type(arg1) # revealed: type & ~type[OnlyMethodMembers]
|
||||
reveal_type(arg1) # revealed: type & ~type[RuntimeCheckableHasX]
|
||||
|
||||
if issubclass(arg2, OnlyMethodMembers): # no error!
|
||||
reveal_type(arg2) # revealed: type[OnlyMethodMembers]
|
||||
|
@ -1289,8 +1291,6 @@ def _(some_list: list, some_tuple: tuple[int, str], some_sized: Sized):
|
|||
|
||||
Add tests for:
|
||||
|
||||
- Assignments without declarations in protocol class bodies. And various weird ways of creating
|
||||
attributes in a class body or instance method. [Example mypy tests][mypy_weird_protocols].
|
||||
- More tests for protocols inside `type[]`. [Spec reference][protocols_inside_type_spec].
|
||||
- Protocols with instance-method members
|
||||
- Protocols with `@classmethod` and `@staticmethod`
|
||||
|
@ -1313,7 +1313,6 @@ Add tests for:
|
|||
|
||||
[mypy_protocol_docs]: https://mypy.readthedocs.io/en/stable/protocols.html#protocols-and-structural-subtyping
|
||||
[mypy_protocol_tests]: https://github.com/python/mypy/blob/master/test-data/unit/check-protocols.test
|
||||
[mypy_weird_protocols]: https://github.com/python/mypy/blob/a3ce6d5307e99a1b6c181eaa7c5cf134c53b7d8b/test-data/unit/check-protocols.test#L2131-L2132
|
||||
[protocol conformance tests]: https://github.com/python/typing/tree/main/conformance/tests
|
||||
[protocols_inside_type_spec]: https://typing.python.org/en/latest/spec/protocol.html#type-and-class-objects-vs-protocols
|
||||
[recursive_protocols_spec]: https://typing.python.org/en/latest/spec/protocol.html#recursive-protocols
|
||||
|
|
|
@ -0,0 +1,241 @@
|
|||
---
|
||||
source: crates/red_knot_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
---
|
||||
mdtest name: protocols.md - Protocols - Narrowing of protocols
|
||||
mdtest path: crates/red_knot_python_semantic/resources/mdtest/protocols.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | from typing_extensions import Protocol, reveal_type
|
||||
2 |
|
||||
3 | class HasX(Protocol):
|
||||
4 | x: int
|
||||
5 |
|
||||
6 | def f(arg: object, arg2: type):
|
||||
7 | if isinstance(arg, HasX): # error: [invalid-argument-type]
|
||||
8 | reveal_type(arg) # revealed: HasX
|
||||
9 | else:
|
||||
10 | reveal_type(arg) # revealed: ~HasX
|
||||
11 |
|
||||
12 | if issubclass(arg2, HasX): # error: [invalid-argument-type]
|
||||
13 | reveal_type(arg2) # revealed: type[HasX]
|
||||
14 | else:
|
||||
15 | reveal_type(arg2) # revealed: type & ~type[HasX]
|
||||
16 | from typing import runtime_checkable
|
||||
17 |
|
||||
18 | @runtime_checkable
|
||||
19 | class RuntimeCheckableHasX(Protocol):
|
||||
20 | x: int
|
||||
21 |
|
||||
22 | def f(arg: object):
|
||||
23 | if isinstance(arg, RuntimeCheckableHasX): # no error!
|
||||
24 | reveal_type(arg) # revealed: RuntimeCheckableHasX
|
||||
25 | else:
|
||||
26 | reveal_type(arg) # revealed: ~RuntimeCheckableHasX
|
||||
27 | @runtime_checkable
|
||||
28 | class OnlyMethodMembers(Protocol):
|
||||
29 | def method(self) -> None: ...
|
||||
30 |
|
||||
31 | def f(arg1: type, arg2: type):
|
||||
32 | if issubclass(arg1, RuntimeCheckableHasX): # TODO: should emit an error here (has non-method members)
|
||||
33 | reveal_type(arg1) # revealed: type[RuntimeCheckableHasX]
|
||||
34 | else:
|
||||
35 | reveal_type(arg1) # revealed: type & ~type[RuntimeCheckableHasX]
|
||||
36 |
|
||||
37 | if issubclass(arg2, OnlyMethodMembers): # no error!
|
||||
38 | reveal_type(arg2) # revealed: type[OnlyMethodMembers]
|
||||
39 | else:
|
||||
40 | reveal_type(arg2) # revealed: type & ~type[OnlyMethodMembers]
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error: lint:invalid-argument-type: Class `HasX` cannot be used as the second argument to `isinstance`
|
||||
--> /src/mdtest_snippet.py:7:8
|
||||
|
|
||||
6 | def f(arg: object, arg2: type):
|
||||
7 | if isinstance(arg, HasX): # error: [invalid-argument-type]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^ This call will raise `TypeError` at runtime
|
||||
8 | reveal_type(arg) # revealed: HasX
|
||||
9 | else:
|
||||
|
|
||||
info: `HasX` is declared as a protocol class, but it is not declared as runtime-checkable
|
||||
--> /src/mdtest_snippet.py:3:7
|
||||
|
|
||||
1 | from typing_extensions import Protocol, reveal_type
|
||||
2 |
|
||||
3 | class HasX(Protocol):
|
||||
| ^^^^^^^^^^^^^^ `HasX` declared here
|
||||
4 | x: int
|
||||
|
|
||||
info: A protocol class can only be used in `isinstance` checks if it is decorated with `@typing.runtime_checkable` or `@typing_extensions.runtime_checkable`
|
||||
info: See https://docs.python.org/3/library/typing.html#typing.runtime_checkable
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:8:9
|
||||
|
|
||||
6 | def f(arg: object, arg2: type):
|
||||
7 | if isinstance(arg, HasX): # error: [invalid-argument-type]
|
||||
8 | reveal_type(arg) # revealed: HasX
|
||||
| ^^^^^^^^^^^^^^^^ `HasX`
|
||||
9 | else:
|
||||
10 | reveal_type(arg) # revealed: ~HasX
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:10:9
|
||||
|
|
||||
8 | reveal_type(arg) # revealed: HasX
|
||||
9 | else:
|
||||
10 | reveal_type(arg) # revealed: ~HasX
|
||||
| ^^^^^^^^^^^^^^^^ `~HasX`
|
||||
11 |
|
||||
12 | if issubclass(arg2, HasX): # error: [invalid-argument-type]
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error: lint:invalid-argument-type: Class `HasX` cannot be used as the second argument to `issubclass`
|
||||
--> /src/mdtest_snippet.py:12:8
|
||||
|
|
||||
10 | reveal_type(arg) # revealed: ~HasX
|
||||
11 |
|
||||
12 | if issubclass(arg2, HasX): # error: [invalid-argument-type]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^ This call will raise `TypeError` at runtime
|
||||
13 | reveal_type(arg2) # revealed: type[HasX]
|
||||
14 | else:
|
||||
|
|
||||
info: `HasX` is declared as a protocol class, but it is not declared as runtime-checkable
|
||||
--> /src/mdtest_snippet.py:3:7
|
||||
|
|
||||
1 | from typing_extensions import Protocol, reveal_type
|
||||
2 |
|
||||
3 | class HasX(Protocol):
|
||||
| ^^^^^^^^^^^^^^ `HasX` declared here
|
||||
4 | x: int
|
||||
|
|
||||
info: A protocol class can only be used in `issubclass` checks if it is decorated with `@typing.runtime_checkable` or `@typing_extensions.runtime_checkable`
|
||||
info: See https://docs.python.org/3/library/typing.html#typing.runtime_checkable
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:13:9
|
||||
|
|
||||
12 | if issubclass(arg2, HasX): # error: [invalid-argument-type]
|
||||
13 | reveal_type(arg2) # revealed: type[HasX]
|
||||
| ^^^^^^^^^^^^^^^^^ `type[HasX]`
|
||||
14 | else:
|
||||
15 | reveal_type(arg2) # revealed: type & ~type[HasX]
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:15:9
|
||||
|
|
||||
13 | reveal_type(arg2) # revealed: type[HasX]
|
||||
14 | else:
|
||||
15 | reveal_type(arg2) # revealed: type & ~type[HasX]
|
||||
| ^^^^^^^^^^^^^^^^^ `type & ~type[HasX]`
|
||||
16 | from typing import runtime_checkable
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:24:9
|
||||
|
|
||||
22 | def f(arg: object):
|
||||
23 | if isinstance(arg, RuntimeCheckableHasX): # no error!
|
||||
24 | reveal_type(arg) # revealed: RuntimeCheckableHasX
|
||||
| ^^^^^^^^^^^^^^^^ `RuntimeCheckableHasX`
|
||||
25 | else:
|
||||
26 | reveal_type(arg) # revealed: ~RuntimeCheckableHasX
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:26:9
|
||||
|
|
||||
24 | reveal_type(arg) # revealed: RuntimeCheckableHasX
|
||||
25 | else:
|
||||
26 | reveal_type(arg) # revealed: ~RuntimeCheckableHasX
|
||||
| ^^^^^^^^^^^^^^^^ `~RuntimeCheckableHasX`
|
||||
27 | @runtime_checkable
|
||||
28 | class OnlyMethodMembers(Protocol):
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:33:9
|
||||
|
|
||||
31 | def f(arg1: type, arg2: type):
|
||||
32 | if issubclass(arg1, RuntimeCheckableHasX): # TODO: should emit an error here (has non-method members)
|
||||
33 | reveal_type(arg1) # revealed: type[RuntimeCheckableHasX]
|
||||
| ^^^^^^^^^^^^^^^^^ `type[RuntimeCheckableHasX]`
|
||||
34 | else:
|
||||
35 | reveal_type(arg1) # revealed: type & ~type[RuntimeCheckableHasX]
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:35:9
|
||||
|
|
||||
33 | reveal_type(arg1) # revealed: type[RuntimeCheckableHasX]
|
||||
34 | else:
|
||||
35 | reveal_type(arg1) # revealed: type & ~type[RuntimeCheckableHasX]
|
||||
| ^^^^^^^^^^^^^^^^^ `type & ~type[RuntimeCheckableHasX]`
|
||||
36 |
|
||||
37 | if issubclass(arg2, OnlyMethodMembers): # no error!
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:38:9
|
||||
|
|
||||
37 | if issubclass(arg2, OnlyMethodMembers): # no error!
|
||||
38 | reveal_type(arg2) # revealed: type[OnlyMethodMembers]
|
||||
| ^^^^^^^^^^^^^^^^^ `type[OnlyMethodMembers]`
|
||||
39 | else:
|
||||
40 | reveal_type(arg2) # revealed: type & ~type[OnlyMethodMembers]
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info: revealed-type: Revealed type
|
||||
--> /src/mdtest_snippet.py:40:9
|
||||
|
|
||||
38 | reveal_type(arg2) # revealed: type[OnlyMethodMembers]
|
||||
39 | else:
|
||||
40 | reveal_type(arg2) # revealed: type & ~type[OnlyMethodMembers]
|
||||
| ^^^^^^^^^^^^^^^^^ `type & ~type[OnlyMethodMembers]`
|
||||
|
|
||||
|
||||
```
|
|
@ -6231,9 +6231,11 @@ impl<'db> FunctionType<'db> {
|
|||
|
||||
/// Non-exhaustive enumeration of known functions (e.g. `builtins.reveal_type`, ...) that might
|
||||
/// have special behavior.
|
||||
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, strum_macros::EnumString)]
|
||||
#[derive(
|
||||
Debug, Copy, Clone, PartialEq, Eq, Hash, strum_macros::EnumString, strum_macros::IntoStaticStr,
|
||||
)]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
#[cfg_attr(test, derive(strum_macros::EnumIter, strum_macros::IntoStaticStr))]
|
||||
#[cfg_attr(test, derive(strum_macros::EnumIter))]
|
||||
pub enum KnownFunction {
|
||||
/// `builtins.isinstance`
|
||||
#[strum(serialize = "isinstance")]
|
||||
|
|
|
@ -629,12 +629,20 @@ impl<'db> ClassLiteralType<'db> {
|
|||
.collect()
|
||||
}
|
||||
|
||||
/// Is this class final?
|
||||
pub(super) fn is_final(self, db: &'db dyn Db) -> bool {
|
||||
fn known_function_decorators(
|
||||
self,
|
||||
db: &'db dyn Db,
|
||||
) -> impl Iterator<Item = KnownFunction> + 'db {
|
||||
self.decorators(db)
|
||||
.iter()
|
||||
.filter_map(|deco| deco.into_function_literal())
|
||||
.any(|decorator| decorator.is_known(db, KnownFunction::Final))
|
||||
.filter_map(|decorator| decorator.known(db))
|
||||
}
|
||||
|
||||
/// Is this class final?
|
||||
pub(super) fn is_final(self, db: &'db dyn Db) -> bool {
|
||||
self.known_function_decorators(db)
|
||||
.contains(&KnownFunction::Final)
|
||||
}
|
||||
|
||||
/// Attempt to resolve the [method resolution order] ("MRO") for this class.
|
||||
|
@ -1837,6 +1845,11 @@ impl<'db> ProtocolClassLiteral<'db> {
|
|||
|
||||
cached_protocol_members(db, *self)
|
||||
}
|
||||
|
||||
pub(super) fn is_runtime_checkable(self, db: &'db dyn Db) -> bool {
|
||||
self.known_function_decorators(db)
|
||||
.contains(&KnownFunction::RuntimeCheckable)
|
||||
}
|
||||
}
|
||||
|
||||
impl<'db> Deref for ProtocolClassLiteral<'db> {
|
||||
|
|
|
@ -8,7 +8,7 @@ use crate::types::string_annotation::{
|
|||
IMPLICIT_CONCATENATED_STRING_TYPE_ANNOTATION, INVALID_SYNTAX_IN_FORWARD_ANNOTATION,
|
||||
RAW_STRING_TYPE_ANNOTATION,
|
||||
};
|
||||
use crate::types::{KnownInstanceType, Type};
|
||||
use crate::types::{class::ProtocolClassLiteral, KnownFunction, KnownInstanceType, Type};
|
||||
use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, Span, SubDiagnostic};
|
||||
use ruff_python_ast::{self as ast, AnyNodeRef};
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
@ -1375,3 +1375,49 @@ pub(crate) fn report_invalid_arguments_to_callable(
|
|||
KnownInstanceType::Callable.repr(context.db())
|
||||
));
|
||||
}
|
||||
|
||||
pub(crate) fn report_runtime_check_against_non_runtime_checkable_protocol(
|
||||
context: &InferContext,
|
||||
call: &ast::ExprCall,
|
||||
protocol: ProtocolClassLiteral,
|
||||
function: KnownFunction,
|
||||
) {
|
||||
let Some(builder) = context.report_lint(&INVALID_ARGUMENT_TYPE, call) else {
|
||||
return;
|
||||
};
|
||||
let db = context.db();
|
||||
let class_name = protocol.name(db);
|
||||
let function_name: &'static str = function.into();
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Class `{class_name}` cannot be used as the second argument to `{function_name}`",
|
||||
));
|
||||
diagnostic.set_primary_message("This call will raise `TypeError` at runtime");
|
||||
|
||||
let class_scope = protocol.body_scope(db);
|
||||
let class_node = class_scope.node(db).expect_class();
|
||||
let class_def_arguments = class_node
|
||||
.arguments
|
||||
.as_ref()
|
||||
.expect("A `Protocol` class should always have at least one explicit base");
|
||||
let mut class_def_diagnostic = SubDiagnostic::new(
|
||||
Severity::Info,
|
||||
format_args!(
|
||||
"`{class_name}` is declared as a protocol class, \
|
||||
but it is not declared as runtime-checkable"
|
||||
),
|
||||
);
|
||||
class_def_diagnostic.annotate(
|
||||
Annotation::primary(Span::from(class_scope.file(db)).with_range(TextRange::new(
|
||||
class_node.name.start(),
|
||||
class_def_arguments.end(),
|
||||
)))
|
||||
.message(format_args!("`{class_name}` declared here")),
|
||||
);
|
||||
diagnostic.sub(class_def_diagnostic);
|
||||
|
||||
diagnostic.info(format_args!(
|
||||
"A protocol class can only be used in `{function_name}` checks if it is decorated \
|
||||
with `@typing.runtime_checkable` or `@typing_extensions.runtime_checkable`"
|
||||
));
|
||||
diagnostic.info("See https://docs.python.org/3/library/typing.html#typing.runtime_checkable");
|
||||
}
|
||||
|
|
|
@ -99,7 +99,8 @@ use super::diagnostic::{
|
|||
report_bad_argument_to_get_protocol_members, report_index_out_of_bounds,
|
||||
report_invalid_exception_caught, report_invalid_exception_cause,
|
||||
report_invalid_exception_raised, report_invalid_type_checking_constant,
|
||||
report_non_subscriptable, report_possibly_unresolved_reference, report_slice_step_size_zero,
|
||||
report_non_subscriptable, report_possibly_unresolved_reference,
|
||||
report_runtime_check_against_non_runtime_checkable_protocol, report_slice_step_size_zero,
|
||||
report_unresolved_reference, INVALID_METACLASS, INVALID_PROTOCOL, REDUNDANT_CAST,
|
||||
STATIC_ASSERT_ERROR, SUBCLASS_OF_FINAL_CLASS, TYPE_ASSERTION_FAILURE,
|
||||
};
|
||||
|
@ -4497,6 +4498,24 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
}
|
||||
}
|
||||
}
|
||||
KnownFunction::IsInstance | KnownFunction::IsSubclass => {
|
||||
if let [_, Some(Type::ClassLiteral(class))] =
|
||||
overload.parameter_types()
|
||||
{
|
||||
if let Some(protocol_class) =
|
||||
class.into_protocol_class(self.db())
|
||||
{
|
||||
if !protocol_class.is_runtime_checkable(self.db()) {
|
||||
report_runtime_check_against_non_runtime_checkable_protocol(
|
||||
&self.context,
|
||||
call_expression,
|
||||
protocol_class,
|
||||
known_function
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue