Skip to content

Commit

Permalink
Fix callable instance variable support (take 2) (#13400)
Browse files Browse the repository at this point in the history
Fixes #708
Fixes #5485

This builds on the original proposal, but handles three important issues/edge cases:
* This PR fixes serialization of `is_inferred` so that the distinction works correctly in incremental mode (I added a test)
* Dunder operator methods are always considered class variables (this is a relatively common pattern and matches Python semantics; there is an existing tests that previously needed `ClassVar[...]`)
* If we detect a `Too few arguments` error for a variable with callable type we give a note suggesting to try `ClassVar[...]`

I also add a short doc paragraph on this.

Co-authored-by: wyfo <joperez@hotmail.fr>
  • Loading branch information
ilevkivskyi and wyfo authored Aug 13, 2022
1 parent a0e2a2d commit dc9c304
Show file tree
Hide file tree
Showing 12 changed files with 147 additions and 86 deletions.
16 changes: 16 additions & 0 deletions docs/source/class_basics.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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]]``
Expand Down
27 changes: 26 additions & 1 deletion mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 and not node.node.is_classvar:
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],
Expand Down
19 changes: 18 additions & 1 deletion mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,18 @@ def instance_alias_type(alias: TypeAlias, named_type: Callable[[str], Instance])
return expand_type_by_instance(tp, target)


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,
Expand Down Expand Up @@ -690,7 +702,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) or mx.is_operator)
and isinstance(typ, FunctionLike)
and not typ.is_type_obj()
):
if mx.is_lvalue:
if var.is_property:
if not var.is_settable_property:
Expand Down
1 change: 1 addition & 0 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 5 additions & 1 deletion mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions test-data/unit/check-classvar.test
Original file line number Diff line number Diff line change
Expand Up @@ -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[...]
75 changes: 18 additions & 57 deletions test-data/unit/check-dataclasses.test
Original file line number Diff line number Diff line change
Expand Up @@ -1304,80 +1304,41 @@ 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(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
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"
[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]
Expand Down
40 changes: 26 additions & 14 deletions test-data/unit/check-functions.test
Original file line number Diff line number Diff line change
Expand Up @@ -571,39 +571,51 @@ 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())
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]
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-functools.test
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ 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:
Expand Down
17 changes: 17 additions & 0 deletions test-data/unit/check-incremental.test
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading

0 comments on commit dc9c304

Please sign in to comment.