From 8d517090c4a93590f8cf40c3ca23d8c0a7c81527 Mon Sep 17 00:00:00 2001 From: wyfo Date: Mon, 14 Jun 2021 01:34:28 +0200 Subject: [PATCH 1/7] Fix callable instance variable support (#10548) Fixes #708 and Fixes #5485 Prevent handling as bounded method of callable members declared as instance variables. --- mypy/checkmember.py | 33 ++++++++---- test-data/unit/check-dataclasses.test | 76 +++++++-------------------- test-data/unit/check-functions.test | 40 +++++++++----- test-data/unit/check-functools.test | 6 +-- test-data/unit/check-selftype.test | 18 +++---- 5 files changed, 79 insertions(+), 94 deletions(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index a2f9db117325..5f380cce21b6 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -659,15 +659,23 @@ def instance_alias_type(alias: TypeAlias, named_type: Callable[[str], Instance]) return expand_type_by_instance(tp, target) -def analyze_var( - name: str, - var: Var, - itype: Instance, - info: TypeInfo, - mx: MemberContext, - *, - implicit: bool = False, -) -> Type: +def is_instance_var(var: Var, info: TypeInfo) -> bool: + """Return if var is an instance variable according to PEP 526.""" + return ( + # check the type_info node is the var (not a decorated function, etc.) + var.name in info.names and info.names[var.name].node is var + and not var.is_classvar + # variables without annotations are treated as classvar + and not var.is_inferred + ) + + +def analyze_var(name: str, + var: Var, + itype: Instance, + info: TypeInfo, + mx: MemberContext, *, + implicit: bool = False) -> Type: """Analyze access to an attribute via a Var node. This is conceptually part of analyze_member_access and the arguments are similar. @@ -690,7 +698,12 @@ def analyze_var( t = get_proper_type(expand_type_by_instance(typ, itype)) result: Type = t typ = get_proper_type(typ) - if var.is_initialized_in_class and isinstance(typ, FunctionLike) and not typ.is_type_obj(): + if ( + var.is_initialized_in_class + and not is_instance_var(var, info) + and isinstance(typ, FunctionLike) + and not typ.is_type_obj() + ): if mx.is_lvalue: if var.is_property: if not var.is_settable_property: diff --git a/test-data/unit/check-dataclasses.test b/test-data/unit/check-dataclasses.test index 40c6b66d5c39..c8382f52b7f0 100644 --- a/test-data/unit/check-dataclasses.test +++ b/test-data/unit/check-dataclasses.test @@ -1304,80 +1304,40 @@ reveal_type(A.__dataclass_fields__) # N: Revealed type is "builtins.dict[builti [builtins fixtures/dict.pyi] -[case testDataclassCallableProperty] +[case testDataclassCallableFieldAccess] # flags: --python-version 3.7 from dataclasses import dataclass from typing import Callable @dataclass class A: - foo: Callable[[int], int] + x: Callable[[int], int] + y: Callable[[int], int] = lambda i: i -def my_foo(x: int) -> int: - return x +a = A(lambda i:i) +x: int = a.x(0) +y: str = a.y(0) # E: Incompatible types in assignment (expression has type "int", variable has type "str") +reveal_type(a.x) # N: Revealed type is "def (builtins.int) -> builtins.int" +reveal_type(a.y) # N: Revealed type is "def (builtins.int) -> builtins.int" +reveal_type(A.y) # N: Revealed type is "def (builtins.int) -> builtins.int" -a = A(foo=my_foo) -a.foo(1) -reveal_type(a.foo) # N: Revealed type is "def (builtins.int) -> builtins.int" -reveal_type(A.foo) # N: Revealed type is "def (builtins.int) -> builtins.int" -[typing fixtures/typing-medium.pyi] -[builtins fixtures/dataclasses.pyi] - -[case testDataclassCallableAssignment] -# flags: --python-version 3.7 -from dataclasses import dataclass -from typing import Callable - -@dataclass -class A: - foo: Callable[[int], int] - -def my_foo(x: int) -> int: - return x - -a = A(foo=my_foo) - -def another_foo(x: int) -> int: - return x - -a.foo = another_foo -[builtins fixtures/dataclasses.pyi] - -[case testDataclassCallablePropertyWrongType] +[case testDataclassCallableFieldAssignment] # flags: --python-version 3.7 from dataclasses import dataclass from typing import Callable @dataclass class A: - foo: Callable[[int], int] + x: Callable[[int], int] -def my_foo(x: int) -> str: - return "foo" +def x(i: int) -> int: + return i +def x2(s: str) -> str: + return s -a = A(foo=my_foo) # E: Argument "foo" to "A" has incompatible type "Callable[[int], str]"; expected "Callable[[int], int]" -[typing fixtures/typing-medium.pyi] -[builtins fixtures/dataclasses.pyi] - -[case testDataclassCallablePropertyWrongTypeAssignment] -# flags: --python-version 3.7 -from dataclasses import dataclass -from typing import Callable - -@dataclass -class A: - foo: Callable[[int], int] - -def my_foo(x: int) -> int: - return x - -a = A(foo=my_foo) - -def another_foo(x: int) -> str: - return "foo" - -a.foo = another_foo # E: Incompatible types in assignment (expression has type "Callable[[int], str]", variable has type "Callable[[int], int]") -[typing fixtures/typing-medium.pyi] +a = A(lambda i:i) +a.x = x +a.x = x2 # E: Incompatible types in assignment (expression has type "Callable[[str], str]", variable has type "Callable[[int], int]") [builtins fixtures/dataclasses.pyi] [case testDataclassFieldDoesNotFailOnKwargsUnpacking] diff --git a/test-data/unit/check-functions.test b/test-data/unit/check-functions.test index a91b6099a02e..f6297f147e7a 100644 --- a/test-data/unit/check-functions.test +++ b/test-data/unit/check-functions.test @@ -571,12 +571,12 @@ A().f('') # E: Argument 1 to "f" of "A" has incompatible type "str"; expected "i [case testMethodAsDataAttribute] -from typing import Any, Callable +from typing import Any, Callable, ClassVar class B: pass x = None # type: Any class A: - f = x # type: Callable[[A], None] - g = x # type: Callable[[A, B], None] + f = x # type: ClassVar[Callable[[A], None]] + g = x # type: ClassVar[Callable[[A, B], None]] a = None # type: A a.f() a.g(B()) @@ -584,26 +584,38 @@ a.f(a) # E: Too many arguments a.g() # E: Too few arguments [case testMethodWithInvalidMethodAsDataAttribute] -from typing import Any, Callable +from typing import Any, Callable, ClassVar class B: pass x = None # type: Any class A: - f = x # type: Callable[[], None] - g = x # type: Callable[[B], None] + f = x # type: ClassVar[Callable[[], None]] + g = x # type: ClassVar[Callable[[B], None]] a = None # type: A a.f() # E: Attribute function "f" with type "Callable[[], None]" does not accept self argument a.g() # E: Invalid self argument "A" to attribute function "g" with type "Callable[[B], None]" [case testMethodWithDynamicallyTypedMethodAsDataAttribute] -from typing import Any, Callable +from typing import Any, Callable, ClassVar class B: pass x = None # type: Any class A: - f = x # type: Callable[[Any], Any] + f = x # type: ClassVar[Callable[[Any], Any]] a = None # type: A a.f() a.f(a) # E: Too many arguments +[case testMethodWithInferredMethodAsDataAttribute] +from typing import Any +def m(self: "A") -> int: ... + +class A: + n = m + +a = A() +reveal_type(a.n()) # N: Revealed type is "builtins.int" +reveal_type(A.n(a)) # N: Revealed type is "builtins.int" +A.n() # E: Too few arguments + [case testOverloadedMethodAsDataAttribute] from foo import * [file foo.pyi] @@ -645,35 +657,35 @@ a.g(B()) a.g(a) # E: Argument 1 has incompatible type "A[B]"; expected "B" [case testInvalidMethodAsDataAttributeInGenericClass] -from typing import Any, TypeVar, Generic, Callable +from typing import Any, TypeVar, Generic, Callable, ClassVar t = TypeVar('t') class B: pass class C: pass x = None # type: Any class A(Generic[t]): - f = x # type: Callable[[A[B]], None] + f = x # type: ClassVar[Callable[[A[B]], None]] ab = None # type: A[B] ac = None # type: A[C] ab.f() ac.f() # E: Invalid self argument "A[C]" to attribute function "f" with type "Callable[[A[B]], None]" [case testPartiallyTypedSelfInMethodDataAttribute] -from typing import Any, TypeVar, Generic, Callable +from typing import Any, TypeVar, Generic, Callable, ClassVar t = TypeVar('t') class B: pass class C: pass x = None # type: Any class A(Generic[t]): - f = x # type: Callable[[A], None] + f = x # type: ClassVar[Callable[[A], None]] ab = None # type: A[B] ac = None # type: A[C] ab.f() ac.f() [case testCallableDataAttribute] -from typing import Callable +from typing import Callable, ClassVar class A: - g = None # type: Callable[[A], None] + g = None # type: ClassVar[Callable[[A], None]] def __init__(self, f: Callable[[], None]) -> None: self.f = f a = A(None) diff --git a/test-data/unit/check-functools.test b/test-data/unit/check-functools.test index a2c6ba2eee05..17d3933b40bc 100644 --- a/test-data/unit/check-functools.test +++ b/test-data/unit/check-functools.test @@ -25,12 +25,12 @@ Ord() >= 1 # E: Unsupported operand types for >= ("Ord" and "int") [case testTotalOrderingLambda] from functools import total_ordering -from typing import Any, Callable +from typing import Any, Callable, ClassVar @total_ordering class Ord: - __eq__: Callable[[Any, object], bool] = lambda self, other: False - __lt__: Callable[[Any, "Ord"], bool] = lambda self, other: False + __eq__: ClassVar[Callable[[Any, object], bool]] = lambda self, other: False + __lt__: ClassVar[Callable[[Any, "Ord"], bool]] = lambda self, other: False reveal_type(Ord() < Ord()) # N: Revealed type is "builtins.bool" reveal_type(Ord() <= Ord()) # N: Revealed type is "builtins.bool" diff --git a/test-data/unit/check-selftype.test b/test-data/unit/check-selftype.test index ebbfe4e8ae8a..506e8bfe8ab1 100644 --- a/test-data/unit/check-selftype.test +++ b/test-data/unit/check-selftype.test @@ -366,7 +366,7 @@ reveal_type(x.f) # N: Revealed type is "builtins.int" [builtins fixtures/property.pyi] [case testSelfTypeProperSupertypeAttribute] -from typing import Callable, TypeVar +from typing import Callable, TypeVar, ClassVar class K: pass T = TypeVar('T', bound=K) class A(K): @@ -374,8 +374,8 @@ class A(K): def g(self: K) -> int: return 0 @property def gt(self: T) -> T: return self - f: Callable[[object], int] - ft: Callable[[T], T] + f: ClassVar[Callable[[object], int]] + ft: ClassVar[Callable[[T], T]] class B(A): pass @@ -392,15 +392,15 @@ reveal_type(B().ft()) # N: Revealed type is "__main__.B" [builtins fixtures/property.pyi] [case testSelfTypeProperSupertypeAttributeTuple] -from typing import Callable, TypeVar, Tuple +from typing import Callable, TypeVar, Tuple, ClassVar T = TypeVar('T') class A(Tuple[int, int]): @property def g(self: object) -> int: return 0 @property def gt(self: T) -> T: return self - f: Callable[[object], int] - ft: Callable[[T], T] + f: ClassVar[Callable[[object], int]] + ft: ClassVar[Callable[[T], T]] class B(A): pass @@ -450,7 +450,7 @@ reveal_type(X1.ft()) # N: Revealed type is "Type[__main__.X]" [builtins fixtures/property.pyi] [case testSelfTypeProperSupertypeAttributeGeneric] -from typing import Callable, TypeVar, Generic +from typing import Callable, TypeVar, Generic, ClassVar Q = TypeVar('Q', covariant=True) class K(Generic[Q]): q: Q @@ -460,8 +460,8 @@ class A(K[Q]): def g(self: K[object]) -> int: return 0 @property def gt(self: K[T]) -> T: return self.q - f: Callable[[object], int] - ft: Callable[[T], T] + f: ClassVar[Callable[[object], int]] + ft: ClassVar[Callable[[T], T]] class B(A[Q]): pass From e8e2413184f6bc9e405d1a302bad097d7372d7c6 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 12 Aug 2022 22:31:36 +0100 Subject: [PATCH 2/7] Fix cherry-pick --- test-data/unit/check-dataclasses.test | 1 + 1 file changed, 1 insertion(+) diff --git a/test-data/unit/check-dataclasses.test b/test-data/unit/check-dataclasses.test index c8382f52b7f0..6abb5597e464 100644 --- a/test-data/unit/check-dataclasses.test +++ b/test-data/unit/check-dataclasses.test @@ -1320,6 +1320,7 @@ y: str = a.y(0) # E: Incompatible types in assignment (expression has type "int" reveal_type(a.x) # N: Revealed type is "def (builtins.int) -> builtins.int" reveal_type(a.y) # N: Revealed type is "def (builtins.int) -> builtins.int" reveal_type(A.y) # N: Revealed type is "def (builtins.int) -> builtins.int" +[builtins fixtures/dataclasses.pyi] [case testDataclassCallableFieldAssignment] # flags: --python-version 3.7 From 61557b0be2d6670dc63c5206e3ce8cdeb84fbbac Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 12 Aug 2022 22:32:36 +0100 Subject: [PATCH 3/7] Re-apply black --- mypy/checkmember.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 5f380cce21b6..9891033415de 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -663,19 +663,23 @@ def is_instance_var(var: Var, info: TypeInfo) -> bool: """Return if var is an instance variable according to PEP 526.""" return ( # check the type_info node is the var (not a decorated function, etc.) - var.name in info.names and info.names[var.name].node is var + var.name in info.names + and info.names[var.name].node is var and not var.is_classvar # variables without annotations are treated as classvar and not var.is_inferred ) -def analyze_var(name: str, - var: Var, - itype: Instance, - info: TypeInfo, - mx: MemberContext, *, - implicit: bool = False) -> Type: +def analyze_var( + name: str, + var: Var, + itype: Instance, + info: TypeInfo, + mx: MemberContext, + *, + implicit: bool = False, +) -> Type: """Analyze access to an attribute via a Var node. This is conceptually part of analyze_member_access and the arguments are similar. From 1e08b6e579c5d6c1240208d726c43b6ceebe34cf Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 12 Aug 2022 22:37:17 +0100 Subject: [PATCH 4/7] Fix incremental bug --- mypy/nodes.py | 1 + test-data/unit/check-incremental.test | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/mypy/nodes.py b/mypy/nodes.py index b7b3a6ef87f3..4eb5f2c0e4e5 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -939,6 +939,7 @@ def deserialize(cls, data: JsonDict) -> "Decorator": "final_set_in_init", "explicit_self_type", "is_ready", + "is_inferred", "from_module_getattr", "has_explicit_value", "allow_incompatible_override", diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 0cf048bee959..d63fc60dc3d8 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -5659,6 +5659,23 @@ class D(C): [out2] tmp/a.py:9: error: Trying to assign name "z" that is not in "__slots__" of type "a.D" +[case testMethodAliasIncremental] +import b +[file a.py] +class A: + def f(self) -> None: pass + g = f + +[file b.py] +from a import A +A().g() +[file b.py.2] +# trivial change +from a import A +A().g() +[out] +[out2] + [case testIncrementalWithDifferentKindsOfNestedTypesWithinMethod] # flags: --python-version 3.7 From 6a7f91b0a04401670377ff82f9af7e6ae23d01db Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 12 Aug 2022 23:02:30 +0100 Subject: [PATCH 5/7] Fix tests; allow operators --- mypy/checkmember.py | 2 +- mypy/semanal.py | 6 +++++- test-data/unit/check-functools.test | 4 ++-- test-data/unit/check-slots.test | 3 +-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 9891033415de..1ee2d64e25f0 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -704,7 +704,7 @@ def analyze_var( typ = get_proper_type(typ) if ( var.is_initialized_in_class - and not is_instance_var(var, info) + and (not is_instance_var(var, info) or mx.is_operator) and isinstance(typ, FunctionLike) and not typ.is_type_obj() ): diff --git a/mypy/semanal.py b/mypy/semanal.py index 2a30783d5bdc..88565058e146 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3851,10 +3851,14 @@ def check_classvar(self, s: AssignmentStmt) -> None: if isinstance(node, Var): node.is_classvar = True analyzed = self.anal_type(s.type) - if analyzed is not None and get_type_vars(analyzed): + assert self.type is not None + if analyzed is not None and set(get_type_vars(analyzed)) & set( + self.type.defn.type_vars + ): # This means that we have a type var defined inside of a ClassVar. # This is not allowed by PEP526. # See https://github.com/python/mypy/issues/11538 + self.fail(message_registry.CLASS_VAR_WITH_TYPEVARS, s) elif not isinstance(lvalue, MemberExpr) or self.is_self_member_ref(lvalue): # In case of member access, report error only when assigning to self diff --git a/test-data/unit/check-functools.test b/test-data/unit/check-functools.test index 17d3933b40bc..9233ece6ccfc 100644 --- a/test-data/unit/check-functools.test +++ b/test-data/unit/check-functools.test @@ -29,8 +29,8 @@ from typing import Any, Callable, ClassVar @total_ordering class Ord: - __eq__: ClassVar[Callable[[Any, object], bool]] = lambda self, other: False - __lt__: ClassVar[Callable[[Any, "Ord"], bool]] = lambda self, other: False + __eq__: Callable[[Any, object], bool] = lambda self, other: False + __lt__: Callable[[Any, "Ord"], bool] = lambda self, other: False reveal_type(Ord() < Ord()) # N: Revealed type is "builtins.bool" reveal_type(Ord() <= Ord()) # N: Revealed type is "builtins.bool" diff --git a/test-data/unit/check-slots.test b/test-data/unit/check-slots.test index 254aa983f82f..96e4eba3c966 100644 --- a/test-data/unit/check-slots.test +++ b/test-data/unit/check-slots.test @@ -361,8 +361,7 @@ a.a = 1 a.b = custom_obj a.c = custom_obj a.d = custom_obj -# TODO: Should this be allowed? -a.e = custom_obj # E: Cannot assign to a method +a.e = custom_obj [out] [builtins fixtures/tuple.pyi] [builtins fixtures/dict.pyi] From d10bea60f1e8892e7bec037653579265fab9fba9 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 12 Aug 2022 23:22:33 +0100 Subject: [PATCH 6/7] Add a ClassVar diagnostic note --- mypy/checkexpr.py | 27 ++++++++++++++++++++++++++- test-data/unit/check-classvar.test | 9 +++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index df349a3cb5bc..ba77a582ceac 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -1318,7 +1318,14 @@ def check_callable_call( arg_types = self.infer_arg_types_in_context(callee, args, arg_kinds, formal_to_actual) self.check_argument_count( - callee, arg_types, arg_kinds, arg_names, formal_to_actual, context + callee, + arg_types, + arg_kinds, + arg_names, + formal_to_actual, + context, + object_type, + callable_name, ) self.check_argument_types( @@ -1723,6 +1730,8 @@ def check_argument_count( actual_names: Optional[Sequence[Optional[str]]], formal_to_actual: List[List[int]], context: Optional[Context], + object_type: Optional[Type] = None, + callable_name: Optional[str] = None, ) -> bool: """Check that there is a value for all required arguments to a function. @@ -1753,6 +1762,8 @@ def check_argument_count( # No actual for a mandatory formal if kind.is_positional(): self.msg.too_few_arguments(callee, context, actual_names) + if object_type and callable_name and "." in callable_name: + self.missing_classvar_callable_note(object_type, callable_name, context) else: argname = callee.arg_names[i] or "?" self.msg.missing_named_argument(callee, context, argname) @@ -1836,6 +1847,20 @@ def check_for_extra_actual_arguments( return ok, is_unexpected_arg_error + def missing_classvar_callable_note( + self, object_type: Type, callable_name: str, context: Context + ) -> None: + if isinstance(object_type, ProperType) and isinstance(object_type, Instance): + _, var_name = callable_name.rsplit(".", maxsplit=1) + node = object_type.type.get(var_name) + if node is not None and isinstance(node.node, Var): + if not node.node.is_inferred: + self.msg.note( + f'"{var_name}" is considered instance variable,' + " to make it class variable use ClassVar[...]", + context, + ) + def check_argument_types( self, arg_types: List[Type], diff --git a/test-data/unit/check-classvar.test b/test-data/unit/check-classvar.test index d84bc8d5bf9d..1e87e441dea2 100644 --- a/test-data/unit/check-classvar.test +++ b/test-data/unit/check-classvar.test @@ -325,3 +325,12 @@ class Good(A[int, str]): x = 42 reveal_type(Good.x) # N: Revealed type is "builtins.int" [builtins fixtures/classmethod.pyi] + +[case testSuggestClassVarOnTooFewArgumentsMethod] +from typing import Callable + +class C: + foo: Callable[[C], int] +c:C +c.foo() # E: Too few arguments \ + # N: "foo" is considered instance variable, to make it class variable use ClassVar[...] From da235c445d877270ef92367ea9ffbbbff7583715 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 12 Aug 2022 23:53:13 +0100 Subject: [PATCH 7/7] Fix a test; add docs --- docs/source/class_basics.rst | 16 ++++++++++++++++ mypy/checkexpr.py | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/docs/source/class_basics.rst b/docs/source/class_basics.rst index 2b0c8cf1b490..1eaba59a10c2 100644 --- a/docs/source/class_basics.rst +++ b/docs/source/class_basics.rst @@ -147,6 +147,22 @@ a :py:data:`~typing.ClassVar` annotation, but this might not do what you'd expec In this case the type of the attribute will be implicitly ``Any``. This behavior will change in the future, since it's surprising. +An explicit :py:data:`~typing.ClassVar` may be particularly handy to distinguish +between class and instance variables with callable types. For example: + +.. code-block:: python + + from typing import Callable, ClassVar + + class A: + foo: Callable[[int], None] + bar: ClassVar[Callable[[A, int], None]] + bad: Callable[[A], None] + + A().foo(42) # OK + A().bar(42) # OK + A().bad() # Error: Too few arguments + .. note:: A :py:data:`~typing.ClassVar` type parameter cannot include type variables: ``ClassVar[T]`` and ``ClassVar[list[T]]`` diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index ba77a582ceac..598fe952fafa 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -1854,7 +1854,7 @@ def missing_classvar_callable_note( _, var_name = callable_name.rsplit(".", maxsplit=1) node = object_type.type.get(var_name) if node is not None and isinstance(node.node, Var): - if not node.node.is_inferred: + if not node.node.is_inferred and not node.node.is_classvar: self.msg.note( f'"{var_name}" is considered instance variable,' " to make it class variable use ClassVar[...]",