From 3db4f2f68cc4b9f12147baeeb64ae9ffb2107385 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 22 Apr 2025 19:16:43 +0100 Subject: [PATCH] [red-knot] Emit diagnostics for `isinstance()` and `issubclass()` calls where a non-runtime-checkable protocol is the second argument --- .../resources/mdtest/protocols.md | 17 +- ..._-_Protocols_-_Narrowing_of_protocols.snap | 241 ++++++++++++++++++ crates/red_knot_python_semantic/src/types.rs | 6 +- .../src/types/class.rs | 19 +- .../src/types/diagnostic.rs | 48 +++- .../src/types/infer.rs | 21 +- 6 files changed, 336 insertions(+), 16 deletions(-) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/snapshots/protocols.md_-_Protocols_-_Narrowing_of_protocols.snap diff --git a/crates/red_knot_python_semantic/resources/mdtest/protocols.md b/crates/red_knot_python_semantic/resources/mdtest/protocols.md index 9988ea761fae5..abba908825da8 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/protocols.md +++ b/crates/red_knot_python_semantic/resources/mdtest/protocols.md @@ -1199,23 +1199,25 @@ static_assert(is_assignable_to(HasGetAttrAndSetAttr, XAsymmetricProperty)) # er ## Narrowing of protocols + + 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 diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/protocols.md_-_Protocols_-_Narrowing_of_protocols.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/protocols.md_-_Protocols_-_Narrowing_of_protocols.snap new file mode 100644 index 0000000000000..5767ef3dbf4f8 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/protocols.md_-_Protocols_-_Narrowing_of_protocols.snap @@ -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]` + | + +``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 8e9d3edb5ae49..ef386b3585499 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -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")] diff --git a/crates/red_knot_python_semantic/src/types/class.rs b/crates/red_knot_python_semantic/src/types/class.rs index 82f88a35deaa2..20e0624dd35ac 100644 --- a/crates/red_knot_python_semantic/src/types/class.rs +++ b/crates/red_knot_python_semantic/src/types/class.rs @@ -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 + '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> { diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index ec546255cfb8d..c4b7d4f87b31a 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -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"); +} diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 8dcfe14167541..9ca92e493eaad 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -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 + ); + } + } + } + } _ => {} } }