Skip to content

Commit bf9dc4c

Browse files
elazargJukkaL
elazarg
authored and
JukkaL
committed
Fix checking of self when accessing a non-method callable attribute (take 2) (#4016)
Fix #3223. Minimal examples: ``` from typing import Callable class X: f: Callable[[object], None] X().f # E: Invalid method type ``` ``` class X: @Property def g(self: object): pass X().g # E: Invalid method type ``` The problem is that for non-methods the check performed on access to self is whether object <: X instead of the other way around. 1. Reverse subtyping check. This change alone fixes the issue described above. 2. For classmethod, fall back on the argument instead of on the parameter. 3. Mimic dispatch better: meet original_type with static class. This handles the case were instead of X() above there was something of type Union[X, Y]. 4. Better error message. Take 2 of #3227, but without the reverse-operator part.
1 parent 5bdf0bf commit bf9dc4c

5 files changed

Lines changed: 181 additions & 46 deletions

File tree

‎mypy/checkmember.py‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
from mypy.plugin import Plugin, AttributeContext
2121
from mypy import messages
2222
from mypy import subtypes
23+
from mypy import meet
24+
2325
MYPY = False
2426
if MYPY: # import for forward declaration only
2527
import mypy.checker
@@ -295,6 +297,7 @@ def analyze_var(name: str, var: Var, itype: Instance, info: TypeInfo, node: Cont
295297
296298
This is conceptually part of analyze_member_access and the arguments are similar.
297299
300+
itype is the class object in which var is dedined
298301
original_type is the type of E in the expression E.var
299302
"""
300303
# Found a member variable.
@@ -319,15 +322,21 @@ def analyze_var(name: str, var: Var, itype: Instance, info: TypeInfo, node: Cont
319322
msg.cant_assign_to_method(node)
320323

321324
if not var.is_staticmethod:
322-
# Class-level function objects and classmethods become bound
323-
# methods: the former to the instance, the latter to the
324-
# class.
325+
# Class-level function objects and classmethods become bound methods:
326+
# the former to the instance, the latter to the class.
325327
functype = t
326-
check_method_type(functype, itype, var.is_classmethod, node, msg)
328+
# Use meet to narrow original_type to the dispatched type.
329+
# For example, assume
330+
# * A.f: Callable[[A1], None] where A1 <: A (maybe A1 == A)
331+
# * B.f: Callable[[B1], None] where B1 <: B (maybe B1 == B)
332+
# * x: Union[A1, B1]
333+
# In `x.f`, when checking `x` against A1 we assume x is compatible with A
334+
# and similarly for B1 when checking agains B
335+
dispatched_type = meet.meet_types(original_type, itype)
336+
check_self_arg(functype, dispatched_type, var.is_classmethod, node, name, msg)
327337
signature = bind_self(functype, original_type, var.is_classmethod)
328338
if var.is_property:
329-
# A property cannot have an overloaded type => the cast
330-
# is fine.
339+
# A property cannot have an overloaded type => the cast is fine.
331340
assert isinstance(signature, CallableType)
332341
result = signature.ret_type
333342
else:
@@ -379,33 +388,28 @@ def lookup_member_var_or_accessor(info: TypeInfo, name: str,
379388
return None
380389

381390

382-
def check_method_type(functype: FunctionLike, itype: Instance, is_classmethod: bool,
383-
context: Context, msg: MessageBuilder) -> None:
391+
def check_self_arg(functype: FunctionLike, dispatched_arg_type: Type, is_classmethod: bool,
392+
context: Context, name: str, msg: MessageBuilder) -> None:
393+
"""For x.f where A.f: A1 -> T, check that meet(type(x), A) <: A1 for each overload.
394+
395+
dispatched_arg_type is meet(B, A) in the following example
396+
397+
def g(x: B): x.f
398+
class A:
399+
f: Callable[[A1], None]
400+
"""
401+
# TODO: this is too strict. We can return filtered overloads for matching definitions
384402
for item in functype.items():
385403
if not item.arg_types or item.arg_kinds[0] not in (ARG_POS, ARG_STAR):
386404
# No positional first (self) argument (*args is okay).
387-
msg.invalid_method_type(item, context)
388-
elif not is_classmethod:
389-
# Check that self argument has type 'Any' or valid instance type.
390-
selfarg = item.arg_types[0]
391-
# If this is a method of a tuple class, correct for the fact that
392-
# we passed to typ.fallback in analyze_member_access. See #1432.
393-
if isinstance(selfarg, TupleType):
394-
selfarg = selfarg.fallback
395-
if not subtypes.is_subtype(selfarg, itype):
396-
msg.invalid_method_type(item, context)
405+
msg.no_formal_self(name, item, context)
397406
else:
398-
# Check that cls argument has type 'Any' or valid class type.
399-
# (This is sufficient for the current treatment of @classmethod,
400-
# but probably needs to be revisited when we implement Type[C]
401-
# or advanced variants of it like Type[<args>, C].)
402-
clsarg = item.arg_types[0]
403-
if isinstance(clsarg, CallableType) and clsarg.is_type_obj():
404-
if not subtypes.is_equivalent(clsarg.ret_type, itype):
405-
msg.invalid_class_method_type(item, context)
406-
else:
407-
if not subtypes.is_equivalent(clsarg, AnyType(TypeOfAny.special_form)):
408-
msg.invalid_class_method_type(item, context)
407+
selfarg = item.arg_types[0]
408+
if is_classmethod:
409+
dispatched_arg_type = TypeType.make_normalized(dispatched_arg_type)
410+
if not subtypes.is_subtype(dispatched_arg_type, erase_to_bound(selfarg)):
411+
msg.incompatible_self_argument(name, dispatched_arg_type, item,
412+
is_classmethod, context)
409413

410414

411415
def analyze_class_attribute_access(itype: Instance,

‎mypy/messages.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -863,11 +863,15 @@ def cannot_determine_type(self, name: str, context: Context) -> None:
863863
def cannot_determine_type_in_base(self, name: str, base: str, context: Context) -> None:
864864
self.fail("Cannot determine type of '%s' in base class '%s'" % (name, base), context)
865865

866-
def invalid_method_type(self, sig: CallableType, context: Context) -> None:
867-
self.fail('Invalid method type', context)
868-
869-
def invalid_class_method_type(self, sig: CallableType, context: Context) -> None:
870-
self.fail('Invalid class method type', context)
866+
def no_formal_self(self, name: str, item: CallableType, context: Context) -> None:
867+
self.fail('Attribute function "%s" with type %s does not accept self argument'
868+
% (name, self.format(item)), context)
869+
870+
def incompatible_self_argument(self, name: str, arg: Type, sig: CallableType,
871+
is_classmethod: bool, context: Context) -> None:
872+
kind = 'class attribute function' if is_classmethod else 'attribute function'
873+
self.fail('Invalid self argument %s to %s "%s" with type %s'
874+
% (self.format(arg), kind, name, self.format(sig)), context)
871875

872876
def incompatible_conditional_function_def(self, defn: FuncDef) -> None:
873877
self.fail('All conditional function variants must have identical '

‎test-data/unit/check-classes.test‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2105,7 +2105,7 @@ class B:
21052105
a = A
21062106
bad = lambda: 42
21072107

2108-
B().bad() # E: Invalid method type
2108+
B().bad() # E: Attribute function "bad" with type "Callable[[], int]" does not accept self argument
21092109
reveal_type(B.a) # E: Revealed type is 'def () -> __main__.A'
21102110
reveal_type(B().a) # E: Revealed type is 'def () -> __main__.A'
21112111
reveal_type(B().a()) # E: Revealed type is '__main__.A'

‎test-data/unit/check-functions.test‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,8 @@ class A:
507507
f = x # type: Callable[[], None]
508508
g = x # type: Callable[[B], None]
509509
a = None # type: A
510-
a.f() # E: Invalid method type
511-
a.g() # E: Invalid method type
510+
a.f() # E: Attribute function "f" with type "Callable[[], None]" does not accept self argument
511+
a.g() # E: Invalid self argument "A" to attribute function "g" with type "Callable[[B], None]"
512512

513513
[case testMethodWithDynamicallyTypedMethodAsDataAttribute]
514514
from typing import Any, Callable
@@ -568,7 +568,7 @@ class A(Generic[t]):
568568
ab = None # type: A[B]
569569
ac = None # type: A[C]
570570
ab.f()
571-
ac.f() # E: Invalid method type
571+
ac.f() # E: Invalid self argument "A[C]" to attribute function "f" with type "Callable[[A[B]], None]"
572572

573573
[case testPartiallyTypedSelfInMethodDataAttribute]
574574
from typing import Any, TypeVar, Generic, Callable

‎test-data/unit/check-selftype.test‎

Lines changed: 135 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -351,21 +351,130 @@ class E:
351351
def __init_subclass__(cls) -> None:
352352
reveal_type(cls) # E: Revealed type is 'def () -> __main__.E'
353353

354-
[case testSelfTypeProperty]
355-
from typing import TypeVar
354+
[case testSelfTypePropertyUnion]
355+
from typing import Union
356+
class A:
357+
@property
358+
def f(self: A) -> int: pass
356359

357-
T = TypeVar('T', bound='A')
360+
class B:
361+
@property
362+
def f(self: B) -> int: pass
363+
x: Union[A, B]
364+
reveal_type(x.f) # E: Revealed type is 'builtins.int'
358365

359-
class A:
366+
[builtins fixtures/property.pyi]
367+
368+
[case testSelfTypeProperSupertypeAttribute]
369+
from typing import Callable, TypeVar
370+
class K: pass
371+
T = TypeVar('T', bound=K)
372+
class A(K):
360373
@property
361-
def member(self: T) -> T:
362-
pass
374+
def g(self: K) -> int: return 0
375+
@property
376+
def gt(self: T) -> T: return self
377+
f: Callable[[object], int]
378+
ft: Callable[[T], T]
379+
380+
class B(A):
381+
pass
382+
383+
reveal_type(A().g) # E: Revealed type is 'builtins.int'
384+
reveal_type(A().gt) # E: Revealed type is '__main__.A*'
385+
reveal_type(A().f()) # E: Revealed type is 'builtins.int'
386+
reveal_type(A().ft()) # E: Revealed type is '__main__.A*'
387+
reveal_type(B().g) # E: Revealed type is 'builtins.int'
388+
reveal_type(B().gt) # E: Revealed type is '__main__.B*'
389+
reveal_type(B().f()) # E: Revealed type is 'builtins.int'
390+
reveal_type(B().ft()) # E: Revealed type is '__main__.B*'
391+
392+
[builtins fixtures/property.pyi]
393+
394+
[case testSelfTypeProperSupertypeAttributeTuple]
395+
from typing import Callable, TypeVar, Tuple
396+
T = TypeVar('T')
397+
class A(Tuple[int, int]):
398+
@property
399+
def g(self: object) -> int: return 0
400+
@property
401+
def gt(self: T) -> T: return self
402+
f: Callable[[object], int]
403+
ft: Callable[[T], T]
363404

364405
class B(A):
365406
pass
366407

367-
reveal_type(A().member) # E: Revealed type is '__main__.A*'
368-
reveal_type(B().member) # E: Revealed type is '__main__.B*'
408+
reveal_type(A().g) # E: Revealed type is 'builtins.int'
409+
reveal_type(A().gt) # E: Revealed type is 'Tuple[builtins.int, builtins.int, fallback=__main__.A]'
410+
reveal_type(A().f()) # E: Revealed type is 'builtins.int'
411+
reveal_type(A().ft()) # E: Revealed type is 'Tuple[builtins.int, builtins.int, fallback=__main__.A]'
412+
reveal_type(B().g) # E: Revealed type is 'builtins.int'
413+
reveal_type(B().gt) # E: Revealed type is 'Tuple[builtins.int, builtins.int, fallback=__main__.B]'
414+
reveal_type(B().f()) # E: Revealed type is 'builtins.int'
415+
reveal_type(B().ft()) # E: Revealed type is 'Tuple[builtins.int, builtins.int, fallback=__main__.B]'
416+
417+
[builtins fixtures/property.pyi]
418+
419+
[case testSelfTypeProperSupertypeAttributeMeta]
420+
from typing import Callable, TypeVar, Type
421+
T = TypeVar('T')
422+
class A(type):
423+
@property
424+
def g(cls: object) -> int: return 0
425+
@property
426+
def gt(cls: T) -> T: return cls
427+
f: Callable[[object], int]
428+
ft: Callable[[T], T]
429+
430+
class B(A):
431+
pass
432+
433+
class X(metaclass=B):
434+
def __init__(self, x: int) -> None: pass
435+
class Y(X): pass
436+
X1: Type[X]
437+
reveal_type(X.g) # E: Revealed type is 'builtins.int'
438+
reveal_type(X.gt) # E: Revealed type is 'def (x: builtins.int) -> __main__.X'
439+
reveal_type(X.f()) # E: Revealed type is 'builtins.int'
440+
reveal_type(X.ft()) # E: Revealed type is 'def (x: builtins.int) -> __main__.X'
441+
reveal_type(Y.g) # E: Revealed type is 'builtins.int'
442+
reveal_type(Y.gt) # E: Revealed type is 'def (x: builtins.int) -> __main__.Y'
443+
reveal_type(Y.f()) # E: Revealed type is 'builtins.int'
444+
reveal_type(Y.ft()) # E: Revealed type is 'def (x: builtins.int) -> __main__.Y'
445+
reveal_type(X1.g) # E: Revealed type is 'builtins.int'
446+
reveal_type(X1.gt) # E: Revealed type is 'Type[__main__.X]'
447+
reveal_type(X1.f()) # E: Revealed type is 'builtins.int'
448+
reveal_type(X1.ft()) # E: Revealed type is 'Type[__main__.X]'
449+
450+
[builtins fixtures/property.pyi]
451+
452+
[case testSelfTypeProperSupertypeAttributeGeneric]
453+
from typing import Callable, TypeVar, Generic
454+
Q = TypeVar('Q', covariant=True)
455+
class K(Generic[Q]):
456+
q: Q
457+
T = TypeVar('T')
458+
class A(K[Q]):
459+
@property
460+
def g(self: K[object]) -> int: return 0
461+
@property
462+
def gt(self: K[T]) -> T: return self.q
463+
f: Callable[[object], int]
464+
ft: Callable[[T], T]
465+
466+
class B(A[Q]):
467+
pass
468+
a: A[int]
469+
b: B[str]
470+
reveal_type(a.g) # E: Revealed type is 'builtins.int'
471+
--reveal_type(a.gt) # E: Revealed type is 'builtins.int'
472+
reveal_type(a.f()) # E: Revealed type is 'builtins.int'
473+
reveal_type(a.ft()) # E: Revealed type is '__main__.A*[builtins.int]'
474+
reveal_type(b.g) # E: Revealed type is 'builtins.int'
475+
--reveal_type(b.gt) # E: Revealed type is '__main__.B*[builtins.str]'
476+
reveal_type(b.f()) # E: Revealed type is 'builtins.int'
477+
reveal_type(b.ft()) # E: Revealed type is '__main__.B*[builtins.str]'
369478

370479
[builtins fixtures/property.pyi]
371480

@@ -376,3 +485,21 @@ class A:
376485
# def g(self: None) -> None: ... see in check-python2.test
377486
[out]
378487
main:3: error: Self argument missing for a non-static method (or an invalid type for self)
488+
489+
[case testUnionPropertyField]
490+
from typing import Union
491+
492+
class A:
493+
x: int
494+
495+
class B:
496+
@property
497+
def x(self) -> int: return 1
498+
499+
class C:
500+
@property
501+
def x(self) -> int: return 1
502+
503+
ab: Union[A, B, C]
504+
reveal_type(ab.x) # E: Revealed type is 'builtins.int'
505+
[builtins fixtures/property.pyi]

0 commit comments

Comments
 (0)