From 68c1fa86c8d18801f3cfc943d6f16a78e745dda2 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 18 Oct 2025 15:01:46 +0100 Subject: [PATCH] [ty] Fix panic when attempting to validate the members of a protocol that inherits from a protocol in another module (#20956) --- .../resources/mdtest/protocols.md | 27 +++++++++++++ ...gnostics_for_prot…_(585a3e9545d41b64).snap | 40 ++++++++++++++----- .../src/types/infer/builder.rs | 2 +- .../src/types/protocol_class.rs | 11 ++--- 4 files changed, 63 insertions(+), 17 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/protocols.md b/crates/ty_python_semantic/resources/mdtest/protocols.md index 5a939e4880..2818de99e4 100644 --- a/crates/ty_python_semantic/resources/mdtest/protocols.md +++ b/crates/ty_python_semantic/resources/mdtest/protocols.md @@ -1114,6 +1114,8 @@ it's a large section). +`a.py`: + ```py from typing import Protocol @@ -1140,6 +1142,31 @@ class A(Protocol): pass ``` +Validation of protocols that had cross-module inheritance used to break, so we test that explicitly +here too: + +`b.py`: + +```py +from typing import Protocol + +# Ensure the number of scopes in `b.py` is greater than the number of scopes in `c.py`: +class SomethingUnrelated: ... + +class A(Protocol): + x: int +``` + +`c.py`: + +```py +from b import A +from typing import Protocol + +class C(A, Protocol): + x = 42 # fine, due to declaration in the base class +``` + ## Equivalence of protocols ```toml diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/protocols.md_-_Protocols_-_Diagnostics_for_prot…_(585a3e9545d41b64).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/protocols.md_-_Protocols_-_Diagnostics_for_prot…_(585a3e9545d41b64).snap index 425665b486..6609a48adb 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/protocols.md_-_Protocols_-_Diagnostics_for_prot…_(585a3e9545d41b64).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/protocols.md_-_Protocols_-_Diagnostics_for_prot…_(585a3e9545d41b64).snap @@ -9,7 +9,7 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/protocols.md # Python source files -## mdtest_snippet.py +## a.py ``` 1 | from typing import Protocol @@ -37,11 +37,33 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/protocols.md 23 | pass ``` +## b.py + +``` +1 | from typing import Protocol +2 | +3 | # Ensure the number of scopes in `b.py` is greater than the number of scopes in `c.py`: +4 | class SomethingUnrelated: ... +5 | +6 | class A(Protocol): +7 | x: int +``` + +## c.py + +``` +1 | from b import A +2 | from typing import Protocol +3 | +4 | class C(A, Protocol): +5 | x = 42 # fine, due to declaration in the base class +``` + # Diagnostics ``` warning[ambiguous-protocol-member]: Cannot assign to undeclared variable in the body of a protocol class - --> src/mdtest_snippet.py:12:5 + --> src/a.py:12:5 | 11 | # error: [ambiguous-protocol-member] 12 | a = None # type: int @@ -50,7 +72,7 @@ warning[ambiguous-protocol-member]: Cannot assign to undeclared variable in the 14 | b = ... # type: str | info: Assigning to an undeclared variable in a protocol class leads to an ambiguous interface - --> src/mdtest_snippet.py:6:7 + --> src/a.py:6:7 | 4 | return True 5 | @@ -66,7 +88,7 @@ info: rule `ambiguous-protocol-member` is enabled by default ``` warning[ambiguous-protocol-member]: Cannot assign to undeclared variable in the body of a protocol class - --> src/mdtest_snippet.py:14:5 + --> src/a.py:14:5 | 12 | a = None # type: int 13 | # error: [ambiguous-protocol-member] @@ -76,7 +98,7 @@ warning[ambiguous-protocol-member]: Cannot assign to undeclared variable in the 16 | if coinflip(): | info: Assigning to an undeclared variable in a protocol class leads to an ambiguous interface - --> src/mdtest_snippet.py:6:7 + --> src/a.py:6:7 | 4 | return True 5 | @@ -92,7 +114,7 @@ info: rule `ambiguous-protocol-member` is enabled by default ``` warning[ambiguous-protocol-member]: Cannot assign to undeclared variable in the body of a protocol class - --> src/mdtest_snippet.py:17:9 + --> src/a.py:17:9 | 16 | if coinflip(): 17 | c = 1 # error: [ambiguous-protocol-member] @@ -101,7 +123,7 @@ warning[ambiguous-protocol-member]: Cannot assign to undeclared variable in the 19 | c = 2 | info: Assigning to an undeclared variable in a protocol class leads to an ambiguous interface - --> src/mdtest_snippet.py:6:7 + --> src/a.py:6:7 | 4 | return True 5 | @@ -117,7 +139,7 @@ info: rule `ambiguous-protocol-member` is enabled by default ``` warning[ambiguous-protocol-member]: Cannot assign to undeclared variable in the body of a protocol class - --> src/mdtest_snippet.py:22:9 + --> src/a.py:22:9 | 21 | # error: [ambiguous-protocol-member] 22 | for d in range(42): @@ -125,7 +147,7 @@ warning[ambiguous-protocol-member]: Cannot assign to undeclared variable in the 23 | pass | info: Assigning to an undeclared variable in a protocol class leads to an ambiguous interface - --> src/mdtest_snippet.py:6:7 + --> src/a.py:6:7 | 4 | return True 5 | diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index f0fd9ec502..e0ad80ac06 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -932,7 +932,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } if let Some(protocol) = class.into_protocol_class(self.db()) { - protocol.validate_members(&self.context, self.index); + protocol.validate_members(&self.context); } } } diff --git a/crates/ty_python_semantic/src/types/protocol_class.rs b/crates/ty_python_semantic/src/types/protocol_class.rs index eaa88bc793..904d9d97d0 100644 --- a/crates/ty_python_semantic/src/types/protocol_class.rs +++ b/crates/ty_python_semantic/src/types/protocol_class.rs @@ -10,9 +10,7 @@ use crate::types::TypeContext; use crate::{ Db, FxOrderSet, place::{Definedness, Place, PlaceAndQualifiers, place_from_bindings, place_from_declarations}, - semantic_index::{ - SemanticIndex, definition::Definition, place::ScopedPlaceId, place_table, use_def_map, - }, + semantic_index::{definition::Definition, place::ScopedPlaceId, place_table, use_def_map}, types::{ ApplyTypeMappingVisitor, BoundTypeVarInstance, CallableType, ClassBase, ClassLiteral, ClassType, FindLegacyTypeVarsVisitor, HasRelationToVisitor, @@ -77,11 +75,11 @@ impl<'db> ProtocolClass<'db> { /// Iterate through the body of the protocol class. Check that all definitions /// in the protocol class body are either explicitly declared directly in the /// class body, or are declared in a superclass of the protocol class. - pub(super) fn validate_members(self, context: &InferContext, index: &SemanticIndex<'db>) { + pub(super) fn validate_members(self, context: &InferContext) { let db = context.db(); let interface = self.interface(db); let body_scope = self.class_literal(db).0.body_scope(db); - let class_place_table = index.place_table(body_scope.file_scope_id(db)); + let class_place_table = place_table(db, body_scope); for (symbol_id, mut bindings_iterator) in use_def_map(db, body_scope).all_end_of_scope_symbol_bindings() @@ -104,8 +102,7 @@ impl<'db> ProtocolClass<'db> { }; !place_from_declarations( db, - index - .use_def_map(superclass_scope.file_scope_id(db)) + use_def_map(db, superclass_scope) .end_of_scope_declarations(ScopedPlaceId::Symbol(scoped_symbol_id)), ) .into_place_and_conflicting_declarations()