Skip to content

Commit

Permalink
Start using TypeAliasType in the semantic analyzer (#7923)
Browse files Browse the repository at this point in the history
This PR starts using the new `TypeAliasType` in the semantic analyzer. This PR doesn't yet pulls the trigger to enable the recursive types, but it is now essentially one line of code away. This PR:
* Makes type analyzer return a `TypeAliasType` instead of eagerly expanding the alias target.
* Refactors `TypeAliasExpr` to refer to `TypeAlias` (sorry for the noise).
* Makes few minor fixes to make all existing tests pass.
* Adds few logistic changes around `get_proper_type()` I found necessary while playing with actual recursive types over the weekend.

Here are some strategical comments:
* Taking into account how easy it was to make all existing tests pass, I don't think it is necessary to introduce a hidden option flag that would eagerly expand all type aliases after semantic analyzis.
It would probably make sense to test this well locally before a public release.
* There is a special case for no arguments generic aliases. Currently one is allowed to write `L = List; x: L[int]`, I preserve this by using eager expansion in this special case, otherwise it would complicate the whole logic significantly. This is also mostly a legacy thing because we have built-in aliases like `List = list` magically added by semantic analyzer.
* I have found that just carelessly sprinkling `get_proper_type()` is not a best strategy. It saves all the existing special-casing but also introduces a risk for infinite recursion. In particular, "type ops tangle" should ideally always pass on the original alias type. Unfortunately, there is no way to fix/enforce this (without having some severe performance impact). Note it is mostly fine to "carelessly" use `get_proper_type()` in the "front end" (like `checker.py`, `checkexpr.py`, `checkmember.py` etc).

Here is my plan for the next five PRs:
1. I am going to try merging `SubtypeVisitor` and `ProperSubtypeVisitor`, there is very large amount of code duplication (there is already an issue for this).
2. I am going to try to get rid of `sametypes.py` (I am going to open a new issue, see my arguments there).
3. I am going to enable the recursive aliases and add sufficiently many tests to be sure we are safe about infinite recursion in type ops.
4. I am going to change how named tuples and typed dicts are represented internally, currently they are stored as `TypeInfo`s, but will be stored as `TypeAlias`. Essentially there will be almost no difference between `A = Tuple[int, int]` and `A = NamedTuple('A', [('x', int), ('y', int)])`. This will allow typed dicts and named tuple participate in recursive types.
5. I am going to switch from using unbound type variables to bound type variables for generic type aliases, since now they are almost identical to `TypeInfo`s so it IMO it really makes sense to make them uniform (and avoid confusions and code duplication in future).
5a. Potentially as a follow-up I may add support for generic named tuples and typed dicts, since steps 4 plus 5 will make this almost trivial.

There is another important thing to call out, previously unions never contained another unions as items (because constructor flattened them), and some code might implicitly rely on this. IMO we should probably update these places, since maintaining this guarantee may be painful.

Yet another important thing is that this may break many plugins, so we need to announce this in #6617 when will merge this.
  • Loading branch information
ilevkivskyi authored Nov 14, 2019
1 parent e97377c commit 47bafd6
Show file tree
Hide file tree
Showing 15 changed files with 136 additions and 86 deletions.
15 changes: 6 additions & 9 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,7 @@ def analyze_ref_expr(self, e: RefExpr, lvalue: bool = False) -> Type:
# Something that refers to a type alias appears in runtime context.
# Note that we suppress bogus errors for alias redefinitions,
# they are already reported in semanal.py.
result = self.alias_type_in_runtime_context(node.target, node.alias_tvars,
node.no_args, e,
result = self.alias_type_in_runtime_context(node, node.no_args, e,
alias_definition=e.is_alias_rvalue
or lvalue)
else:
Expand Down Expand Up @@ -2917,9 +2916,7 @@ def visit_type_application(self, tapp: TypeApplication) -> Type:
"""
if isinstance(tapp.expr, RefExpr) and isinstance(tapp.expr.node, TypeAlias):
# Subscription of a (generic) alias in runtime context, expand the alias.
target = tapp.expr.node.target
all_vars = tapp.expr.node.alias_tvars
item = expand_type_alias(target, all_vars, tapp.types, self.chk.fail,
item = expand_type_alias(tapp.expr.node, tapp.types, self.chk.fail,
tapp.expr.node.no_args, tapp)
item = get_proper_type(item)
if isinstance(item, Instance):
Expand Down Expand Up @@ -2951,10 +2948,10 @@ def visit_type_alias_expr(self, alias: TypeAliasExpr) -> Type:
both `reveal_type` instances will reveal the same type `def (...) -> builtins.list[Any]`.
Note that type variables are implicitly substituted with `Any`.
"""
return self.alias_type_in_runtime_context(alias.type, alias.tvars, alias.no_args,
return self.alias_type_in_runtime_context(alias.node, alias.no_args,
alias, alias_definition=True)

def alias_type_in_runtime_context(self, target: Type, alias_tvars: List[str],
def alias_type_in_runtime_context(self, alias: TypeAlias,
no_args: bool, ctx: Context,
*,
alias_definition: bool = False) -> Type:
Expand All @@ -2971,14 +2968,14 @@ class LongName(Generic[T]): ...
x = A()
y = cast(A, ...)
"""
if isinstance(target, Instance) and target.invalid: # type: ignore
if isinstance(alias.target, Instance) and alias.target.invalid: # type: ignore
# An invalid alias, error already has been reported
return AnyType(TypeOfAny.from_error)
# If this is a generic alias, we set all variables to `Any`.
# For example:
# A = List[Tuple[T, T]]
# x = A() <- same as List[Tuple[Any, Any]], see PEP 484.
item = get_proper_type(set_any_tvars(target, alias_tvars, ctx.line, ctx.column))
item = get_proper_type(set_any_tvars(alias, ctx.line, ctx.column))
if isinstance(item, Instance):
# Normally we get a callable type (or overloaded) with .is_type_obj() true
# representing the class's constructor
Expand Down
4 changes: 2 additions & 2 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,8 @@ def instance_alias_type(alias: TypeAlias,
target = get_proper_type(alias.target) # type: Type
assert isinstance(get_proper_type(target),
Instance), "Must be called only with aliases to classes"
target = set_any_tvars(target, alias.alias_tvars, alias.line, alias.column)
assert isinstance(target, Instance) # type: ignore[misc]
target = get_proper_type(set_any_tvars(alias, alias.line, alias.column))
assert isinstance(target, Instance)
tp = type_object_type(target.type, builtin_type)
return expand_type_by_instance(tp, target)

Expand Down
6 changes: 3 additions & 3 deletions mypy/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ def infer_constraints(template: Type, actual: Type,
"""
if any(get_proper_type(template) == get_proper_type(t) for t in TypeState._inferring):
return []
if (isinstance(template, TypeAliasType) and isinstance(actual, TypeAliasType) and
template.is_recursive and actual.is_recursive):
if isinstance(template, TypeAliasType) and template.is_recursive:
# This case requires special care because it may cause infinite recursion.
TypeState._inferring.append(template)
res = _infer_constraints(template, actual, direction)
Expand All @@ -105,6 +104,7 @@ def infer_constraints(template: Type, actual: Type,
def _infer_constraints(template: Type, actual: Type,
direction: int) -> List[Constraint]:

orig_template = template
template = get_proper_type(template)
actual = get_proper_type(actual)

Expand All @@ -129,7 +129,7 @@ def _infer_constraints(template: Type, actual: Type,
if direction == SUPERTYPE_OF and isinstance(actual, UnionType):
res = []
for a_item in actual.items:
res.extend(infer_constraints(template, a_item, direction))
res.extend(infer_constraints(orig_template, a_item, direction))
return res

# Now the potential subtype is known not to be a Union or a type
Expand Down
9 changes: 5 additions & 4 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2116,11 +2116,12 @@ class TypeAliasExpr(Expression):
# A = List[Any]
no_args = False # type: bool

def __init__(self, type: 'mypy.types.Type', tvars: List[str], no_args: bool) -> None:
def __init__(self, node: 'TypeAlias') -> None:
super().__init__()
self.type = type
self.tvars = tvars
self.no_args = no_args
self.type = node.target
self.tvars = node.alias_tvars
self.no_args = node.no_args
self.node = node

def accept(self, visitor: ExpressionVisitor[T]) -> T:
return visitor.visit_type_alias_expr(self)
Expand Down
10 changes: 6 additions & 4 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -2495,18 +2495,20 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
context=s)
# When this type alias gets "inlined", the Any is not explicit anymore,
# so we need to replace it with non-explicit Anys.
res = make_any_non_explicit(res)
if not has_placeholder(res):
res = make_any_non_explicit(res)
no_args = isinstance(res, Instance) and not res.args # type: ignore
fix_instance_types(res, self.fail, self.note)
alias_node = TypeAlias(res, self.qualified_name(lvalue.name), s.line, s.column,
alias_tvars=alias_tvars, no_args=no_args)
if isinstance(s.rvalue, (IndexExpr, CallExpr)): # CallExpr is for `void = type(None)`
s.rvalue.analyzed = TypeAliasExpr(res, alias_tvars, no_args)
s.rvalue.analyzed = TypeAliasExpr(alias_node)
s.rvalue.analyzed.line = s.line
# we use the column from resulting target, to get better location for errors
s.rvalue.analyzed.column = res.column
elif isinstance(s.rvalue, RefExpr):
s.rvalue.is_alias_rvalue = True
alias_node = TypeAlias(res, self.qualified_name(lvalue.name), s.line, s.column,
alias_tvars=alias_tvars, no_args=no_args)

if existing:
# An alias gets updated.
updated = False
Expand Down
19 changes: 17 additions & 2 deletions mypy/semanal_typeargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
operations, including subtype checks.
"""

from typing import List, Optional
from typing import List, Optional, Set

from mypy.nodes import TypeInfo, Context, MypyFile, FuncItem, ClassDef, Block
from mypy.types import Type, Instance, TypeVarType, AnyType, get_proper_types
from mypy.types import (
Type, Instance, TypeVarType, AnyType, get_proper_types, TypeAliasType, get_proper_type
)
from mypy.mixedtraverser import MixedTraverserVisitor
from mypy.subtypes import is_subtype
from mypy.sametypes import is_same_type
Expand All @@ -27,6 +29,9 @@ def __init__(self, errors: Errors, options: Options, is_typeshed_file: bool) ->
self.scope = Scope()
# Should we also analyze function definitions, or only module top-levels?
self.recurse_into_functions = True
# Keep track of the type aliases already visited. This is needed to avoid
# infinite recursion on types like A = Union[int, List[A]].
self.seen_aliases = set() # type: Set[TypeAliasType]

def visit_mypy_file(self, o: MypyFile) -> None:
self.errors.set_file(o.path, o.fullname, scope=self.scope)
Expand All @@ -48,6 +53,16 @@ def visit_block(self, o: Block) -> None:
if not o.is_unreachable:
super().visit_block(o)

def visit_type_alias_type(self, t: TypeAliasType) -> None:
super().visit_type_alias_type(t)
if t in self.seen_aliases:
# Avoid infinite recursion on recursive type aliases.
# Note: it is fine to skip the aliases we have already seen in non-recursive types,
# since errors there have already already reported.
return
self.seen_aliases.add(t)
get_proper_type(t).accept(self)

def visit_instance(self, t: Instance) -> None:
# Type argument counts were checked in the main semantic analyzer pass. We assume
# that the counts are correct here.
Expand Down
4 changes: 4 additions & 0 deletions mypy/server/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,10 @@ def visit_type_alias_type(self, typ: TypeAliasType) -> List[str]:
triggers = [trigger]
for arg in typ.args:
triggers.extend(self.get_type_triggers(arg))
# TODO: Add guard for infinite recursion here. Moreover, now that type aliases
# are its own kind of types we can simplify the logic to rely on intermediate
# dependencies (like for instance types).
triggers.extend(self.get_type_triggers(typ.alias.target))
return triggers

def visit_any(self, typ: AnyType) -> List[str]:
Expand Down
32 changes: 20 additions & 12 deletions mypy/subtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
Type, AnyType, UnboundType, TypeVisitor, FormalArgument, NoneType,
Instance, TypeVarType, CallableType, TupleType, TypedDictType, UnionType, Overloaded,
ErasedType, PartialType, DeletedType, UninhabitedType, TypeType, is_named_instance,
FunctionLike, TypeOfAny, LiteralType, ProperType, get_proper_type, TypeAliasType
FunctionLike, TypeOfAny, LiteralType, get_proper_type, TypeAliasType
)
import mypy.applytype
import mypy.constraints
Expand Down Expand Up @@ -103,6 +103,8 @@ def _is_subtype(left: Type, right: Type,
ignore_pos_arg_names: bool = False,
ignore_declared_variance: bool = False,
ignore_promotions: bool = False) -> bool:
orig_right = right
orig_left = left
left = get_proper_type(left)
right = get_proper_type(right)

Expand All @@ -113,7 +115,7 @@ def _is_subtype(left: Type, right: Type,
# Normally, when 'left' is not itself a union, the only way
# 'left' can be a subtype of the union 'right' is if it is a
# subtype of one of the items making up the union.
is_subtype_of_item = any(is_subtype(left, item,
is_subtype_of_item = any(is_subtype(orig_left, item,
ignore_type_params=ignore_type_params,
ignore_pos_arg_names=ignore_pos_arg_names,
ignore_declared_variance=ignore_declared_variance,
Expand All @@ -130,7 +132,7 @@ def _is_subtype(left: Type, right: Type,
elif is_subtype_of_item:
return True
# otherwise, fall through
return left.accept(SubtypeVisitor(right,
return left.accept(SubtypeVisitor(orig_right,
ignore_type_params=ignore_type_params,
ignore_pos_arg_names=ignore_pos_arg_names,
ignore_declared_variance=ignore_declared_variance,
Expand All @@ -155,13 +157,14 @@ def is_equivalent(a: Type, b: Type,

class SubtypeVisitor(TypeVisitor[bool]):

def __init__(self, right: ProperType,
def __init__(self, right: Type,
*,
ignore_type_params: bool,
ignore_pos_arg_names: bool = False,
ignore_declared_variance: bool = False,
ignore_promotions: bool = False) -> None:
self.right = right
self.right = get_proper_type(right)
self.orig_right = right
self.ignore_type_params = ignore_type_params
self.ignore_pos_arg_names = ignore_pos_arg_names
self.ignore_declared_variance = ignore_declared_variance
Expand Down Expand Up @@ -449,7 +452,7 @@ def visit_overloaded(self, left: Overloaded) -> bool:
return False

def visit_union_type(self, left: UnionType) -> bool:
return all(self._is_subtype(item, self.right) for item in left.items)
return all(self._is_subtype(item, self.orig_right) for item in left.items)

def visit_partial_type(self, left: PartialType) -> bool:
# This is indeterminate as we don't really know the complete type yet.
Expand Down Expand Up @@ -1083,7 +1086,8 @@ def restrict_subtype_away(t: Type, s: Type, *, ignore_promotions: bool = False)
s = get_proper_type(s)

if isinstance(t, UnionType):
new_items = [item for item in t.relevant_items()
new_items = [restrict_subtype_away(item, s, ignore_promotions=ignore_promotions)
for item in t.relevant_items()
if (isinstance(get_proper_type(item), AnyType) or
not covers_at_runtime(item, s, ignore_promotions))]
return UnionType.make_union(new_items)
Expand Down Expand Up @@ -1139,22 +1143,26 @@ def is_proper_subtype(left: Type, right: Type, *, ignore_promotions: bool = Fals

def _is_proper_subtype(left: Type, right: Type, *, ignore_promotions: bool = False,
erase_instances: bool = False) -> bool:
orig_left = left
orig_right = right
left = get_proper_type(left)
right = get_proper_type(right)

if isinstance(right, UnionType) and not isinstance(left, UnionType):
return any([is_proper_subtype(left, item, ignore_promotions=ignore_promotions,
return any([is_proper_subtype(orig_left, item, ignore_promotions=ignore_promotions,
erase_instances=erase_instances)
for item in right.items])
return left.accept(ProperSubtypeVisitor(right, ignore_promotions=ignore_promotions,
return left.accept(ProperSubtypeVisitor(orig_right,
ignore_promotions=ignore_promotions,
erase_instances=erase_instances))


class ProperSubtypeVisitor(TypeVisitor[bool]):
def __init__(self, right: ProperType, *,
def __init__(self, right: Type, *,
ignore_promotions: bool = False,
erase_instances: bool = False) -> None:
self.right = right
self.right = get_proper_type(right)
self.orig_right = right
self.ignore_promotions = ignore_promotions
self.erase_instances = erase_instances
self._subtype_kind = ProperSubtypeVisitor.build_subtype_kind(
Expand Down Expand Up @@ -1313,7 +1321,7 @@ def visit_overloaded(self, left: Overloaded) -> bool:
return False

def visit_union_type(self, left: UnionType) -> bool:
return all([self._is_proper_subtype(item, self.right) for item in left.items])
return all([self._is_proper_subtype(item, self.orig_right) for item in left.items])

def visit_partial_type(self, left: PartialType) -> bool:
# TODO: What's the right thing to do here?
Expand Down
2 changes: 1 addition & 1 deletion mypy/treetransform.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ def visit_type_var_expr(self, node: TypeVarExpr) -> TypeVarExpr:
self.type(node.upper_bound), variance=node.variance)

def visit_type_alias_expr(self, node: TypeAliasExpr) -> TypeAliasExpr:
return TypeAliasExpr(node.type, node.tvars, node.no_args)
return TypeAliasExpr(node.node)

def visit_newtype_expr(self, node: NewTypeExpr) -> NewTypeExpr:
res = NewTypeExpr(node.name, node.old_type, line=node.line, column=node.column)
Expand Down
29 changes: 19 additions & 10 deletions mypy/type_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

from abc import abstractmethod
from collections import OrderedDict
from typing import Generic, TypeVar, cast, Any, List, Callable, Iterable, Optional
from typing import Generic, TypeVar, cast, Any, List, Callable, Iterable, Optional, Set
from mypy_extensions import trait

T = TypeVar('T')
Expand Down Expand Up @@ -246,14 +246,21 @@ def visit_type_alias_type(self, t: TypeAliasType) -> Type:
class TypeQuery(SyntheticTypeVisitor[T]):
"""Visitor for performing queries of types.
strategy is used to combine results for a series of types
strategy is used to combine results for a series of types,
common use cases involve a boolean query using `any` or `all`.
Common use cases involve a boolean query using `any` or `all`
Note: this visitor keeps an internal state (tracks type aliases to avoid
recursion), so it should *never* be re-used for querying different types,
create a new visitor instance instead.
# TODO: check that we don't have existing violations of this rule.
"""

def __init__(self, strategy: Callable[[Iterable[T]], T]) -> None:
self.strategy = strategy
self.seen = [] # type: List[Type]
# Keep track of the type aliases already visited. This is needed to avoid
# infinite recursion on types like A = Union[int, List[A]].
self.seen_aliases = set() # type: Set[TypeAliasType]

def visit_unbound_type(self, t: UnboundType) -> T:
return self.query_types(t.args)
Expand Down Expand Up @@ -329,14 +336,16 @@ def query_types(self, types: Iterable[Type]) -> T:
"""Perform a query for a list of types.
Use the strategy to combine the results.
Skip types already visited types to avoid infinite recursion.
Note: types can be recursive until they are fully analyzed and "unentangled"
in patches after the semantic analysis.
Skip type aliases already visited types to avoid infinite recursion.
"""
res = [] # type: List[T]
for t in types:
if any(t is s for s in self.seen):
continue
self.seen.append(t)
if isinstance(t, TypeAliasType):
# Avoid infinite recursion for recursive type aliases.
# TODO: Ideally we should fire subvisitors here (or use caching) if we care
# about duplicates.
if t in self.seen_aliases:
continue
self.seen_aliases.add(t)
res.append(t.accept(self))
return self.strategy(res)
Loading

0 comments on commit 47bafd6

Please sign in to comment.