Skip to content

Make TypeVisitor and SyntheticTypeVisitor (almost) fully abstract #7312

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions mypy/fixup.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ def visit_overloaded(self, t: Overloaded) -> None:
for ct in t.items():
ct.accept(self)

def visit_erased_type(self, o: Any) -> None:
# This type should exist only temporarily during type inference
raise RuntimeError("Shouldn't get here", o)

def visit_deleted_type(self, o: Any) -> None:
pass # Nothing to descend into.

Expand Down
19 changes: 2 additions & 17 deletions mypy/indirection.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from typing import Dict, Iterable, List, Optional, Set, Union

from mypy.types import SyntheticTypeVisitor
from mypy.types import TypeVisitor
import mypy.types as types
from mypy.util import split_module_names

Expand All @@ -15,7 +15,7 @@ def extract_module_names(type_name: Optional[str]) -> List[str]:
return []


class TypeIndirectionVisitor(SyntheticTypeVisitor[Set[str]]):
class TypeIndirectionVisitor(TypeVisitor[Set[str]]):
"""Returns all module references within a particular type."""

def __init__(self) -> None:
Expand All @@ -39,12 +39,6 @@ def _visit(self, typ_or_typs: Union[types.Type, Iterable[types.Type]]) -> Set[st
def visit_unbound_type(self, t: types.UnboundType) -> Set[str]:
return self._visit(t.args)

def visit_type_list(self, t: types.TypeList) -> Set[str]:
return self._visit(t.items)

def visit_callable_argument(self, t: types.CallableArgument) -> Set[str]:
return self._visit(t.typ)

def visit_any(self, t: types.AnyType) -> Set[str]:
return set()

Expand Down Expand Up @@ -90,23 +84,14 @@ def visit_tuple_type(self, t: types.TupleType) -> Set[str]:
def visit_typeddict_type(self, t: types.TypedDictType) -> Set[str]:
return self._visit(t.items.values()) | self._visit(t.fallback)

def visit_raw_expression_type(self, t: types.RawExpressionType) -> Set[str]:
assert False, "Unexpected RawExpressionType after semantic analysis phase"

def visit_literal_type(self, t: types.LiteralType) -> Set[str]:
return self._visit(t.fallback)

def visit_star_type(self, t: types.StarType) -> Set[str]:
return set()

def visit_union_type(self, t: types.UnionType) -> Set[str]:
return self._visit(t.items)

def visit_partial_type(self, t: types.PartialType) -> Set[str]:
return set()

def visit_ellipsis_type(self, t: types.EllipsisType) -> Set[str]:
return set()

def visit_type_type(self, t: types.TypeType) -> Set[str]:
return self._visit(t.item)
12 changes: 10 additions & 2 deletions mypy/server/astmerge.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@
)
from mypy.traverser import TraverserVisitor
from mypy.types import (
Type, SyntheticTypeVisitor, Instance, AnyType, NoneType, CallableType, DeletedType,
Type, SyntheticTypeVisitor, Instance, AnyType, NoneType, CallableType, ErasedType, DeletedType,
TupleType, TypeType, TypeVarType, TypedDictType, UnboundType, UninhabitedType, UnionType,
Overloaded, TypeVarDef, TypeList, CallableArgument, EllipsisType, StarType, LiteralType,
RawExpressionType, PartialType,
RawExpressionType, PartialType, PlaceholderType,
)
from mypy.util import get_prefix, replace_object_state
from mypy.typestate import TypeState
Expand Down Expand Up @@ -373,6 +373,10 @@ def visit_overloaded(self, t: Overloaded) -> None:
if t.fallback is not None:
t.fallback.accept(self)

def visit_erased_type(self, t: ErasedType) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Does it actually break some tests when you add a raise here?

# This type should exist only temporarily during type inference
raise RuntimeError

def visit_deleted_type(self, typ: DeletedType) -> None:
pass

Expand Down Expand Up @@ -429,6 +433,10 @@ def visit_union_type(self, typ: UnionType) -> None:
for item in typ.items:
item.accept(self)

def visit_placeholder_type(self, t: PlaceholderType) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

I think this also should raise, since this should be called after semantic analysis is done. But maybe I am wrong because there are many visit methods for synthetic types. Or maybe it was mistakenly made SyntheticTypeVisitor instead of just TypeVisitor? Anyway, this is not important for this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was intentional that this visitor is a SyntheticTypeVisitor -- we apparently try running this visitor on FuncBase.unanalyzed_type in the process_base_func method up above. Not entirely sure why, but I figured it'd be safest to just leave this alone.

for item in t.args:
item.accept(self)

# Helpers

def fixup(self, node: SN) -> SN:
Expand Down
6 changes: 5 additions & 1 deletion mypy/server/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class 'mod.Cls'. This can also refer to an attribute inherited from a
from mypy.types import (
Type, Instance, AnyType, NoneType, TypeVisitor, CallableType, DeletedType, PartialType,
TupleType, TypeType, TypeVarType, TypedDictType, UnboundType, UninhabitedType, UnionType,
FunctionLike, Overloaded, TypeOfAny, LiteralType, get_proper_type, ProperType
FunctionLike, Overloaded, TypeOfAny, LiteralType, ErasedType, get_proper_type, ProperType
)
from mypy.server.trigger import make_trigger, make_wildcard_trigger
from mypy.util import correct_relative_import
Expand Down Expand Up @@ -901,6 +901,10 @@ def visit_overloaded(self, typ: Overloaded) -> List[str]:
triggers.extend(self.get_type_triggers(item))
return triggers

