Skip to content

Fix checking of self when accessing a non-method callable attribute (take 2) #4016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 12, 2017
62 changes: 33 additions & 29 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
from mypy.plugin import Plugin, AttributeContext
from mypy import messages
from mypy import subtypes
from mypy import meet

MYPY = False
if MYPY: # import for forward declaration only
import mypy.checker
Expand Down Expand Up @@ -286,6 +288,7 @@ def analyze_var(name: str, var: Var, itype: Instance, info: TypeInfo, node: Cont

This is conceptually part of analyze_member_access and the arguments are similar.

itype is the class object in which var is dedined
original_type is the type of E in the expression E.var
"""
# Found a member variable.
Expand All @@ -310,15 +313,21 @@ def analyze_var(name: str, var: Var, itype: Instance, info: TypeInfo, node: Cont
msg.cant_assign_to_method(node)

if not var.is_staticmethod:
# Class-level function objects and classmethods become bound
# methods: the former to the instance, the latter to the
# class.
# Class-level function objects and classmethods become bound methods:
# the former to the instance, the latter to the class.
functype = t
check_method_type(functype, itype, var.is_classmethod, node, msg)
# Use meet to narrow original_type to the dispatched type.
# For example, assume
# * A.f: Callable[[A1], None] where A1 <: A (maybe A1 == A)
# * B.f: Callable[[B1], None] where B1 <: B (maybe B1 == B)
# * x: Union[A1, B1]
# In `x.f`, when checking `x` against A1 we assume x is compatible with A
# and similarly for B1 when checking agains B
dispatched_type = meet.meet_types(original_type, itype)
check_self_arg(functype, dispatched_type, var.is_classmethod, node, name, msg)
signature = bind_self(functype, original_type, var.is_classmethod)
if var.is_property:
# A property cannot have an overloaded type => the cast
# is fine.
# A property cannot have an overloaded type => the cast is fine.
assert isinstance(signature, CallableType)
result = signature.ret_type
else:
Expand Down Expand Up @@ -370,33 +379,28 @@ def lookup_member_var_or_accessor(info: TypeInfo, name: str,
return None


def check_method_type(functype: FunctionLike, itype: Instance, is_classmethod: bool,
context: Context, msg: MessageBuilder) -> None:
def check_self_arg(functype: FunctionLike, dispatched_arg_type: Type, is_classmethod: bool,
context: Context, name: str, msg: MessageBuilder) -> None:
"""For x.f where A.f: A1 -> T, check that meet(type(x), A) <: A1 for each overload.

dispatched_arg_type is meet(B, A) in the following example

def g(x: B): x.f
class A:
f: Callable[[A1], None]
"""
# TODO: this is too strict. We can return filtered overloads for matching definitions
for item in functype.items():
if not item.arg_types or item.arg_kinds[0] not in (ARG_POS, ARG_STAR):
# No positional first (self) argument (*args is okay).
msg.invalid_method_type(item, context)
elif not is_classmethod:
# Check that self argument has type 'Any' or valid instance type.
selfarg = item.arg_types[0]
# If this is a method of a tuple class, correct for the fact that
# we passed to typ.fallback in analyze_member_access. See #1432.
if isinstance(selfarg, TupleType):
selfarg = selfarg.fallback
if not subtypes.is_subtype(selfarg, itype):
msg.invalid_method_type(item, context)
msg.no_formal_self(name, item, context)
else:
# Check that cls argument has type 'Any' or valid class type.
# (This is sufficient for the current treatment of @classmethod,
# but probably needs to be revisited when we implement Type[C]
# or advanced variants of it like Type[<args>, C].)
clsarg = item.arg_types[0]
if isinstance(clsarg, CallableType) and clsarg.is_type_obj():
if not subtypes.is_equivalent(clsarg.ret_type, itype):
msg.invalid_class_method_type(item, context)
else:
if not subtypes.is_equivalent(clsarg, AnyType(TypeOfAny.special_form)):
msg.invalid_class_method_type(item, context)
selfarg = item.arg_types[0]
if is_classmethod:
dispatched_arg_type = TypeType.make_normalized(dispatched_arg_type)
if not subtypes.is_subtype(dispatched_arg_type, erase_to_bound(selfarg)):
msg.incompatible_self_argument(name, dispatched_arg_type, item,
is_classmethod, context)


def analyze_class_attribute_access(itype: Instance,
Expand Down
14 changes: 9 additions & 5 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,11 +863,15 @@ def cannot_determine_type(self, name: str, context: Context) -> None:
def cannot_determine_type_in_base(self, name: str, base: str, context: Context) -> None:
self.fail("Cannot determine type of '%s' in base class '%s'" % (name, base), context)

def invalid_method_type(self, sig: CallableType, context: Context) -> None:
self.fail('Invalid method type', context)

def invalid_class_method_type(self, sig: CallableType, context: Context) -> None:
self.fail('Invalid class method type', context)
def no_formal_self(self, name: str, item: CallableType, context: Context) -> None:
self.fail('Attribute function "%s" with type %s does not accept self argument'
% (name, self.format(item)), context)

def incompatible_self_argument(self, name: str, arg: Type, sig: CallableType,
is_classmethod: bool, context: Context) -> None:
kind = 'class attribute function' if is_classmethod else 'attribute function'
self.fail('Invalid self argument %s to %s "%s" with type %s'
% (self.format(arg), kind, name, self.format(sig)), context)

def incompatible_conditional_function_def(self, defn: FuncDef) -> None:
self.fail('All conditional function variants must have identical '
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -2105,7 +2105,7 @@ class B:
a = A
bad = lambda: 42

B().bad() # E: Invalid method type
B().bad() # E: Attribute function "bad" with type "Callable[[], int]" does not accept self argument
reveal_type(B.a) # E: Revealed type is 'def () -> __main__.A'
reveal_type(B().a) # E: Revealed type is 'def () -> __main__.A'
reveal_type(B().a()) # E: Revealed type is '__main__.A'
Expand Down
6 changes: 3 additions & 3 deletions test-data/unit/check-functions.test
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,8 @@ class A:
f = x # type: Callable[[], None]
g = x # type: Callable[[B], None]
a = None # type: A
a.f() # E: Invalid method type
a.g() # E: Invalid method type
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
Expand Down Expand Up @@ -568,7 +568,7 @@ class A(Generic[t]):
ab = None # type: A[B]
ac = None # type: A[C]
ab.f()
ac.f() # E: Invalid method type
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
Expand Down
143 changes: 135 additions & 8 deletions test-data/unit/check-selftype.test
Original file line number Diff line number Diff line change
Expand Up @@ -351,21 +351,130 @@ class E:
def __init_subclass__(cls) -> None:
reveal_type(cls) # E: Revealed type is 'def () -> __main__.E'

[case testSelfTypeProperty]
from typing import TypeVar
[case testSelfTypePropertyUnion]
from typing import Union
class A:
@property
def f(self: A) -> int: pass

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

class A:
[builtins fixtures/property.pyi]

[case testSelfTypeProperSupertypeAttribute]
from typing import Callable, TypeVar
class K: pass
T = TypeVar('T', bound=K)
class A(K):
@property
def member(self: T) -> T:
pass
def g(self: K) -> int: return 0
@property
def gt(self: T) -> T: return self
f: Callable[[object], int]
ft: Callable[[T], T]

class B(A):
pass

reveal_type(A().g) # E: Revealed type is 'builtins.int'
reveal_type(A().gt) # E: Revealed type is '__main__.A*'
reveal_type(A().f()) # E: Revealed type is 'builtins.int'
reveal_type(A().ft()) # E: Revealed type is '__main__.A*'
reveal_type(B().g) # E: Revealed type is 'builtins.int'
reveal_type(B().gt) # E: Revealed type is '__main__.B*'
reveal_type(B().f()) # E: Revealed type is 'builtins.int'
reveal_type(B().ft()) # E: Revealed type is '__main__.B*'

[builtins fixtures/property.pyi]

[case testSelfTypeProperSupertypeAttributeTuple]
from typing import Callable, TypeVar, Tuple
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]

class B(A):
pass

reveal_type(A().member) # E: Revealed type is '__main__.A*'
reveal_type(B().member) # E: Revealed type is '__main__.B*'
reveal_type(A().g) # E: Revealed type is 'builtins.int'
reveal_type(A().gt) # E: Revealed type is 'Tuple[builtins.int, builtins.int, fallback=__main__.A]'
reveal_type(A().f()) # E: Revealed type is 'builtins.int'
reveal_type(A().ft()) # E: Revealed type is 'Tuple[builtins.int, builtins.int, fallback=__main__.A]'
reveal_type(B().g) # E: Revealed type is 'builtins.int'
reveal_type(B().gt) # E: Revealed type is 'Tuple[builtins.int, builtins.int, fallback=__main__.B]'
reveal_type(B().f()) # E: Revealed type is 'builtins.int'
reveal_type(B().ft()) # E: Revealed type is 'Tuple[builtins.int, builtins.int, fallback=__main__.B]'

[builtins fixtures/property.pyi]

[case testSelfTypeProperSupertypeAttributeMeta]
from typing import Callable, TypeVar, Type
T = TypeVar('T')
class A(type):
@property
def g(cls: object) -> int: return 0
@property
def gt(cls: T) -> T: return cls
f: Callable[[object], int]
ft: Callable[[T], T]

class B(A):
pass

class X(metaclass=B):
def __init__(self, x: int) -> None: pass
class Y(X): pass
X1: Type[X]
reveal_type(X.g) # E: Revealed type is 'builtins.int'
reveal_type(X.gt) # E: Revealed type is 'def (x: builtins.int) -> __main__.X'
reveal_type(X.f()) # E: Revealed type is 'builtins.int'
reveal_type(X.ft()) # E: Revealed type is 'def (x: builtins.int) -> __main__.X'
reveal_type(Y.g) # E: Revealed type is 'builtins.int'
reveal_type(Y.gt) # E: Revealed type is 'def (x: builtins.int) -> __main__.Y'
reveal_type(Y.f()) # E: Revealed type is 'builtins.int'
reveal_type(Y.ft()) # E: Revealed type is 'def (x: builtins.int) -> __main__.Y'
reveal_type(X1.g) # E: Revealed type is 'builtins.int'
reveal_type(X1.gt) # E: Revealed type is 'Type[__main__.X]'
reveal_type(X1.f()) # E: Revealed type is 'builtins.int'
reveal_type(X1.ft()) # E: Revealed type is 'Type[__main__.X]'

[builtins fixtures/property.pyi]

[case testSelfTypeProperSupertypeAttributeGeneric]
from typing import Callable, TypeVar, Generic
Q = TypeVar('Q', covariant=True)
class K(Generic[Q]):
q: Q
T = TypeVar('T')
class A(K[Q]):
@property
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]

class B(A[Q]):
pass
a: A[int]
b: B[str]
reveal_type(a.g) # E: Revealed type is 'builtins.int'
--reveal_type(a.gt) # E: Revealed type is 'builtins.int'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these commented out?

Copy link
Contributor Author

@elazarg elazarg Oct 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The instantiation of selftype fails in the commented tests. I think it is not a problem in this PR, and fixing these requires changes in bind_self.

I just forced an update that removed two of the comments; I hope it doesn't cause a mess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - I didn't force it; I will add as a new commit in a moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To elaborate, bind_self does not unify K[T] and A[T] even if it can. It just does not check for this case, and simply trims the first argument. It only performs such unification is for TypeType.

reveal_type(a.f()) # E: Revealed type is 'builtins.int'
reveal_type(a.ft()) # E: Revealed type is '__main__.A*[builtins.int]'
reveal_type(b.g) # E: Revealed type is 'builtins.int'
--reveal_type(b.gt) # E: Revealed type is '__main__.B*[builtins.str]'
reveal_type(b.f()) # E: Revealed type is 'builtins.int'
reveal_type(b.ft()) # E: Revealed type is '__main__.B*[builtins.str]'

[builtins fixtures/property.pyi]

Expand All @@ -376,3 +485,21 @@ class A:
# def g(self: None) -> None: ... see in check-python2.test
[out]
main:3: error: Self argument missing for a non-static method (or an invalid type for self)

[case testUnionPropertyField]
from typing import Union

class A:
x: int

class B:
@property
def x(self) -> int: return 1

class C:
@property
def x(self) -> int: return 1

ab: Union[A, B, C]
reveal_type(ab.x) # E: Revealed type is 'builtins.int'
[builtins fixtures/property.pyi]