From 24ea7eb55b098fc060a5ffa8c4754c82523bb5c0 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Sat, 22 Feb 2025 21:31:56 +0000 Subject: [PATCH 1/3] Process superclass methods before subclass methods in semanal --- mypy/semanal_main.py | 44 +++++++++++++++++++++++----- test-data/unit/check-classes.test | 7 ++--- test-data/unit/check-newsemanal.test | 18 ++++++++++++ 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index ded2a9412168..1fc8473eeebd 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -27,6 +27,7 @@ from __future__ import annotations from contextlib import nullcontext +from functools import cmp_to_key from typing import TYPE_CHECKING, Callable, Final, Optional, Union from typing_extensions import TypeAlias as _TypeAlias @@ -232,26 +233,48 @@ def process_top_levels(graph: Graph, scc: list[str], patches: Patches) -> None: final_iteration = not any_progress +def method_order_by_subclassing(left: FullTargetInfo, right: FullTargetInfo) -> int: + _, _, _, left_info = left + _, _, _, right_info = right + if left_info is None or right_info is None: + return 0 + if left_info is right_info: + return 0 + if left_info in right_info.mro: + return -1 + if right_info in left_info.mro: + return 1 + return 0 + + def process_functions(graph: Graph, scc: list[str], patches: Patches) -> None: # Process functions. + all_targets = [] for module in scc: tree = graph[module].tree assert tree is not None - analyzer = graph[module].manager.semantic_analyzer # In principle, functions can be processed in arbitrary order, # but _methods_ must be processed in the order they are defined, # because some features (most notably partial types) depend on # order of definitions on self. # # There can be multiple generated methods per line. Use target - # name as the second sort key to get a repeatable sort order on - # Python 3.5, which doesn't preserve dictionary order. + # name as the second sort key to get a repeatable sort order. targets = sorted(get_all_leaf_targets(tree), key=lambda x: (x[1].line, x[0])) - for target, node, active_type in targets: - assert isinstance(node, (FuncDef, OverloadedFuncDef, Decorator)) - process_top_level_function( - analyzer, graph[module], module, target, node, active_type, patches - ) + all_targets.extend( + [(module, target, node, active_type) for target, node, active_type in targets] + ) + + # Additionally, we process superclass methods before subclass methods. Here we rely + # on stability of Python sort and just do a separate sort. + for module, target, node, active_type in sorted( + all_targets, key=cmp_to_key(method_order_by_subclassing) + ): + analyzer = graph[module].manager.semantic_analyzer + assert isinstance(node, (FuncDef, OverloadedFuncDef, Decorator)) + process_top_level_function( + analyzer, graph[module], module, target, node, active_type, patches + ) def process_top_level_function( @@ -308,6 +331,11 @@ def process_top_level_function( str, Union[MypyFile, FuncDef, OverloadedFuncDef, Decorator], Optional[TypeInfo] ] +# Same as above but includes module as first item. +FullTargetInfo: _TypeAlias = tuple[ + str, str, Union[MypyFile, FuncDef, OverloadedFuncDef, Decorator], Optional[TypeInfo] +] + def get_all_leaf_targets(file: MypyFile) -> list[TargetInfo]: """Return all leaf targets in a symbol table (module-level and methods).""" diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index d48a27dbed03..06a863ad0499 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -7007,11 +7007,10 @@ class C: [case testAttributeDefOrder2] class D(C): def g(self) -> None: - self.x = '' + self.x = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int") def f(self) -> None: - # https://github.com/python/mypy/issues/7162 - reveal_type(self.x) # N: Revealed type is "builtins.str" + reveal_type(self.x) # N: Revealed type is "builtins.int" class C: @@ -7025,7 +7024,7 @@ class E(C): def f(self) -> None: reveal_type(self.x) # N: Revealed type is "builtins.int" -[targets __main__, __main__, __main__.D.g, __main__.D.f, __main__.C.__init__, __main__.E.g, __main__.E.f] +[targets __main__, __main__, __main__.C.__init__, __main__.D.g, __main__.D.f, __main__.E.g, __main__.E.f] [case testNewReturnType1] class A: diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 9250f3cea0a6..b6756abafc49 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -3256,3 +3256,21 @@ class b: x = x[1] # E: Cannot resolve name "x" (possible cyclic definition) y = 1[y] # E: Value of type "int" is not indexable \ # E: Cannot determine type of "y" + +[case testForwardBaseDeferAttr] +from typing import Optional, Callable, TypeVar + +class C(B): + def a(self) -> None: + reveal_type(self._foo) # N: Revealed type is "Union[builtins.int, None]" + self._foo = defer() + +class B: + def __init__(self) -> None: + self._foo: Optional[int] = None + +T = TypeVar("T") +def deco(fn: Callable[[], T]) -> Callable[[], T]: ... + +@deco +def defer() -> int: ... From a068451079a55b9f13b68e25178d24fc7f3b1a86 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 24 Feb 2025 19:48:12 +0000 Subject: [PATCH 2/3] Work around mypyc bug --- mypy/semanal_main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 1fc8473eeebd..16e4bd50ae83 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -234,8 +234,8 @@ def process_top_levels(graph: Graph, scc: list[str], patches: Patches) -> None: def method_order_by_subclassing(left: FullTargetInfo, right: FullTargetInfo) -> int: - _, _, _, left_info = left - _, _, _, right_info = right + left_info = left[3] + right_info = right[3] if left_info is None or right_info is None: return 0 if left_info is right_info: From 3aee57dd9838a70d3bf69782d97ab72d9c291805 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 28 Feb 2025 09:35:13 +0000 Subject: [PATCH 3/3] Use an explicit ordering algorithm --- mypy/semanal_main.py | 55 +++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 16e4bd50ae83..92a1c24b7b4c 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -26,8 +26,9 @@ from __future__ import annotations +from collections.abc import Iterator from contextlib import nullcontext -from functools import cmp_to_key +from itertools import groupby from typing import TYPE_CHECKING, Callable, Final, Optional, Union from typing_extensions import TypeAlias as _TypeAlias @@ -233,18 +234,40 @@ def process_top_levels(graph: Graph, scc: list[str], patches: Patches) -> None: final_iteration = not any_progress -def method_order_by_subclassing(left: FullTargetInfo, right: FullTargetInfo) -> int: - left_info = left[3] - right_info = right[3] - if left_info is None or right_info is None: - return 0 - if left_info is right_info: - return 0 - if left_info in right_info.mro: - return -1 - if right_info in left_info.mro: - return 1 - return 0 +def order_by_subclassing(targets: list[FullTargetInfo]) -> Iterator[FullTargetInfo]: + """Make sure that superclass methods are always processed before subclass methods. + + This algorithm is not very optimal, but it is simple and should work well for lists + that are already almost correctly ordered. + """ + + # First, group the targets by their TypeInfo (since targets are sorted by line, + # we know that each TypeInfo will appear as group key only once). + grouped = [(k, list(g)) for k, g in groupby(targets, key=lambda x: x[3])] + remaining_infos = {info for info, _ in grouped if info is not None} + + next_group = 0 + while grouped: + if next_group >= len(grouped): + # This should never happen, if there is an MRO cycle, it should be reported + # and fixed during top-level processing. + raise ValueError("Cannot order method targets by MRO") + next_info, group = grouped[next_group] + if next_info is None: + # Trivial case, not methods but functions, process them straight away. + yield from group + grouped.pop(next_group) + continue + if any(parent in remaining_infos for parent in next_info.mro[1:]): + # We cannot process this method group yet, try a next one. + next_group += 1 + continue + yield from group + grouped.pop(next_group) + remaining_infos.discard(next_info) + # Each time after processing a method group we should retry from start, + # since there may be some groups that are not blocked on parents anymore. + next_group = 0 def process_functions(graph: Graph, scc: list[str], patches: Patches) -> None: @@ -265,11 +288,7 @@ def process_functions(graph: Graph, scc: list[str], patches: Patches) -> None: [(module, target, node, active_type) for target, node, active_type in targets] ) - # Additionally, we process superclass methods before subclass methods. Here we rely - # on stability of Python sort and just do a separate sort. - for module, target, node, active_type in sorted( - all_targets, key=cmp_to_key(method_order_by_subclassing) - ): + for module, target, node, active_type in order_by_subclassing(all_targets): analyzer = graph[module].manager.semantic_analyzer assert isinstance(node, (FuncDef, OverloadedFuncDef, Decorator)) process_top_level_function(