From f3f285ebaa34a94e416af86f91859d5f136c7219 Mon Sep 17 00:00:00 2001 From: Stephen Morton Date: Thu, 24 Oct 2024 23:54:50 -0700 Subject: [PATCH 1/6] Defer type analysis of integer Literal when builtins.int is slow to resolve --- mypy/typeanal.py | 66 +++++++++++++++++-------------- test-data/unit/check-literal.test | 37 +++++++++++++++++ 2 files changed, 74 insertions(+), 29 deletions(-) diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 0a6b7689136e..3f3dbb7a41dd 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -297,40 +297,43 @@ def not_declared_in_type_params(self, tvar_name: str) -> bool: and tvar_name not in self.alias_type_params_names ) + def handle_placeholder_node(self, node: PlaceholderNode, t: UnboundType) -> Type: + if node.becomes_typeinfo: + # Reference to placeholder type. + if self.api.final_iteration: + self.cannot_resolve_type(t) + return AnyType(TypeOfAny.from_error) + elif self.allow_placeholder: + self.api.defer() + else: + self.api.record_incomplete_ref() + # Always allow ParamSpec for placeholders, if they are actually not valid, + # they will be reported later, after we resolve placeholders. + return PlaceholderType( + node.fullname, + self.anal_array( + t.args, + allow_param_spec=True, + allow_param_spec_literals=True, + allow_unpack=True, + ), + t.line, + ) + else: + if self.api.final_iteration: + self.cannot_resolve_type(t) + return AnyType(TypeOfAny.from_error) + else: + # Reference to an unknown placeholder node. + self.api.record_incomplete_ref() + return AnyType(TypeOfAny.special_form) + def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) -> Type: sym = self.lookup_qualified(t.name, t) if sym is not None: node = sym.node if isinstance(node, PlaceholderNode): - if node.becomes_typeinfo: - # Reference to placeholder type. - if self.api.final_iteration: - self.cannot_resolve_type(t) - return AnyType(TypeOfAny.from_error) - elif self.allow_placeholder: - self.api.defer() - else: - self.api.record_incomplete_ref() - # Always allow ParamSpec for placeholders, if they are actually not valid, - # they will be reported later, after we resolve placeholders. - return PlaceholderType( - node.fullname, - self.anal_array( - t.args, - allow_param_spec=True, - allow_param_spec_literals=True, - allow_unpack=True, - ), - t.line, - ) - else: - if self.api.final_iteration: - self.cannot_resolve_type(t) - return AnyType(TypeOfAny.from_error) - else: - # Reference to an unknown placeholder node. - self.api.record_incomplete_ref() - return AnyType(TypeOfAny.special_form) + return self.handle_placeholder_node(node, t) if node is None: self.fail(f"Internal error (node is None, kind={sym.kind})", t) return AnyType(TypeOfAny.special_form) @@ -1701,6 +1704,11 @@ def analyze_literal_param(self, idx: int, arg: Type, ctx: Context) -> list[Type] # helpful, but it generally won't make sense in the context of a Literal[...]. return None + # Make sure the literal's class is ready + sym = self.lookup_fully_qualified(arg.base_type_name) + if isinstance(sym.node, PlaceholderNode): + return [self.handle_placeholder_node(sym.node, UnboundType(arg.base_type_name))] + # Remap bytes and unicode into the appropriate type for the correct Python version fallback = self.named_type(arg.base_type_name) assert isinstance(fallback, Instance) diff --git a/test-data/unit/check-literal.test b/test-data/unit/check-literal.test index 2f94b5df0f83..8c9112191f28 100644 --- a/test-data/unit/check-literal.test +++ b/test-data/unit/check-literal.test @@ -2984,3 +2984,40 @@ class C(Base): reveal_type(sep) # N: Revealed type is "Union[Literal['a'], Literal['b']]" return super().feed_data(sep) [builtins fixtures/tuple.pyi] + + +-- This is constructed so that when mypy performs first pass +-- type analysis on _LiteralInteger, builtins.int is still a +-- placeholder, and the analysis must defer to avoid an error. +[case testDeferIntLiteral] +[file typing.py] +import abc + +[file abc.py] +import collections.abc + +[file collections/__init__.py] + +[file collections/abc.py] +from _collections_abc import * + +[file _collections_abc.py] + +[file typing_extensions.py] +Literal: object + +[file numbers.py] +class Number: ... + +[file builtins.py] +import numbers +import typing +from typing_extensions import Literal + +_LiteralInteger: Literal[0] + +class int(numbers.Number): ... +class str: ... +class list: ... +class dict: ... +class ellipsis: ... From 80b1cd9daff1b9d42ce8bb6bbfebf46cb444ae03 Mon Sep 17 00:00:00 2001 From: Stephen Morton Date: Fri, 25 Oct 2024 18:17:04 -0700 Subject: [PATCH 2/6] Handle more cases where builtins.int isn't ready yet --- mypy/semanal.py | 12 ++++-- mypy/typeanal.py | 72 ++++++++++++++++--------------- test-data/unit/check-literal.test | 36 ++++++++++++++++ 3 files changed, 81 insertions(+), 39 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index e182322cfbe6..4400604931d7 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3181,7 +3181,7 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: else: s.rvalue.accept(self) - if self.found_incomplete_ref(tag) or self.should_wait_rhs(s.rvalue): + if self.found_incomplete_ref(tag) or self.should_wait_rhs(s.rvalue, s): # Initializer couldn't be fully analyzed. Defer the current node and give up. # Make sure that if we skip the definition of some local names, they can't be # added later in this scope, since an earlier definition should take precedence. @@ -3280,7 +3280,7 @@ def analyze_identity_global_assignment(self, s: AssignmentStmt) -> bool: node.fullname = sym.node.fullname return True - def should_wait_rhs(self, rv: Expression) -> bool: + def should_wait_rhs(self, rv: Expression, s: AssignmentStmt) -> bool: """Can we already classify this r.h.s. of an assignment or should we wait? This returns True if we don't have enough information to decide whether @@ -3291,6 +3291,10 @@ def should_wait_rhs(self, rv: Expression) -> bool: if self.final_iteration: # No chance, nothing has changed. return False + if isinstance(s.type, UnboundType): + lookup = self.lookup_qualified(s.type.name, s, suppress_errors=True) + if lookup and isinstance(lookup.node, PlaceholderNode): + return True if isinstance(rv, NameExpr): n = self.lookup(rv.name, rv) if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: @@ -3302,11 +3306,11 @@ def should_wait_rhs(self, rv: Expression) -> bool: if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: return True elif isinstance(rv, IndexExpr) and isinstance(rv.base, RefExpr): - return self.should_wait_rhs(rv.base) + return self.should_wait_rhs(rv.base, s) elif isinstance(rv, CallExpr) and isinstance(rv.callee, RefExpr): # This is only relevant for builtin SCC where things like 'TypeVar' # may be not ready. - return self.should_wait_rhs(rv.callee) + return self.should_wait_rhs(rv.callee, s) return False def can_be_type_alias(self, rv: Expression, allow_none: bool = False) -> bool: diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 3f3dbb7a41dd..80d9e12b2b3c 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -297,43 +297,40 @@ def not_declared_in_type_params(self, tvar_name: str) -> bool: and tvar_name not in self.alias_type_params_names ) - def handle_placeholder_node(self, node: PlaceholderNode, t: UnboundType) -> Type: - if node.becomes_typeinfo: - # Reference to placeholder type. - if self.api.final_iteration: - self.cannot_resolve_type(t) - return AnyType(TypeOfAny.from_error) - elif self.allow_placeholder: - self.api.defer() - else: - self.api.record_incomplete_ref() - # Always allow ParamSpec for placeholders, if they are actually not valid, - # they will be reported later, after we resolve placeholders. - return PlaceholderType( - node.fullname, - self.anal_array( - t.args, - allow_param_spec=True, - allow_param_spec_literals=True, - allow_unpack=True, - ), - t.line, - ) - else: - if self.api.final_iteration: - self.cannot_resolve_type(t) - return AnyType(TypeOfAny.from_error) - else: - # Reference to an unknown placeholder node. - self.api.record_incomplete_ref() - return AnyType(TypeOfAny.special_form) - def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool) -> Type: sym = self.lookup_qualified(t.name, t) if sym is not None: node = sym.node if isinstance(node, PlaceholderNode): - return self.handle_placeholder_node(node, t) + if node.becomes_typeinfo: + # Reference to placeholder type. + if self.api.final_iteration: + self.cannot_resolve_type(t) + return AnyType(TypeOfAny.from_error) + elif self.allow_placeholder: + self.api.defer() + else: + self.api.record_incomplete_ref() + # Always allow ParamSpec for placeholders, if they are actually not valid, + # they will be reported later, after we resolve placeholders. + return PlaceholderType( + node.fullname, + self.anal_array( + t.args, + allow_param_spec=True, + allow_param_spec_literals=True, + allow_unpack=True, + ), + t.line, + ) + else: + if self.api.final_iteration: + self.cannot_resolve_type(t) + return AnyType(TypeOfAny.from_error) + else: + # Reference to an unknown placeholder node. + self.api.record_incomplete_ref() + return AnyType(TypeOfAny.special_form) if node is None: self.fail(f"Internal error (node is None, kind={sym.kind})", t) return AnyType(TypeOfAny.special_form) @@ -1705,9 +1702,14 @@ def analyze_literal_param(self, idx: int, arg: Type, ctx: Context) -> list[Type] return None # Make sure the literal's class is ready - sym = self.lookup_fully_qualified(arg.base_type_name) - if isinstance(sym.node, PlaceholderNode): - return [self.handle_placeholder_node(sym.node, UnboundType(arg.base_type_name))] + sym = self.api.lookup_fully_qualified_or_none(arg.base_type_name) + if sym is None or isinstance(sym.node, PlaceholderNode): + assert arg.base_type_name.startswith("builtins.") + if self.api.is_incomplete_namespace("builtins"): + self.api.record_incomplete_ref() + else: + self.fail(f'Name "{arg.base_type_name}" is not defined', arg) + return [AnyType(TypeOfAny.special_form)] # Remap bytes and unicode into the appropriate type for the correct Python version fallback = self.named_type(arg.base_type_name) diff --git a/test-data/unit/check-literal.test b/test-data/unit/check-literal.test index 8c9112191f28..44e08f23ebd4 100644 --- a/test-data/unit/check-literal.test +++ b/test-data/unit/check-literal.test @@ -3021,3 +3021,39 @@ class str: ... class list: ... class dict: ... class ellipsis: ... + + +-- In this version, looking up the node for builtins.int +-- during type analysis of _LiteralInteger returns None +[case testDeferIntLiteral2] +[file _collections_abc.pyi] +[file abc.pyi] +[file collections/__init__.pyi] +[file collections/abc.pyi] +[file types.pyi] +class UnionType: ... + +[file typing_extensions.pyi] +from typing import Any +TypeAlias: Any + +[file typing.pyi] +Any = object() +Literal: Any + +[file builtins.pyi] +import _collections_abc +import abc +import collections.abc +import types +from typing import Literal +from typing_extensions import TypeAlias + +class int: ... +class ellipsis: ... +class str: ... +class list: ... +class dict: ... + +_PositiveInteger: TypeAlias = Literal[1] +_LiteralInteger = _PositiveInteger | Literal[0] From 83df83d3752500ab834202534847e8de2b60a88f Mon Sep 17 00:00:00 2001 From: Stephen Morton Date: Sat, 26 Oct 2024 17:50:01 -0700 Subject: [PATCH 3/6] make the new check its own method --- mypy/semanal.py | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 4400604931d7..33e0daabeddc 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3181,7 +3181,11 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: else: s.rvalue.accept(self) - if self.found_incomplete_ref(tag) or self.should_wait_rhs(s.rvalue, s): + if ( + self.found_incomplete_ref(tag) + or self.should_wait_rhs(s.rvalue) + or self.should_wait_lhs(s) + ): # Initializer couldn't be fully analyzed. Defer the current node and give up. # Make sure that if we skip the definition of some local names, they can't be # added later in this scope, since an earlier definition should take precedence. @@ -3280,7 +3284,22 @@ def analyze_identity_global_assignment(self, s: AssignmentStmt) -> bool: node.fullname = sym.node.fullname return True - def should_wait_rhs(self, rv: Expression, s: AssignmentStmt) -> bool: + def should_wait_lhs(self, s: AssignmentStmt) -> bool: + """Is the l.h.s of an assignment ready? + + If the eventual l.h.s. type turns out to be a special form, we need to know that before + we can process the r.h.s. properly. + """ + if self.final_iteration: + # No chance, nothing has changed. + return False + if isinstance(s.type, UnboundType): + lookup = self.lookup_qualified(s.type.name, s, suppress_errors=True) + if lookup and isinstance(lookup.node, PlaceholderNode): + return True + return False + + def should_wait_rhs(self, rv: Expression) -> bool: """Can we already classify this r.h.s. of an assignment or should we wait? This returns True if we don't have enough information to decide whether @@ -3291,10 +3310,6 @@ def should_wait_rhs(self, rv: Expression, s: AssignmentStmt) -> bool: if self.final_iteration: # No chance, nothing has changed. return False - if isinstance(s.type, UnboundType): - lookup = self.lookup_qualified(s.type.name, s, suppress_errors=True) - if lookup and isinstance(lookup.node, PlaceholderNode): - return True if isinstance(rv, NameExpr): n = self.lookup(rv.name, rv) if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: @@ -3306,11 +3321,11 @@ def should_wait_rhs(self, rv: Expression, s: AssignmentStmt) -> bool: if n and isinstance(n.node, PlaceholderNode) and not n.node.becomes_typeinfo: return True elif isinstance(rv, IndexExpr) and isinstance(rv.base, RefExpr): - return self.should_wait_rhs(rv.base, s) + return self.should_wait_rhs(rv.base) elif isinstance(rv, CallExpr) and isinstance(rv.callee, RefExpr): # This is only relevant for builtin SCC where things like 'TypeVar' # may be not ready. - return self.should_wait_rhs(rv.callee, s) + return self.should_wait_rhs(rv.callee) return False def can_be_type_alias(self, rv: Expression, allow_none: bool = False) -> bool: From 168e67afd9b5ca9580df6b63b1a8a2f46a989445 Mon Sep 17 00:00:00 2001 From: Stephen Morton Date: Sat, 26 Oct 2024 17:57:00 -0700 Subject: [PATCH 4/6] fix test accidentally got it into a form that didn't reproduce the crash on master it's important the int is declared after the literals --- test-data/unit/check-literal.test | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test-data/unit/check-literal.test b/test-data/unit/check-literal.test index 44e08f23ebd4..15b207ed5f55 100644 --- a/test-data/unit/check-literal.test +++ b/test-data/unit/check-literal.test @@ -3049,11 +3049,11 @@ import types from typing import Literal from typing_extensions import TypeAlias +_PositiveInteger: TypeAlias = Literal[1] +_LiteralInteger = _PositiveInteger | Literal[0] + class int: ... class ellipsis: ... class str: ... class list: ... class dict: ... - -_PositiveInteger: TypeAlias = Literal[1] -_LiteralInteger = _PositiveInteger | Literal[0] From 3197cd7ca5c1390456a9b332717b67cbdfe52686 Mon Sep 17 00:00:00 2001 From: Stephen Morton Date: Sat, 26 Oct 2024 18:32:19 -0700 Subject: [PATCH 5/6] restrict lhs defer to TypeAlias --- mypy/semanal.py | 7 ++++--- mypy/typeanal.py | 6 +----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 33e0daabeddc..eebeb5de54b8 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3294,9 +3294,10 @@ def should_wait_lhs(self, s: AssignmentStmt) -> bool: # No chance, nothing has changed. return False if isinstance(s.type, UnboundType): - lookup = self.lookup_qualified(s.type.name, s, suppress_errors=True) - if lookup and isinstance(lookup.node, PlaceholderNode): - return True + if s.type.name == "TypeAlias": + lookup = self.lookup_qualified(s.type.name, s, suppress_errors=True) + if lookup and isinstance(lookup.node, PlaceholderNode): + return True return False def should_wait_rhs(self, rv: Expression) -> bool: diff --git a/mypy/typeanal.py b/mypy/typeanal.py index 80d9e12b2b3c..0e94a6299a23 100644 --- a/mypy/typeanal.py +++ b/mypy/typeanal.py @@ -1704,11 +1704,7 @@ def analyze_literal_param(self, idx: int, arg: Type, ctx: Context) -> list[Type] # Make sure the literal's class is ready sym = self.api.lookup_fully_qualified_or_none(arg.base_type_name) if sym is None or isinstance(sym.node, PlaceholderNode): - assert arg.base_type_name.startswith("builtins.") - if self.api.is_incomplete_namespace("builtins"): - self.api.record_incomplete_ref() - else: - self.fail(f'Name "{arg.base_type_name}" is not defined', arg) + self.api.record_incomplete_ref() return [AnyType(TypeOfAny.special_form)] # Remap bytes and unicode into the appropriate type for the correct Python version From d8efa86df7cd1a87a7782da765ff3f0d8486826e Mon Sep 17 00:00:00 2001 From: Stephen Morton Date: Sat, 26 Oct 2024 18:44:50 -0700 Subject: [PATCH 6/6] less hacky way to narrow lhs defers --- mypy/semanal.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index eebeb5de54b8..81d3d069f4cb 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3294,10 +3294,11 @@ def should_wait_lhs(self, s: AssignmentStmt) -> bool: # No chance, nothing has changed. return False if isinstance(s.type, UnboundType): - if s.type.name == "TypeAlias": - lookup = self.lookup_qualified(s.type.name, s, suppress_errors=True) - if lookup and isinstance(lookup.node, PlaceholderNode): - return True + lookup = self.lookup_qualified(s.type.name, s, suppress_errors=True) + if lookup and isinstance(lookup.node, PlaceholderNode): + if isinstance(lookup.node.node, ImportFrom): + if lookup.node.node.id in ("typing", "typing_extensions"): + return True return False def should_wait_rhs(self, rv: Expression) -> bool: