Skip to content

Fix class method type variables #7862

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.

Sign up for GitHub

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 9 commits into from
Nov 12, 2019
Merged

Fix class method type variables #7862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
47cb240
Start fixing
ilevkivskyi Nov 3, 2019
534976e
Some fixes
ilevkivskyi Nov 3, 2019
0a79576
Tweak also direct cls call
ilevkivskyi Nov 3, 2019
2866429
Update tests
ilevkivskyi Nov 3, 2019
b7efd22
Fix lint
ilevkivskyi Nov 3, 2019
f938d1a
Merge remote-tracking branch 'upstream/master' into fix-class-method-…
ilevkivskyi Nov 11, 2019
a59cecf
Fix self-check
ilevkivskyi Nov 11, 2019
5921ef9
Address CR
Nov 12, 2019
3bbf7de
Put back an empty line
Nov 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion mypy/checkexpr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,12 @@ def analyze_type_type_callee(self, item: ProperType, context: Context) -> Type:
res = type_object_type(item.type, self.named_type)
if isinstance(res, CallableType):
res = res.copy_modified(from_type_type=True)
return expand_type_by_instance(res, item)
expanded = get_proper_type(expand_type_by_instance(res, item))
if isinstance(expanded, CallableType):
# Callee of the form Type[...] should never be generic, only
# proper class objects can be.
expanded = expanded.copy_modified(variables=[])
return expanded
if isinstance(item, UnionType):
return UnionType([self.analyze_type_type_callee(get_proper_type(tp), context)
for tp in item.relevant_items()], item.line)
Expand Down
41 changes: 22 additions & 19 deletions mypy/checkmember.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Type checking of attribute access"""

from typing import cast, Callable, Optional, Union
from typing import cast, Callable, Optional, Union, List
from typing_extensions import TYPE_CHECKING

from mypy.types import (
Expand Down Expand Up @@ -240,7 +240,9 @@ def analyze_type_callable_member_access(name: str,
# This check makes sure that when we encounter an operator, we skip looking up
# the corresponding method in the current instance to avoid this edge case.
# See https://github.com/python/mypy/pull/1787 for more info.
result = analyze_class_attribute_access(ret_type, name, mx)
# TODO: do not rely on same type variables being present in all constructor overloads.
result = analyze_class_attribute_access(ret_type, name, mx,
original_vars=typ.items()[0].variables)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if picking the first item may cause problems with overloaded methods. If so, it would be good to add a comment and maybe create a follow-up issue (unless the fix is trivial).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I use thus because I noticed overloaded constructors have "uniform" type variables, but maybe it is not guaranteed, I will add a test case and a TODO here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Funny enough, when adding a test for this (that seem to work well as I said) I discovered a bug because dozen lines above we use ret_type = typ.items()[0].ret_type and now this is clearly wrong.

if result:
return result
# Look up from the 'type' type.
Expand Down Expand Up @@ -649,8 +651,15 @@ def f(self: S) -> T: ...
def analyze_class_attribute_access(itype: Instance,
name: str,
mx: MemberContext,
override_info: Optional[TypeInfo] = None) -> Optional[Type]:
"""original_type is the type of E in the expression E.var"""
override_info: Optional[TypeInfo] = None,
original_vars: Optional[List[TypeVarDef]] = None
) -> Optional[Type]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Document the new argument.

"""Analyze access to an attribute on a class object.

itype is the return type of the class object callable, original_type is the type
of E in the expression E.var, original_vars are type variables of the class callable
(for generic classes).
"""
info = itype.type
if override_info:
info = override_info
Expand Down Expand Up @@ -718,7 +727,7 @@ def analyze_class_attribute_access(itype: Instance,
# C[int].x # Also an error, since C[int] is same as C at runtime
if isinstance(t, TypeVarType) or has_type_vars(t):
# Exception: access on Type[...], including first argument of class methods is OK.
if not isinstance(get_proper_type(mx.original_type), TypeType):
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:
Expand All @@ -737,7 +746,7 @@ def analyze_class_attribute_access(itype: Instance,
if isinstance(t, FunctionLike) and is_classmethod:
t = check_self_arg(t, mx.self_type, False, mx.context, name, mx.msg)
result = add_class_tvars(t, itype, isuper, is_classmethod,
mx.builtin_type, mx.self_type)
mx.builtin_type, mx.self_type, original_vars=original_vars)
if not mx.is_lvalue:
result = analyze_descriptor_access(mx.original_type, result, mx.builtin_type,
mx.msg, mx.context, chk=mx.chk)
Expand Down Expand Up @@ -783,7 +792,8 @@ def analyze_class_attribute_access(itype: Instance,
def add_class_tvars(t: ProperType, itype: Instance, isuper: Optional[Instance],
is_classmethod: bool,
builtin_type: Callable[[str], Instance],
original_type: Type) -> Type:
original_type: Type,
original_vars: Optional[List[TypeVarDef]] = None) -> Type:
"""Instantiate type variables during analyze_class_attribute_access,
e.g T and Q in the following:

