Skip to content

Commit

Permalink
Fix crash on recursive type aliases (#565)
Browse files Browse the repository at this point in the history
  • Loading branch information
JelleZijlstra authored Nov 9, 2022
1 parent 825b380 commit a6cc64f
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 12 deletions.
1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- Fix crash on recursive type aliases. Recursive type aliases now fall back to `Any` (#565)
- Support `in` on objects with only `__getitem__` (#564)
- Add support for `except*` (PEP 654) (#562)
- Add type inference support for more constructs in `except` and `except*` (#562)
Expand Down
38 changes: 28 additions & 10 deletions pyanalyze/annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@
cast,
Container,
ContextManager,
Generator,
Mapping,
NamedTuple,
NewType,
Optional,
Sequence,
Set,
Tuple,
TYPE_CHECKING,
TypeVar,
Expand Down Expand Up @@ -138,11 +140,29 @@ class Context:

should_suppress_undefined_names: bool = field(default=False, init=False)
"""While this is True, no errors are shown for undefined names."""
_being_evaluated: Set[int] = field(default_factory=set, init=False)

def suppress_undefined_names(self) -> ContextManager[None]:
"""Temporarily suppress errors about undefined names."""
return qcore.override(self, "should_suppress_undefined_names", True)

def is_being_evaluted(self, obj: object) -> bool:
return id(obj) in self._being_evaluated

@contextlib.contextmanager
def add_evaluation(self, obj: object) -> Generator[None, None, None]:
"""Temporarily add an object to the set of objects being evaluated.
This is used to prevent infinite recursion when evaluating forward references.
"""
obj_id = id(obj)
self._being_evaluated.add(obj_id)
try:
yield
finally:
self._being_evaluated.remove(obj_id)

def show_error(
self,
message: str,
Expand Down Expand Up @@ -508,17 +528,15 @@ def _type_from_runtime(
elif is_instance_of_typing_name(val, "_ForwardRef") or is_instance_of_typing_name(
val, "ForwardRef"
):
# This has issues because the forward ref may be defined in a different file, in
# which case we don't know which names are valid in it.
with ctx.suppress_undefined_names():
try:
code = ast.parse(val.__forward_arg__)
except SyntaxError:
ctx.show_error(
f"Syntax error in forward reference: {val.__forward_arg__}"
if ctx.is_being_evaluted(val):
return AnyValue(AnySource.inference)
with ctx.add_evaluation(val):
# This is necessary because the forward ref may be defined in a different file, in
# which case we don't know which names are valid in it.
with ctx.suppress_undefined_names():
return _eval_forward_ref(
val.__forward_arg__, ctx, is_typeddict=is_typeddict
)
return AnyValue(AnySource.error)
return _type_from_ast(code.body[0], ctx, is_typeddict=is_typeddict)
elif val is Ellipsis:
# valid in Callable[..., ]
return AnyValue(AnySource.explicit)
Expand Down
5 changes: 4 additions & 1 deletion pyanalyze/stubs/_pyanalyze_tests-stubs/recursion.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
from typing import AnyStr, ContextManager
from typing import AnyStr, ContextManager, Dict, Union
from typing_extensions import TypeAlias

class _ScandirIterator(ContextManager[_ScandirIterator[AnyStr]]):
def close(self) -> None: ...

StrJson: TypeAlias = Union[str, Dict[str, StrJson]]
3 changes: 2 additions & 1 deletion pyanalyze/typeshed.py
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,8 @@ def _value_from_info(
self, info: typeshed_client.resolver.ResolvedName, module: str
) -> Value:
# This guard against infinite recursion if a type refers to itself
# (real-world example: os._ScandirIterator).
# (real-world example: os._ScandirIterator). Needs to change in
# order to support recursive types.
if info in self._active_infos:
return AnyValue(AnySource.inference)
self._active_infos.append(info)
Expand Down

0 comments on commit a6cc64f

Please sign in to comment.