-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Fix crashes and fails in forward references #3952
Changes from 1 commit
45e5931
cb4caa5
1cdc980
260ef02
a58a217
950a022
411b24d
b9b8528
48d6de4
ec45441
ac32ed4
3fb3019
f9b1320
cf014b8
665236b
9a318aa
757fbd9
4502ce2
3b39d40
c8b28fe
9f92b0f
9779103
b914bdb
3568fdb
54d9331
b9ddacc
5bfe9ca
a2912e9
10c65b8
21dfbfe
f2ddbcd
03597ee
649ef32
83f8907
13c7176
79b10d6
321a809
076c909
c1a63ec
97e6f47
8f52654
6edd078
514b8bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4212,9 +4212,11 @@ def visit_decorator(self, dec: Decorator) -> None: | |
dec.var.type = sig | ||
|
||
def visit_assignment_stmt(self, s: AssignmentStmt) -> None: | ||
"""Traverse the actual assignment statement and synthetic types | ||
"""Traverse the assignment statement. | ||
|
||
This includes the actual assignment and synthetic types | ||
resulted from this assignment (if any). Currently this includes | ||
NewType, TypedDict, and NamedTuple. | ||
NewType, TypedDict, NamedTuple, and TypeVar. | ||
""" | ||
self.analyze(s.type, s) | ||
if isinstance(s.rvalue, IndexExpr) and isinstance(s.rvalue.analyzed, TypeAliasExpr): | ||
|
@@ -4240,6 +4242,9 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: | |
self.accept(sym.node) | ||
if isinstance(sym.node, Var): | ||
self.analyze(sym.node.type, sym.node) | ||
# We need to pay additional attention to assignments that define a type alias. | ||
# The resulting type is also stored in the 'type_override' attribute of | ||
# the corresponding SymbolTableNode. | ||
if isinstance(s.lvalues[0], RefExpr) and isinstance(s.lvalues[0].node, Var): | ||
self.analyze(s.lvalues[0].node.type, s.lvalues[0].node) | ||
if isinstance(s.lvalues[0], NameExpr): | ||
|
@@ -4347,8 +4352,8 @@ def analyze(self, type: Optional[Type], node: Union[Node, SymbolTableNode], | |
if indicator.get('forward') or indicator.get('synthetic'): | ||
def patch() -> None: | ||
self.perform_transform(node, | ||
lambda tp: tp.accept(TypeReplacer(self.fail, | ||
node, warn))) | ||
lambda tp: tp.accept(ForwardReferenceResolver(self.fail, | ||
node, warn))) | ||
self.patches.append(patch) | ||
|
||
def analyze_types(self, types: List[Type], node: Node) -> None: | ||
|
@@ -4368,8 +4373,8 @@ def analyze_types(self, types: List[Type], node: Node) -> None: | |
if indicator.get('forward') or indicator.get('synthetic'): | ||
def patch() -> None: | ||
self.perform_transform(node, | ||
lambda tp: tp.accept(TypeReplacer(self.fail, | ||
node, warn=False))) | ||
lambda tp: tp.accept(ForwardReferenceResolver(self.fail, | ||
node, warn=False))) | ||
self.patches.append(patch) | ||
|
||
def check_for_omitted_generics(self, typ: Type) -> None: | ||
|
@@ -4798,9 +4803,17 @@ def visit_any(self, t: AnyType) -> Type: | |
return t | ||
|
||
|
||
class TypeReplacer(TypeTranslator): | ||
"""This is very similar TypeTranslator but tracks visited nodes to avoid | ||
class ForwardReferenceResolver(TypeTranslator): | ||
"""Visitor to replace previously detected forward reference to synthetic types. | ||
|
||
This is similar to TypeTranslator but tracks visited nodes to avoid | ||
infinite recursion on potentially circular (self- or mutually-referential) types. | ||
This visitor: | ||
* Fixes forward references by unwrapping the linked type. | ||
* Generates errors for unsupported type recursion and breaks recursion by resolving | ||
recursive back references to Any types. | ||
* Replaces instance types generated from unanalyzed NamedTuple and TypedDict class syntax | ||
found in first pass with analyzed TupleType and TypedDictType. | ||
""" | ||
def __init__(self, fail: Callable[[str, Context], None], | ||
start: Union[Node, SymbolTableNode], warn: bool) -> None: | ||
|
@@ -4809,66 +4822,64 @@ def __init__(self, fail: Callable[[str, Context], None], | |
self.start = start | ||
self.warn = warn | ||
|
||
def warn_recursion(self) -> None: | ||
if self.warn: | ||
assert isinstance(self.start, Node), "Internal error: invalid error context" | ||
self.fail('Recursive types not fully supported yet,' | ||
' nested types replaced with "Any"', self.start) | ||
|
||
def check(self, t: Type) -> bool: | ||
def check_recursion(self, t: Type) -> bool: | ||
if any(t is s for s in self.seen): | ||
self.warn_recursion() | ||
if self.warn: | ||
assert isinstance(self.start, Node), "Internal error: invalid error context" | ||
self.fail('Recursive types not fully supported yet,' | ||
' nested types replaced with "Any"', self.start) | ||
return True | ||
self.seen.append(t) | ||
return False | ||
|
||
def visit_forwardref_type(self, t: ForwardRef) -> Type: | ||
"""This visitor method tracks situations like this: | ||
|
||
x: A # this type is not yet known and therefore wrapped in ForwardRef | ||
# it's content is updated in ThirdPass, now we need to unwrap this type. | ||
x: A # This type is not yet known and therefore wrapped in ForwardRef, | ||
# its content is updated in ThirdPass, now we need to unwrap this type. | ||
A = NewType('A', int) | ||
""" | ||
return t.link.accept(self) | ||
|
||
def visit_instance(self, t: Instance, from_fallback: bool = False) -> Type: | ||
"""This visitor method tracks situations like this: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Document |
||
|
||
x: A # when analyzing this type we will get an Instance from FirstPass | ||
# now we need to update this to actual analyzed TupleType. | ||
x: A # When analyzing this type we will get an Instance from FirstPass. | ||
# Now we need to update this to actual analyzed TupleType. | ||
class A(NamedTuple): | ||
attr: str | ||
|
||
If from_fallback is True, then we always return an Instance type. This is needed | ||
since TupleType and TypedDictType fallbacks are always instances. | ||
""" | ||
info = t.type | ||
# Special case, analyzed bases transformed the type into TupleType. | ||
if info.tuple_type and not from_fallback: | ||
items = [it.accept(self) for it in info.tuple_type.items] | ||
info.tuple_type.items = items | ||
return TupleType(items, Instance(info, [])) | ||
# Update forward Instance's to corresponding analyzed NamedTuple's. | ||
# Update forward Instances to corresponding analyzed NamedTuples. | ||
if info.replaced and info.replaced.tuple_type: | ||
tp = info.replaced.tuple_type | ||
if any((s is tp) or (s is t) for s in self.seen): | ||
self.warn_recursion() | ||
# The key idea is that when we return to the place where | ||
# we already was, we break the cycle and put AnyType as a leaf. | ||
if self.check_recursion(tp): | ||
# The key idea is that when we recursively return to a type already traversed, | ||
# then we break the cycle and put AnyType as a leaf. | ||
return AnyType(TypeOfAny.from_error) | ||
self.seen.append(t) | ||
return tp.copy_modified(fallback=Instance(info.replaced, [])).accept(self) | ||
# Same as above but for TypedDict's. | ||
# Same as above but for TypedDicts. | ||
if info.replaced and info.replaced.typeddict_type: | ||
td = info.replaced.typeddict_type | ||
if any((s is td) or (s is t) for s in self.seen): | ||
self.warn_recursion() | ||
if self.check_recursion(td): | ||
# We also break the cycles for TypedDicts as explained above for NamedTuples. | ||
return AnyType(TypeOfAny.from_error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add comment that references the above comment about the same situation with named tuples. |
||
self.seen.append(t) | ||
return td.copy_modified(fallback=Instance(info.replaced, [])).accept(self) | ||
if self.check(t): | ||
if self.check_recursion(t): | ||
# We also need to break a potential cycle with normal (non-synthetic) instance types. | ||
return Instance(t.type, [AnyType(TypeOfAny.from_error)] * len(t.type.defn.type_vars)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add comment about forcibly breaking recursion here to avoid infinite recursion. |
||
return super().visit_instance(t) | ||
|
||
def visit_type_var(self, t: TypeVarType) -> Type: | ||
if self.check(t): | ||
if self.check_recursion(t): | ||
return AnyType(TypeOfAny.from_error) | ||
if t.upper_bound: | ||
t.upper_bound = t.upper_bound.accept(self) | ||
|
@@ -4877,7 +4888,7 @@ def visit_type_var(self, t: TypeVarType) -> Type: | |
return t | ||
|
||
def visit_callable_type(self, t: CallableType) -> Type: | ||
if self.check(t): | ||
if self.check_recursion(t): | ||
return AnyType(TypeOfAny.from_error) | ||
arg_types = [tp.accept(self) for tp in t.arg_types] | ||
ret_type = t.ret_type.accept(self) | ||
|
@@ -4890,29 +4901,29 @@ def visit_callable_type(self, t: CallableType) -> Type: | |
return t.copy_modified(arg_types=arg_types, ret_type=ret_type, variables=variables) | ||
|
||
def visit_overloaded(self, t: Overloaded) -> Type: | ||
if self.check(t): | ||
if self.check_recursion(t): | ||
return AnyType(TypeOfAny.from_error) | ||
return super().visit_overloaded(t) | ||
|
||
def visit_tuple_type(self, t: TupleType) -> Type: | ||
if self.check(t): | ||
if self.check_recursion(t): | ||
return AnyType(TypeOfAny.from_error) | ||
items = [it.accept(self) for it in t.items] | ||
fallback = self.visit_instance(t.fallback, from_fallback=True) | ||
assert isinstance(fallback, Instance) | ||
return TupleType(items, fallback) | ||
|
||
def visit_typeddict_type(self, t: TypedDictType) -> Type: | ||
if self.check(t): | ||
if self.check_recursion(t): | ||
return AnyType(TypeOfAny.from_error) | ||
return super().visit_typeddict_type(t) | ||
|
||
def visit_union_type(self, t: UnionType) -> Type: | ||
if self.check(t): | ||
if self.check_recursion(t): | ||
return AnyType(TypeOfAny.from_error) | ||
return super().visit_union_type(t) | ||
|
||
def visit_type_type(self, t: TypeType) -> Type: | ||
if self.check(t): | ||
if self.check_recursion(t): | ||
return AnyType(TypeOfAny.from_error) | ||
return super().visit_type_type(t) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1376,8 +1376,21 @@ def deserialize(cls, data: JsonDict) -> Type: | |
|
||
|
||
class ForwardRef(Type): | ||
"""Class to wrap forward references to other types.""" | ||
link = None # type: Type # the wrapped type | ||
"""Class to wrap forward references to other types. | ||
|
||
This is used when a forward reference to an (unanalyzed) synthetic type is found, | ||
for example: | ||
|
||
x: A | ||
A = TypedDict('A', {'x': int}) | ||
|
||
To avoid false positives and crashes in such situations, we first wrap the second | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be 'the first occurrence ...'? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, will fix this docstring. |
||
occurrence of 'A' in ForwardRef. Then, the wrapped UnboundType is updated in the third | ||
pass of semantic analysis and ultimately fixed in the patches after the third pass. | ||
So that ForwardRefs are temporary and will be completely replaced with the linked types | ||
or Any (to avoid cyclic references) before the type checking stage. | ||
""" | ||
link = None # type: Type # The wrapped type | ||
|
||
def __init__(self, link: Type) -> None: | ||
self.link = link | ||
|
@@ -1392,6 +1405,8 @@ def serialize(self): | |
name = self.link.type.name() | ||
else: | ||
name = self.link.__class__.__name__ | ||
# We should never get here since all forward references should be resolved | ||
# and removed during semantic analysis. | ||
assert False, "Internal error: Unresolved forward reference to {}".format(name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add comment about why we shouldn't get here (all forward references should be resolved and removed during semantic analysis). |
||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3611,6 +3611,10 @@ NameInfo = NewType('NameInfo', NameInfoBase) | |
def parse_ast(name_dict: NameDict) -> None: | ||
if isinstance(name_dict[''], int): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reveal tyhe type of |
||
pass | ||
x = name_dict[''] | ||
reveal_type(x) # E: Revealed type is '__main__.NameInfo*' | ||
x = NameInfo(NameInfoBase()) # OK | ||
x = NameInfoBase() # E: Incompatible types in assignment (expression has type "NameInfoBase", variable has type "NameInfo") | ||
[builtins fixtures/isinstancelist.pyi] | ||
[out] | ||
|
||
|
@@ -3666,12 +3670,13 @@ from typing import Union, NamedTuple | |
|
||
NodeType = Union['Foo', 'Bar'] | ||
class Foo(NamedTuple): | ||
pass | ||
x: int | ||
class Bar(NamedTuple): | ||
pass | ||
x: int | ||
|
||
def foo(node: NodeType): | ||
def foo(node: NodeType) -> int: | ||
x = node | ||
return x.x | ||
[out] | ||
|
||
[case testCrashOnForwardUnionOfTypedDicts] | ||
|
@@ -3680,12 +3685,13 @@ from typing import Union | |
|
||
NodeType = Union['Foo', 'Bar'] | ||
class Foo(TypedDict): | ||
pass | ||
x: int | ||
class Bar(TypedDict): | ||
pass | ||
x: int | ||
|
||
def foo(node: NodeType): | ||
def foo(node: NodeType) -> int: | ||
x = node | ||
return x['x'] | ||
[builtins fixtures/isinstancelist.pyi] | ||
[out] | ||
|
||
|
@@ -3696,8 +3702,9 @@ NodeType = Union['Foo', 'Bar'] | |
Foo = NewType('Foo', int) | ||
Bar = NewType('Bar', str) | ||
|
||
def foo(node: NodeType): | ||
def foo(node: NodeType) -> NodeType: | ||
x = node | ||
return x | ||
[out] | ||
|
||
[case testCrashOnComplexCheckWithNamedTupleUnion] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unclear what this is for. Please add a comment.