Expand All @@ -796,10 +806,10 @@ class B(A[str]): pass
B.foo()

original_type is the value of the type B in the expression B.foo() or the corresponding
component in case if a union (this is used to bind the self-types).
component in case if a union (this is used to bind the self-types); original_vars are type
variables of the class callable on which the method was accessed.
"""
# TODO: verify consistency between Q and T
info = itype.type # type: TypeInfo
if is_classmethod:
assert isuper is not None
t = get_proper_type(expand_type_by_instance(t, isuper))
Expand All @@ -815,22 +825,15 @@ class B(A[str]): pass
# This behaviour is useful for defining alternative constructors for generic classes.
# To achieve such behaviour, we add the class type variables that are still free
# (i.e. appear in the return type of the class object on which the method was accessed).
free_ids = {t.id for t in itype.args if isinstance(t, TypeVarType)}

if isinstance(t, CallableType):
# NOTE: in practice either all or none of the variables are free, since
# visit_type_application() will detect any type argument count mismatch and apply
# a correct number of Anys.
tvars = [TypeVarDef(n, n, i + 1, [], builtin_type('builtins.object'), tv.variance)
for (i, n), tv in zip(enumerate(info.type_vars), info.defn.type_vars)
# use 'is' to avoid id clashes with unrelated variables
if any(tv.id is id for id in free_ids)]
tvars = original_vars if original_vars is not None else []
if is_classmethod:
t = bind_self(t, original_type, is_classmethod=True)
return t.copy_modified(variables=tvars + t.variables)
elif isinstance(t, Overloaded):
return Overloaded([cast(CallableType, add_class_tvars(item, itype, isuper, is_classmethod,
builtin_type, original_type))
builtin_type, original_type,
original_vars=original_vars))
for item in t.items()])
return t

Expand Down
30 changes: 14 additions & 16 deletions mypy/plugins/ctypes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import mypy.plugin
from mypy import nodes
from mypy.maptype import map_instance_to_supertype
from mypy.messages import format_type
from mypy.subtypes import is_subtype
from mypy.types import (
AnyType, CallableType, Instance, NoneType, Type, TypeOfAny, UnionType,
Expand Down Expand Up @@ -125,18 +126,17 @@ def array_constructor_callback(ctx: 'mypy.plugin.FunctionContext') -> Type:
for arg_num, (arg_kind, arg_type) in enumerate(zip(ctx.arg_kinds[0], ctx.arg_types[0]), 1):
if arg_kind == nodes.ARG_POS and not is_subtype(arg_type, allowed):
ctx.api.msg.fail(
'Array constructor argument {} of type "{}"'
' is not convertible to the array element type "{}"'
.format(arg_num, arg_type, et),
ctx.context)
'Array constructor argument {} of type {}'
' is not convertible to the array element type {}'
.format(arg_num, format_type(arg_type), format_type(et)), ctx.context)
elif arg_kind == nodes.ARG_STAR:
ty = ctx.api.named_generic_type("typing.Iterable", [allowed])
if not is_subtype(arg_type, ty):
it = ctx.api.named_generic_type("typing.Iterable", [et])
ctx.api.msg.fail(
'Array constructor argument {} of type "{}"'
' is not convertible to the array element type "Iterable[{}]"'
.format(arg_num, arg_type, et),
ctx.context)
'Array constructor argument {} of type {}'
' is not convertible to the array element type {}'
.format(arg_num, format_type(arg_type), format_type(it)), ctx.context)

return ctx.default_return_type

Expand Down Expand Up @@ -204,10 +204,9 @@ def array_value_callback(ctx: 'mypy.plugin.AttributeContext') -> Type:
types.append(_get_text_type(ctx.api))
else:
ctx.api.msg.fail(
'ctypes.Array attribute "value" is only available'
' with element type c_char or c_wchar, not "{}"'
.format(et),
ctx.context)
'Array attribute "value" is only available'
' with element type "c_char" or "c_wchar", not {}'
.format(format_type(et)), ctx.context)
return make_simplified_union(types)
return ctx.default_attr_type

Expand All @@ -223,9 +222,8 @@ def array_raw_callback(ctx: 'mypy.plugin.AttributeContext') -> Type:
types.append(_get_bytes_type(ctx.api))
else:
ctx.api.msg.fail(
'ctypes.Array attribute "raw" is only available'
' with element type c_char, not "{}"'
.format(et),
ctx.context)
'Array attribute "raw" is only available'
' with element type "c_char", not {}'
.format(format_type(et)), ctx.context)
return make_simplified_union(types)
return ctx.default_attr_type
5 changes: 5 additions & 0 deletions mypy/plugins/dataclasses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,11 @@ def collect_attributes(self) -> Optional[List[DataclassAttribute]]:
elif not isinstance(stmt.rvalue, TempNode):
has_default = True

if not has_default:
# Make all non-default attributes implicit because they are de-facto set
# on self in the generated __init__(), not in the class body.
sym.implicit = True

known_attrs.add(lhs.name)
attrs.append(DataclassAttribute(
name=lhs.name,
Expand Down
38 changes: 19 additions & 19 deletions test-data/unit/check-ctypes.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ class MyCInt(ctypes.c_int):

intarr4 = ctypes.c_int * 4
a = intarr4(1, ctypes.c_int(2), MyCInt(3), 4)
intarr4(1, 2, 3, "invalid") # E: Array constructor argument 4 of type "builtins.str" is not convertible to the array element type "ctypes.c_int"
reveal_type(a) # N: Revealed type is 'ctypes.Array[ctypes.c_int]'
intarr4(1, 2, 3, "invalid") # E: Array constructor argument 4 of type "str" is not convertible to the array element type "c_int"
reveal_type(a) # N: Revealed type is 'ctypes.Array[ctypes.c_int*]'
reveal_type(a[0]) # N: Revealed type is 'builtins.int'
reveal_type(a[1:3]) # N: Revealed type is 'builtins.list[builtins.int]'
a[0] = 42
Expand All @@ -30,11 +30,11 @@ class MyCInt(ctypes.c_int):

myintarr4 = MyCInt * 4
mya = myintarr4(1, 2, MyCInt(3), 4)
myintarr4(1, ctypes.c_int(2), MyCInt(3), "invalid") # E: Array constructor argument 2 of type "ctypes.c_int" is not convertible to the array element type "__main__.MyCInt" \
# E: Array constructor argument 4 of type "builtins.str" is not convertible to the array element type "__main__.MyCInt"
reveal_type(mya) # N: Revealed type is 'ctypes.Array[__main__.MyCInt]'
reveal_type(mya[0]) # N: Revealed type is '__main__.MyCInt'
reveal_type(mya[1:3]) # N: Revealed type is 'builtins.list[__main__.MyCInt]'
myintarr4(1, ctypes.c_int(2), MyCInt(3), "invalid") # E: Array constructor argument 2 of type "c_int" is not convertible to the array element type "MyCInt" \
# E: Array constructor argument 4 of type "str" is not convertible to the array element type "MyCInt"
reveal_type(mya) # N: Revealed type is 'ctypes.Array[__main__.MyCInt*]'
reveal_type(mya[0]) # N: Revealed type is '__main__.MyCInt*'
reveal_type(mya[1:3]) # N: Revealed type is 'builtins.list[__main__.MyCInt*]'
mya[0] = 42
mya[1] = ctypes.c_int(42) # E: No overload variant of "__setitem__" of "Array" matches argument types "int", "c_int" \
# N: Possible overload variants: \
Expand Down Expand Up @@ -106,7 +106,7 @@ import ctypes

wca = (ctypes.c_wchar * 4)('a', 'b', 'c', '\x00')
reveal_type(wca.value) # N: Revealed type is 'builtins.str'
wca.raw # E: ctypes.Array attribute "raw" is only available with element type c_char, not "ctypes.c_wchar"
wca.raw # E: Array attribute "raw" is only available with element type "c_char", not "c_wchar"
[builtins fixtures/floatdict.pyi]

[case testCtypesWcharArrayAttrsPy2]
Expand All @@ -115,7 +115,7 @@ import ctypes

wca = (ctypes.c_wchar * 4)(u'a', u'b', u'c', u'\x00')
reveal_type(wca.value) # N: Revealed type is 'builtins.unicode'
wca.raw # E: ctypes.Array attribute "raw" is only available with element type c_char, not "ctypes.c_wchar"
wca.raw # E: Array attribute "raw" is only available with element type "c_char", not "c_wchar"
[builtins_py2 fixtures/floatdict_python2.pyi]

[case testCtypesCharUnionArrayAttrs]
Expand All @@ -124,7 +124,7 @@ from typing import Union

cua: ctypes.Array[Union[ctypes.c_char, ctypes.c_wchar]]
reveal_type(cua.value) # N: Revealed type is 'Union[builtins.bytes, builtins.str]'
cua.raw # E: ctypes.Array attribute "raw" is only available with element type c_char, not "Union[ctypes.c_char, ctypes.c_wchar]"
cua.raw # E: Array attribute "raw" is only available with element type "c_char", not "Union[c_char, c_wchar]"
[builtins fixtures/floatdict.pyi]

[case testCtypesAnyUnionArrayAttrs]
Expand All @@ -141,8 +141,8 @@ import ctypes
from typing import Union

cua: ctypes.Array[Union[ctypes.c_char, ctypes.c_int]]
cua.value # E: ctypes.Array attribute "value" is only available with element type c_char or c_wchar, not "Union[ctypes.c_char, ctypes.c_int]"
cua.raw # E: ctypes.Array attribute "raw" is only available with element type c_char, not "Union[ctypes.c_char, ctypes.c_int]"
cua.value # E: Array attribute "value" is only available with element type "c_char" or "c_wchar", not "Union[c_char, c_int]"
cua.raw # E: Array attribute "raw" is only available with element type "c_char", not "Union[c_char, c_int]"
[builtins fixtures/floatdict.pyi]

[case testCtypesAnyArrayAttrs]
Expand All @@ -157,8 +157,8 @@ reveal_type(aa.raw) # N: Revealed type is 'builtins.bytes'
import ctypes

oa = (ctypes.c_int * 4)(1, 2, 3, 4)
oa.value # E: ctypes.Array attribute "value" is only available with element type c_char or c_wchar, not "ctypes.c_int"
oa.raw # E: ctypes.Array attribute "raw" is only available with element type c_char, not "ctypes.c_int"
oa.value # E: Array attribute "value" is only available with element type "c_char" or "c_wchar", not "c_int"
oa.raw # E: Array attribute "raw" is only available with element type "c_char", not "c_int"
[builtins fixtures/floatdict.pyi]

[case testCtypesArrayConstructorStarargs]
Expand All @@ -168,13 +168,13 @@ intarr4 = ctypes.c_int * 4
intarr6 = ctypes.c_int * 6
int_values = [1, 2, 3, 4]
c_int_values = [ctypes.c_int(1), ctypes.c_int(2), ctypes.c_int(3), ctypes.c_int(4)]
reveal_type(intarr4(*int_values)) # N: Revealed type is 'ctypes.Array[ctypes.c_int]'
reveal_type(intarr4(*c_int_values)) # N: Revealed type is 'ctypes.Array[ctypes.c_int]'
reveal_type(intarr6(1, ctypes.c_int(2), *int_values)) # N: Revealed type is 'ctypes.Array[ctypes.c_int]'
reveal_type(intarr6(1, ctypes.c_int(2), *c_int_values)) # N: Revealed type is 'ctypes.Array[ctypes.c_int]'
reveal_type(intarr4(*int_values)) # N: Revealed type is 'ctypes.Array[ctypes.c_int*]'
reveal_type(intarr4(*c_int_values)) # N: Revealed type is 'ctypes.Array[ctypes.c_int*]'
reveal_type(intarr6(1, ctypes.c_int(2), *int_values)) # N: Revealed type is 'ctypes.Array[ctypes.c_int*]'
reveal_type(intarr6(1, ctypes.c_int(2), *c_int_values)) # N: Revealed type is 'ctypes.Array[ctypes.c_int*]'

float_values = [1.0, 2.0, 3.0, 4.0]
intarr4(*float_values) # E: Array constructor argument 1 of type "builtins.list[builtins.float*]" is not convertible to the array element type "Iterable[ctypes.c_int]"
intarr4(*float_values) # E: Array constructor argument 1 of type "List[float]" is not convertible to the array element type "Iterable[c_int]"
[builtins fixtures/floatdict.pyi]

[case testCtypesArrayConstructorKwargs]
Expand Down
7 changes: 5 additions & 2 deletions test-data/unit/check-dataclasses.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,9 +484,12 @@ class A(Generic[T]):
@classmethod
def foo(cls) -> None:
reveal_type(cls) # N: Revealed type is 'Type[__main__.A[T`1]]'
reveal_type(cls(1)) # N: Revealed type is '__main__.A[builtins.int*]'
reveal_type(cls('wooooo')) # N: Revealed type is '__main__.A[builtins.str*]'
cls.x # E: Access to generic instance variables via class is ambiguous

@classmethod
def other(cls, x: T) -> A[T]: ...

reveal_type(A(0).other) # N: Revealed type is 'def (x: builtins.int*) -> __main__.A[builtins.int*]'
[builtins fixtures/classmethod.pyi]

[case testDataclassesForwardRefs]
Expand Down
Loading