def visit_erased_type(self, t: ErasedType) -> List[str]:
Copy link
Member

Choose a reason for hiding this comment

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

And this one too.

# This type should exist only temporarily during type inference
assert False, "Should not see an erased type here"

def visit_deleted_type(self, typ: DeletedType) -> List[str]:
return []

Expand Down
22 changes: 8 additions & 14 deletions mypy/type_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ class TypeVisitor(Generic[T]):
The parameter T is the return type of the visit methods.
"""

def _notimplemented_helper(self, name: str) -> NotImplementedError:
return NotImplementedError("Method {}.visit_{}() not implemented\n"
.format(type(self).__name__, name)
+ "This is a known bug, track development in "
+ "'https://github.com/JukkaL/mypy/issues/730'")

@abstractmethod
def visit_unbound_type(self, t: UnboundType) -> T:
pass
Expand All @@ -56,8 +50,9 @@ def visit_none_type(self, t: NoneType) -> T:
def visit_uninhabited_type(self, t: UninhabitedType) -> T:
pass

@abstractmethod
def visit_erased_type(self, t: ErasedType) -> T:
raise self._notimplemented_helper('erased_type')
pass

@abstractmethod
def visit_deleted_type(self, t: DeletedType) -> T:
Expand All @@ -75,8 +70,9 @@ def visit_instance(self, t: Instance) -> T:
def visit_callable_type(self, t: CallableType) -> T:
pass

@abstractmethod
def visit_overloaded(self, t: Overloaded) -> T:
raise self._notimplemented_helper('overloaded')
pass

@abstractmethod
def visit_tuple_type(self, t: TupleType) -> T:
Expand Down Expand Up @@ -105,9 +101,6 @@ def visit_type_type(self, t: TypeType) -> T:
def visit_type_alias_type(self, t: TypeAliasType) -> T:
raise NotImplementedError('TODO')

def visit_placeholder_type(self, t: PlaceholderType) -> T:
raise RuntimeError('Internal error: unresolved placeholder type {}'.format(t.fullname))


@trait
class SyntheticTypeVisitor(TypeVisitor[T]):
Expand Down Expand Up @@ -135,6 +128,10 @@ def visit_ellipsis_type(self, t: EllipsisType) -> T:
def visit_raw_expression_type(self, t: RawExpressionType) -> T:
pass

@abstractmethod
def visit_placeholder_type(self, t: PlaceholderType) -> T:
pass


@trait
class TypeTranslator(TypeVisitor[Type]):
Expand Down Expand Up @@ -235,9 +232,6 @@ def visit_overloaded(self, t: Overloaded) -> Type:
def visit_type_type(self, t: TypeType) -> Type:
return TypeType.make_normalized(t.item.accept(self), line=t.line, column=t.column)

def visit_placeholder_type(self, t: PlaceholderType) -> Type:
return PlaceholderType(t.fullname, self.translate_types(t.args), t.line)


@trait
class TypeQuery(SyntheticTypeVisitor[T]):
Expand Down
16 changes: 14 additions & 2 deletions mypy/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
from mypy.options import Options
from mypy.types import (
Type, UnboundType, TypeVarType, TupleType, TypedDictType, UnionType, Instance, AnyType,
CallableType, NoneType, DeletedType, TypeList, TypeVarDef, SyntheticTypeVisitor,
CallableType, NoneType, ErasedType, DeletedType, TypeList, TypeVarDef, SyntheticTypeVisitor,
StarType, PartialType, EllipsisType, UninhabitedType, TypeType, replace_alias_tvars,
CallableArgument, get_type_vars, TypeQuery, union_items, TypeOfAny,
LiteralType, RawExpressionType, PlaceholderType, get_proper_type, ProperType
LiteralType, RawExpressionType, PlaceholderType, Overloaded, get_proper_type, ProperType
)

from mypy.nodes import (
Expand Down Expand Up @@ -456,6 +456,10 @@ def visit_none_type(self, t: NoneType) -> Type:
def visit_uninhabited_type(self, t: UninhabitedType) -> Type:
return t

def visit_erased_type(self, t: ErasedType) -> Type:
Copy link
Member

Choose a reason for hiding this comment

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

I think this too should raise, see visit_partial_type() below.

# This type should exist only temporarily during type inference
assert False, "Internal error: Unexpected erased type"

def visit_deleted_type(self, t: DeletedType) -> Type:
return t

Expand Down Expand Up @@ -490,6 +494,14 @@ def visit_callable_type(self, t: CallableType, nested: bool = True) -> Type:
variables=self.anal_var_defs(variables))
return ret

def visit_overloaded(self, t: Overloaded) -> Type:
# Overloaded types are manually constructed in semanal.py by analyzing the
# AST and combining together the Callable types this visitor converts.
#
# So if we're ever asked to reanalyze an Overloaded type, we know it's
# fine to just return it as-is.
return t
Copy link
Member

Choose a reason for hiding this comment

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

Why this is so different from visit_callable_type() above?


def visit_tuple_type(self, t: TupleType) -> Type:
# Types such as (t1, t2, ...) only allowed in assignment statements. They'll
# generate errors elsewhere, and Tuple[t1, t2, ...] must be used instead.
Expand Down
1 change: 1 addition & 0 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1947,6 +1947,7 @@ def __init__(self, fullname: Optional[str], args: List[Type], line: int) -> None
self.args = args

def accept(self, visitor: 'TypeVisitor[T]') -> T:
assert isinstance(visitor, SyntheticTypeVisitor)
return visitor.visit_placeholder_type(self)

def serialize(self) -> str:
Expand Down