From 62d43b8cf391608d33bb08aa4e0264ee07541c3e Mon Sep 17 00:00:00 2001 From: Shantanu Jain Date: Sun, 3 Nov 2024 12:06:14 -0800 Subject: [PATCH 1/5] Be stricter about access to generic vars from class This behaviour was intentionally chosen in https://github.com/python/mypy/pull/6418 I agree with Ivan's comment there that it's similar to how mypy allows instantiation of `type[Abstract]` -- but that's a thing that I want to explore disallowing. Let's see primer --- mypy/checkmember.py | 12 +++++------- test-data/unit/check-classvar.test | 22 ++++++++++++++++------ test-data/unit/check-generics.test | 4 ++-- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 9dc8d5475b1a..18468fac335c 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -1087,13 +1087,11 @@ def analyze_class_attribute_access( def_vars.add(node.node.info.self_type) typ_vars = set(get_type_vars(t)) if def_vars & typ_vars: - # Exception: access on Type[...], including first argument of class methods is OK. - if not isinstance(get_proper_type(mx.original_type), TypeType) or node.implicit: - if node.node.is_classvar: - message = message_registry.GENERIC_CLASS_VAR_ACCESS - else: - message = message_registry.GENERIC_INSTANCE_VAR_CLASS_ACCESS - mx.msg.fail(message, mx.context) + if node.node.is_classvar: + message = message_registry.GENERIC_CLASS_VAR_ACCESS + else: + message = message_registry.GENERIC_INSTANCE_VAR_CLASS_ACCESS + mx.msg.fail(message, mx.context) t = expand_self_type_if_needed(t, mx, node.node, itype, is_class=True) # Erase non-mapped variables, but keep mapped ones, even if there is an error. # In the above example this means that we infer following types: diff --git a/test-data/unit/check-classvar.test b/test-data/unit/check-classvar.test index 1e87e441dea2..935c8594a597 100644 --- a/test-data/unit/check-classvar.test +++ b/test-data/unit/check-classvar.test @@ -282,17 +282,27 @@ main:2: note: Revealed type is "builtins.int" main:3: error: Cannot assign to class variable "x" via instance [case testClassVarWithGeneric] -from typing import ClassVar, Generic, TypeVar +from typing import ClassVar, Generic, TypeVar, Type T = TypeVar('T') class A(Generic[T]): x: ClassVar[T] # E: ClassVar cannot contain type variables @classmethod def foo(cls) -> T: - return cls.x # OK + return cls.x # E: Access to generic class variables is ambiguous -A.x # E: Access to generic class variables is ambiguous -A.x = 1 # E: Access to generic class variables is ambiguous -A[int].x # E: Access to generic class variables is ambiguous +def main(A_T: Type[A]): + A.x # E: Access to generic class variables is ambiguous + A.x = 1 # E: Access to generic class variables is ambiguous + A[int].x # E: Access to generic class variables is ambiguous + + A_T.x # E: Access to generic class variables is ambiguous + A_T.x = 1 # E: Access to generic class variables is ambiguous + A_T[int].x # E: Value of type "Type[A[Any]]" is not indexable + + a = A + a.x # E: Access to generic class variables is ambiguous + a.x = 1 # E: Access to generic class variables is ambiguous + a[int].x # E: The type "Type[A[Any]]" is not generic and not indexable class Bad(A[int]): pass @@ -311,7 +321,7 @@ class A(Generic[T, U]): x: ClassVar[Union[T, Tuple[U, Type[U]]]] # E: ClassVar cannot contain type variables @classmethod def foo(cls) -> Union[T, Tuple[U, Type[U]]]: - return cls.x # OK + return cls.x # E: Access to generic class variables is ambiguous A.x # E: Access to generic class variables is ambiguous A.x = 1 # E: Access to generic class variables is ambiguous diff --git a/test-data/unit/check-generics.test b/test-data/unit/check-generics.test index b8cc0422b749..0cd3b03dca60 100644 --- a/test-data/unit/check-generics.test +++ b/test-data/unit/check-generics.test @@ -2225,7 +2225,7 @@ class C(Generic[T]): x: T @classmethod def get(cls) -> T: - return cls.x # OK + return cls.x # E: Access to generic instance variables via class is ambiguous x = C.x # E: Access to generic instance variables via class is ambiguous reveal_type(x) # N: Revealed type is "Any" @@ -2288,7 +2288,7 @@ reveal_type(b) # N: Revealed type is "__main__.B" def g(t: Type[Maker[T]]) -> T: if bool(): - return t.x + return t.x # E: Access to generic instance variables via class is ambiguous return t.get() bb = g(B) reveal_type(bb) # N: Revealed type is "__main__.B" From 26618b0a2dba5507fc41da423c0904b0545575dc Mon Sep 17 00:00:00 2001 From: Shantanu Jain Date: Fri, 27 Dec 2024 19:09:29 -0800 Subject: [PATCH 2/5] allow self type --- mypy/checkmember.py | 2 +- test-data/unit/check-generics.test | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 18468fac335c..20345f4773a1 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -1086,7 +1086,7 @@ def analyze_class_attribute_access( if not node.node.is_classvar and node.node.info.self_type: def_vars.add(node.node.info.self_type) typ_vars = set(get_type_vars(t)) - if def_vars & typ_vars: + if any(tv for tv in def_vars & typ_vars if tv.upper_bound != itype): if node.node.is_classvar: message = message_registry.GENERIC_CLASS_VAR_ACCESS else: diff --git a/test-data/unit/check-generics.test b/test-data/unit/check-generics.test index 0cd3b03dca60..a3297d471f6f 100644 --- a/test-data/unit/check-generics.test +++ b/test-data/unit/check-generics.test @@ -2233,6 +2233,22 @@ xi = C[int].x # E: Access to generic instance variables via class is ambiguous reveal_type(xi) # N: Revealed type is "builtins.int" [builtins fixtures/classmethod.pyi] +[case testGenericClassAttrSelfType] +from typing import TypeVar, Generic, Self, Type, Mapping + +class A: + d: Mapping[str, Self] + + @classmethod + def make(cls) -> Self: + return cls.d["asdf"] + +def main(A_t: Type[A]) -> None: + reveal_type(A.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.A]" + reveal_type(A_t.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.A]" + +[builtins fixtures/classmethod.pyi] + [case testGenericClassAttrUnboundOnSubClass] from typing import Generic, TypeVar, Tuple T = TypeVar('T') From 193e605f6889ffe071ab45ef98d7e215f679e066 Mon Sep 17 00:00:00 2001 From: Shantanu Jain Date: Fri, 27 Dec 2024 19:44:52 -0800 Subject: [PATCH 3/5] more --- mypy/checkmember.py | 12 +++++++++++- test-data/unit/check-generics.test | 8 +++++++- test-data/unit/check-selftype.test | 2 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 20345f4773a1..55732b2b5ca4 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -1085,13 +1085,23 @@ def analyze_class_attribute_access( def_vars = set(node.node.info.defn.type_vars) if not node.node.is_classvar and node.node.info.self_type: def_vars.add(node.node.info.self_type) + typ_vars = set(get_type_vars(t)) - if any(tv for tv in def_vars & typ_vars if tv.upper_bound != itype): + for tv in def_vars & typ_vars: + if ( + # Forgive access to variables generic over Self + isinstance(tv.upper_bound, Instance) + and super_info + and tv.upper_bound.type.fullname == super_info.fullname + ): + continue if node.node.is_classvar: message = message_registry.GENERIC_CLASS_VAR_ACCESS else: message = message_registry.GENERIC_INSTANCE_VAR_CLASS_ACCESS mx.msg.fail(message, mx.context) + break + t = expand_self_type_if_needed(t, mx, node.node, itype, is_class=True) # Erase non-mapped variables, but keep mapped ones, even if there is an error. # In the above example this means that we infer following types: diff --git a/test-data/unit/check-generics.test b/test-data/unit/check-generics.test index a3297d471f6f..bbbbe12dd950 100644 --- a/test-data/unit/check-generics.test +++ b/test-data/unit/check-generics.test @@ -2243,10 +2243,16 @@ class A: def make(cls) -> Self: return cls.d["asdf"] -def main(A_t: Type[A]) -> None: +class B(A): ... + +def check_A(A_t: Type[A]) -> None: reveal_type(A.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.A]" reveal_type(A_t.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.A]" +def check_B(B_t: Type[B]) -> None: + reveal_type(B.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.B]" + reveal_type(B_t.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.B]" + [builtins fixtures/classmethod.pyi] [case testGenericClassAttrUnboundOnSubClass] diff --git a/test-data/unit/check-selftype.test b/test-data/unit/check-selftype.test index fa853ac48e5a..90734bfd86c0 100644 --- a/test-data/unit/check-selftype.test +++ b/test-data/unit/check-selftype.test @@ -1485,7 +1485,7 @@ class C: class D(C): ... reveal_type(C.meth) # N: Revealed type is "def [Self <: __main__.C] (self: Self`1) -> builtins.list[Self`1]" -C.attr # E: Access to generic instance variables via class is ambiguous +C.attr reveal_type(D().meth()) # N: Revealed type is "builtins.list[__main__.D]" reveal_type(D().attr) # N: Revealed type is "builtins.list[__main__.D]" From fff6c41ec132015d3b53503b0c9e51edd490765e Mon Sep 17 00:00:00 2001 From: Shantanu Jain Date: Fri, 27 Dec 2024 20:15:53 -0800 Subject: [PATCH 4/5] self type --- mypy/checkmember.py | 17 ++++------------- test-data/unit/check-generics.test | 18 +++++++++++++++++- test-data/unit/check-selftype.test | 2 +- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 55732b2b5ca4..abb671473476 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -1081,26 +1081,17 @@ def analyze_class_attribute_access( # x: T # C.x # Error, ambiguous access # C[int].x # Also an error, since C[int] is same as C at runtime - # Exception is Self type wrapped in ClassVar, that is safe. + # Exception is Self type, which is always allowed def_vars = set(node.node.info.defn.type_vars) - if not node.node.is_classvar and node.node.info.self_type: - def_vars.add(node.node.info.self_type) - + if node.node.info.self_type: + def_vars.discard(node.node.info.self_type) typ_vars = set(get_type_vars(t)) - for tv in def_vars & typ_vars: - if ( - # Forgive access to variables generic over Self - isinstance(tv.upper_bound, Instance) - and super_info - and tv.upper_bound.type.fullname == super_info.fullname - ): - continue + if def_vars & typ_vars: if node.node.is_classvar: message = message_registry.GENERIC_CLASS_VAR_ACCESS else: message = message_registry.GENERIC_INSTANCE_VAR_CLASS_ACCESS mx.msg.fail(message, mx.context) - break t = expand_self_type_if_needed(t, mx, node.node, itype, is_class=True) # Erase non-mapped variables, but keep mapped ones, even if there is an error. diff --git a/test-data/unit/check-generics.test b/test-data/unit/check-generics.test index bbbbe12dd950..c432b0af2204 100644 --- a/test-data/unit/check-generics.test +++ b/test-data/unit/check-generics.test @@ -2234,7 +2234,7 @@ reveal_type(xi) # N: Revealed type is "builtins.int" [builtins fixtures/classmethod.pyi] [case testGenericClassAttrSelfType] -from typing import TypeVar, Generic, Self, Type, Mapping +from typing import TypeVar, Generic, Self, Type, Mapping, ClassVar class A: d: Mapping[str, Self] @@ -2253,6 +2253,22 @@ def check_B(B_t: Type[B]) -> None: reveal_type(B.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.B]" reveal_type(B_t.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.B]" +class AA: + d: ClassVar[Mapping[str, Self]] + + @classmethod + def make(cls) -> Self: + return cls.d["asdf"] + +class BB(AA): ... + +def check_AA(AA_t: Type[AA]) -> None: + reveal_type(AA.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.AA]" + reveal_type(AA_t.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.AA]" + +def check_BB(BB_t: Type[BB]) -> None: + reveal_type(BB.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.BB]" + reveal_type(BB_t.d) # N: Revealed type is "typing.Mapping[builtins.str, __main__.BB]" [builtins fixtures/classmethod.pyi] [case testGenericClassAttrUnboundOnSubClass] diff --git a/test-data/unit/check-selftype.test b/test-data/unit/check-selftype.test index 90734bfd86c0..bf95e15d4659 100644 --- a/test-data/unit/check-selftype.test +++ b/test-data/unit/check-selftype.test @@ -2120,7 +2120,7 @@ c: C reveal_type(c.x) # N: Revealed type is "Tuple[builtins.int, builtins.str, fallback=__main__.C]" reveal_type(c.y) # N: Revealed type is "Tuple[builtins.int, builtins.str, fallback=__main__.C]" reveal_type(C.y) # N: Revealed type is "Tuple[builtins.int, builtins.str, fallback=__main__.C]" -C.x # E: Access to generic instance variables via class is ambiguous +C.x [builtins fixtures/classmethod.pyi] [case testAccessingTypingSelfUnion] From 75e08705d283c90b96536c08c05d5ec01ea9c8a6 Mon Sep 17 00:00:00 2001 From: Shantanu Jain Date: Fri, 27 Dec 2024 21:56:31 -0800 Subject: [PATCH 5/5] fix mysterious ci error --- test-data/unit/check-classvar.test | 1 + 1 file changed, 1 insertion(+) diff --git a/test-data/unit/check-classvar.test b/test-data/unit/check-classvar.test index 935c8594a597..16e19b3e1fdc 100644 --- a/test-data/unit/check-classvar.test +++ b/test-data/unit/check-classvar.test @@ -282,6 +282,7 @@ main:2: note: Revealed type is "builtins.int" main:3: error: Cannot assign to class variable "x" via instance [case testClassVarWithGeneric] +import types from typing import ClassVar, Generic, TypeVar, Type T = TypeVar('T') class A(Generic[T]):