Skip to content
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

[clean strict optional] Use dummy TypeInfo instead of None in Instance.type (and few more fixes) #3285

Merged
merged 7 commits into from Apr 30, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion mypy/checkmember.py
Expand Up @@ -451,7 +451,7 @@ class B(A): pass
info = itype.type # type: TypeInfo
if isinstance(t, CallableType):
# TODO: Should we propagate type variable values?
tvars = [TypeVarDef(n, i + 1, None, builtin_type('builtins.object'), tv.variance)
tvars = [TypeVarDef(n, i + 1, [], builtin_type('builtins.object'), tv.variance)
for (i, n), tv in zip(enumerate(info.type_vars), info.defn.type_vars)]
if is_classmethod:
t = bind_self(t, original_type)
Expand Down
6 changes: 3 additions & 3 deletions mypy/fixup.py
Expand Up @@ -11,7 +11,7 @@
from mypy.types import (
CallableType, EllipsisType, Instance, Overloaded, TupleType, TypedDictType,
TypeList, TypeVarType, UnboundType, UnionType, TypeVisitor,
TypeType
TypeType, NOT_READY
)
from mypy.visitor import NodeVisitor

Expand Down Expand Up @@ -156,7 +156,7 @@ def visit_instance(self, inst: Instance) -> None:
# TODO: Is this needed or redundant?
# Also fix up the bases, just in case.
for base in inst.type.bases:
if base.type is None:
if base.type is NOT_READY:
base.accept(self)
for a in inst.args:
a.accept(self)
Expand Down Expand Up @@ -233,7 +233,7 @@ def visit_type_type(self, t: TypeType) -> None:


def lookup_qualified(modules: Dict[str, MypyFile], name: str,
quick_and_dirty: bool) -> SymbolNode:
quick_and_dirty: bool) -> Optional[SymbolNode]:
stnode = lookup_qualified_stnode(modules, name, quick_and_dirty)
if stnode is None:
return None
Expand Down
10 changes: 2 additions & 8 deletions mypy/maptype.py
Expand Up @@ -28,9 +28,7 @@ def map_instance_to_supertypes(instance: Instance,
# FIX: Currently we should only have one supertype per interface, so no
# need to return an array
result = [] # type: List[Instance]
typ = instance.type
assert typ is not None, 'Instance.type is not fixed after deserialization'
for path in class_derivation_paths(typ, supertype):
for path in class_derivation_paths(instance.type, supertype):
types = [instance]
for sup in path:
a = [] # type: List[Instance]
Expand Down Expand Up @@ -60,7 +58,6 @@ def class_derivation_paths(typ: TypeInfo,

for base in typ.bases:
btype = base.type
assert btype is not None, 'Instance.type is not fixed after deserialization'
if btype == supertype:
result.append([btype])
else:
Expand All @@ -75,7 +72,6 @@ def map_instance_to_direct_supertypes(instance: Instance,
supertype: TypeInfo) -> List[Instance]:
# FIX: There should only be one supertypes, always.
typ = instance.type
assert typ is not None, 'Instance.type is not fixed after deserialization'
result = [] # type: List[Instance]

for b in typ.bases:
Expand Down Expand Up @@ -103,6 +99,4 @@ def instance_to_type_environment(instance: Instance) -> Dict[TypeVarId, Type]:
arguments. The type variables are mapped by their `id`.

"""
typ = instance.type
assert typ is not None, 'Instance.type is not fixed after deserialization'
return {binder.id: arg for binder, arg in zip(typ.defn.type_vars, instance.args)}
return {binder.id: arg for binder, arg in zip(instance.type.defn.type_vars, instance.args)}
5 changes: 5 additions & 0 deletions mypy/nodes.py
Expand Up @@ -2208,6 +2208,11 @@ def deserialize(cls, data: JsonDict) -> 'TypeInfo':
return ti


class FakeInfo(TypeInfo):
def __getattr__(self, attr: str) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ilevkivskyi @gvanrossum This should probably be __getattribute__ instead. Now attributes defined in the body of TypeInfo will not trigger an AssertionError, as far as I know, and thus FakeInfo can mask some actual errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

In any case __getattribute__ looks more robust. Will make a PR soon.

Copy link
Member

Choose a reason for hiding this comment

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

OK, but then I suspect that #3281 will come back. So we should at least have a test in place to verify it doesn't. (Because that's the kind of error that's being masked here -- and since in that particular case it was just getting the wrong full name of some type for some error message I'd rather have a poor error than a crash -- our users really don't like crashes.

Copy link
Member Author

Choose a reason for hiding this comment

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

@gvanrossum
Yes, #3281 comes back, I tried your crash scenario and it indeed crashes (although in a different place). But it looks like Jukka's idea works! At least I have found two actual bugs in fixup.py, both related to metaclass de-serialization. After fixing those, your scenario from #3281 doe snot crash anymore. I will make a PR now so that you can try it.

raise RuntimeError('De-serialization failure: TypeInfo not fixed')


class SymbolTableNode:
# Kind of node. Possible values:
# - LDEF: local definition (of any kind)
Expand Down
6 changes: 3 additions & 3 deletions mypy/semanal.py
Expand Up @@ -182,7 +182,7 @@ class SemanticAnalyzer(NodeVisitor):
# Nested block depths of scopes
block_depth = None # type: List[int]
# TypeInfo of directly enclosing class (or None)
type = None # type: TypeInfo
type = None # type: Optional[TypeInfo]
# Stack of outer classes (the second tuple item contains tvars).
type_stack = None # type: List[TypeInfo]
# Type variables that are bound by the directly enclosing class
Expand Down Expand Up @@ -1745,10 +1745,10 @@ def build_newtype_typeinfo(self, name: str, old_type: Type, base_type: Instance)
info.is_newtype = True

# Add __init__ method
args = [Argument(Var('cls'), NoneTyp(), None, ARG_POS),
args = [Argument(Var('self'), NoneTyp(), None, ARG_POS),
self.make_argument('item', old_type)]
signature = CallableType(
arg_types=[cast(Type, None), old_type],
arg_types=[Instance(info, []), old_type],
arg_kinds=[arg.kind for arg in args],
arg_names=['self', 'item'],
ret_type=old_type,
Expand Down
8 changes: 5 additions & 3 deletions mypy/subtypes.py
Expand Up @@ -323,9 +323,11 @@ def is_callable_subtype(left: CallableType, right: CallableType,

if left.variables:
# Apply generic type variables away in left via type inference.
left = unify_generic_callable(left, right, ignore_return=ignore_return)
if left is None:
unified = unify_generic_callable(left, right, ignore_return=ignore_return)
if unified is None:
return False
else:
left = unified

# Check return types.
if not ignore_return and not is_compat(left.ret_type, right.ret_type):
Expand Down Expand Up @@ -493,7 +495,7 @@ def are_args_compatible(


def unify_generic_callable(type: CallableType, target: CallableType,
ignore_return: bool) -> CallableType:
ignore_return: bool) -> Optional[CallableType]:
"""Try to unify a generic callable type with another callable type.

Return unified CallableType if successful; otherwise, return None.
Expand Down
8 changes: 4 additions & 4 deletions mypy/test/testtypes.py
Expand Up @@ -75,18 +75,18 @@ def test_tuple_type(self) -> None:
assert_equal(str(TupleType([self.x, AnyType()], None)), 'Tuple[X?, Any]')

def test_type_variable_binding(self) -> None:
assert_equal(str(TypeVarDef('X', 1, None, self.fx.o)), 'X')
assert_equal(str(TypeVarDef('X', 1, [], self.fx.o)), 'X')
assert_equal(str(TypeVarDef('X', 1, [self.x, self.y], self.fx.o)),
'X in (X?, Y?)')

def test_generic_function_type(self) -> None:
c = CallableType([self.x, self.y], [ARG_POS, ARG_POS], [None, None],
self.y, self.function, name=None,
variables=[TypeVarDef('X', -1, None, self.fx.o)])
variables=[TypeVarDef('X', -1, [], self.fx.o)])
assert_equal(str(c), 'def [X] (X?, Y?) -> Y?')

v = [TypeVarDef('Y', -1, None, self.fx.o),
TypeVarDef('X', -2, None, self.fx.o)]
v = [TypeVarDef('Y', -1, [], self.fx.o),
TypeVarDef('X', -2, [], self.fx.o)]
c2 = CallableType([], [], [], NoneTyp(), self.function, name=None, variables=v)
assert_equal(str(c2), 'def [Y, X] ()')

Expand Down
2 changes: 1 addition & 1 deletion mypy/typefixture.py
Expand Up @@ -216,7 +216,7 @@ def make_type_info(self, name: str,
variance = variances[id - 1]
else:
variance = COVARIANT
v.append(TypeVarDef(n, id, None, self.o, variance=variance))
v.append(TypeVarDef(n, id, [], self.o, variance=variance))
class_def.type_vars = v

info = TypeInfo(SymbolTable(), class_def, module_name)
Expand Down
70 changes: 38 additions & 32 deletions mypy/types.py
Expand Up @@ -126,9 +126,10 @@ class TypeVarDef(mypy.nodes.Context):
line = 0
column = 0

def __init__(self, name: str, id: Union[TypeVarId, int], values: Optional[List[Type]],
def __init__(self, name: str, id: Union[TypeVarId, int], values: List[Type],
upper_bound: Type, variance: int = INVARIANT, line: int = -1,
column: int = -1) -> None:
assert values is not None, "No restrictions must be represented by empty list"
self.name = name
if isinstance(id, int):
id = TypeVarId(id)
Expand Down Expand Up @@ -164,7 +165,7 @@ def serialize(self) -> JsonDict:
return {'.class': 'TypeVarDef',
'name': self.name,
'id': self.id.raw_id,
'values': None if self.values is None else [v.serialize() for v in self.values],
'values': [v.serialize() for v in self.values],
'upper_bound': self.upper_bound.serialize(),
'variance': self.variance,
}
Expand All @@ -174,8 +175,7 @@ def deserialize(cls, data: JsonDict) -> 'TypeVarDef':
assert data['.class'] == 'TypeVarDef'
return TypeVarDef(data['name'],
data['id'],
None if data['values'] is None
else [deserialize_type(v) for v in data['values']],
[deserialize_type(v) for v in data['values']],
deserialize_type(data['upper_bound']),
data['variance'],
)
Expand Down Expand Up @@ -339,7 +339,7 @@ class DeletedType(Type):
These can be used as lvalues but not rvalues.
"""

source = '' # May be None; name that generated this value
source = '' # type: Optional[str] # May be None; name that generated this value

def __init__(self, source: str = None, line: int = -1, column: int = -1) -> None:
self.source = source
Expand All @@ -358,18 +358,24 @@ def deserialize(cls, data: JsonDict) -> 'DeletedType':
return DeletedType(data['source'])


# Fake TypeInfo to be used as a placeholder during Instance de-serialization.
NOT_READY = mypy.nodes.FakeInfo(mypy.nodes.SymbolTable(),
mypy.nodes.ClassDef('<NOT READY>', mypy.nodes.Block([])),
'<NOT READY>')


class Instance(Type):
"""An instance type of form C[T1, ..., Tn].

The list of type variables may be empty.
"""

type = None # type: Optional[mypy.nodes.TypeInfo]
type = None # type: mypy.nodes.TypeInfo
args = None # type: List[Type]
erased = False # True if result of type variable substitution
invalid = False # True if recovered after incorrect number of type arguments error

def __init__(self, typ: Optional[mypy.nodes.TypeInfo], args: List[Type],
def __init__(self, typ: mypy.nodes.TypeInfo, args: List[Type],
line: int = -1, column: int = -1, erased: bool = False) -> None:
assert(typ is None or typ.fullname() not in ["builtins.Any", "typing.Any"])
self.type = typ
Expand All @@ -396,7 +402,7 @@ def serialize(self) -> Union[JsonDict, str]:
@classmethod
def deserialize(cls, data: Union[JsonDict, str]) -> 'Instance':
if isinstance(data, str):
inst = Instance(None, [])
inst = Instance(NOT_READY, [])
inst.type_ref = data
return inst
assert data['.class'] == 'Instance'
Expand All @@ -405,7 +411,7 @@ def deserialize(cls, data: Union[JsonDict, str]) -> 'Instance':
args_list = data['args']
assert isinstance(args_list, list)
args = [deserialize_type(arg) for arg in args_list]
inst = Instance(None, args)
inst = Instance(NOT_READY, args)
inst.type_ref = data['type_ref'] # Will be fixed up by fixup.py later.
return inst

Expand Down Expand Up @@ -486,7 +492,7 @@ def items(self) -> List['CallableType']: pass
def with_name(self, name: str) -> 'FunctionLike': pass

@abstractmethod
def get_name(self) -> str: pass
def get_name(self) -> Optional[str]: pass

# Corresponding instance type (e.g. builtins.type)
fallback = None # type: Instance
Expand All @@ -507,12 +513,12 @@ class CallableType(FunctionLike):

arg_types = None # type: List[Type] # Types of function arguments
arg_kinds = None # type: List[int] # ARG_ constants
arg_names = None # type: List[str] # None if not a keyword argument
min_args = 0 # Minimum number of arguments; derived from arg_kinds
is_var_arg = False # Is it a varargs function? derived from arg_kinds
ret_type = None # type: Type # Return value type
name = '' # Name (may be None; for error messages)
definition = None # type: SymbolNode # For error messages. May be None.
arg_names = None # type: List[Optional[str]] # None if not a keyword argument
min_args = 0 # Minimum number of arguments; derived from arg_kinds
is_var_arg = False # Is it a varargs function? derived from arg_kinds
ret_type = None # type: Type # Return value type
name = '' # type: Optional[str] # Name (may be None; for error messages)
definition = None # type: Optional[SymbolNode] # For error messages. May be None.
# Type variables for a generic function
variables = None # type: List[TypeVarDef]

Expand All @@ -528,7 +534,7 @@ class CallableType(FunctionLike):
# Was this callable generated by analyzing Type[...] instantiation?
from_type_type = False # type: bool

bound_args = None # type: List[Type]
bound_args = None # type: List[Optional[Type]]

def __init__(self,
arg_types: List[Type],
Expand All @@ -546,11 +552,12 @@ def __init__(self,
is_classmethod_class: bool = False,
special_sig: Optional[str] = None,
from_type_type: bool = False,
bound_args: List[Type] = None,
bound_args: List[Optional[Type]] = None,
) -> None:
if variables is None:
variables = []
assert len(arg_types) == len(arg_kinds)
assert not any(tp is None for tp in arg_types), "No annotation must be Any, not None"
self.arg_types = arg_types
self.arg_kinds = arg_kinds
self.arg_names = arg_names
Expand All @@ -574,18 +581,18 @@ def __init__(self,
def copy_modified(self,
arg_types: List[Type] = _dummy,
arg_kinds: List[int] = _dummy,
arg_names: List[str] = _dummy,
arg_names: List[Optional[str]] = _dummy,
ret_type: Type = _dummy,
fallback: Instance = _dummy,
name: str = _dummy,
name: Optional[str] = _dummy,
definition: SymbolNode = _dummy,
variables: List[TypeVarDef] = _dummy,
line: int = _dummy,
column: int = _dummy,
is_ellipsis_args: bool = _dummy,
special_sig: Optional[str] = _dummy,
from_type_type: bool = _dummy,
bound_args: List[Type] = _dummy) -> 'CallableType':
bound_args: List[Optional[Type]] = _dummy) -> 'CallableType':
return CallableType(
arg_types=arg_types if arg_types is not _dummy else self.arg_types,
arg_kinds=arg_kinds if arg_kinds is not _dummy else self.arg_kinds,
Expand Down Expand Up @@ -630,7 +637,7 @@ def with_name(self, name: str) -> 'CallableType':
"""Return a copy of this type with the specified name."""
return self.copy_modified(ret_type=self.ret_type, name=name)

def get_name(self) -> str:
def get_name(self) -> Optional[str]:
return self.name

def max_fixed_args(self) -> int:
Expand Down Expand Up @@ -663,7 +670,7 @@ def corresponding_argument(self, model: FormalArgument) -> Optional[FormalArgume
return FormalArgument(by_name.name, by_pos.pos, by_name.typ, False)
return by_name if by_name is not None else by_pos

def argument_by_name(self, name: str) -> Optional[FormalArgument]:
def argument_by_name(self, name: Optional[str]) -> Optional[FormalArgument]:
if name is None:
return None
seen_star = False
Expand All @@ -685,7 +692,7 @@ def argument_by_name(self, name: str) -> Optional[FormalArgument]:
return FormalArgument(name, None, star2_type, False)
return None

def argument_by_position(self, position: int) -> Optional[FormalArgument]:
def argument_by_position(self, position: Optional[int]) -> Optional[FormalArgument]:
if position is None:
return None
if self.is_var_arg:
Expand Down Expand Up @@ -727,8 +734,7 @@ def serialize(self) -> JsonDict:
# TODO: As an optimization, leave out everything related to
# generic functions for non-generic functions.
return {'.class': 'CallableType',
'arg_types': [(None if t is None else t.serialize())
for t in self.arg_types],
'arg_types': [t.serialize() for t in self.arg_types],
'arg_kinds': self.arg_kinds,
'arg_names': self.arg_names,
'ret_type': self.ret_type.serialize(),
Expand All @@ -747,8 +753,7 @@ def serialize(self) -> JsonDict:
def deserialize(cls, data: JsonDict) -> 'CallableType':
assert data['.class'] == 'CallableType'
# TODO: Set definition to the containing SymbolNode?
return CallableType([(None if t is None else deserialize_type(t))
for t in data['arg_types']],
return CallableType([deserialize_type(t) for t in data['arg_types']],
data['arg_kinds'],
data['arg_names'],
deserialize_type(data['ret_type']),
Expand Down Expand Up @@ -782,7 +787,7 @@ def __init__(self, items: List[CallableType]) -> None:
def items(self) -> List[CallableType]:
return self._items

def name(self) -> str:
def name(self) -> Optional[str]:
return self.get_name()

def is_type_obj(self) -> bool:
Expand All @@ -801,7 +806,7 @@ def with_name(self, name: str) -> 'Overloaded':
ni.append(it.with_name(name))
return Overloaded(ni)

def get_name(self) -> str:
def get_name(self) -> Optional[str]:
return self._items[0].name

def accept(self, visitor: 'TypeVisitor[T]') -> T:
Expand Down Expand Up @@ -1422,8 +1427,9 @@ def visit_callable_type(self, t: CallableType) -> str:
s += '*'
if t.arg_kinds[i] == ARG_STAR2:
s += '**'
if t.arg_names[i]:
s += t.arg_names[i] + ': '
name = t.arg_names[i]
if name:
s += name + ': '
s += t.arg_types[i].accept(self)
if t.arg_kinds[i] in (ARG_OPT, ARG_NAMED_OPT):
s += ' ='
Expand Down