Skip to content

Commit 5fcca77

Browse files
authored
Process superclass methods before subclass methods in semanal (#18723)
Fixes #7162 See also discussion in #18674 for another situation when this causes problems (deferrals). In general this problem is probably quite rare, but it bugs me, so I decided to go ahead with a simple and explicit (even though a bit ugly) solution.
1 parent e93f06c commit 5fcca77

File tree

3 files changed

+76
-12
lines changed

3 files changed

+76
-12
lines changed

mypy/semanal_main.py

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626

2727
from __future__ import annotations
2828

29+
from collections.abc import Iterator
2930
from contextlib import nullcontext
31+
from itertools import groupby
3032
from typing import TYPE_CHECKING, Callable, Final, Optional, Union
3133
from typing_extensions import TypeAlias as _TypeAlias
3234

@@ -232,26 +234,66 @@ def process_top_levels(graph: Graph, scc: list[str], patches: Patches) -> None:
232234
final_iteration = not any_progress
233235

234236

237+
def order_by_subclassing(targets: list[FullTargetInfo]) -> Iterator[FullTargetInfo]:
238+
"""Make sure that superclass methods are always processed before subclass methods.
239+
240+
This algorithm is not very optimal, but it is simple and should work well for lists
241+
that are already almost correctly ordered.
242+
"""
243+
244+
# First, group the targets by their TypeInfo (since targets are sorted by line,
245+
# we know that each TypeInfo will appear as group key only once).
246+
grouped = [(k, list(g)) for k, g in groupby(targets, key=lambda x: x[3])]
247+
remaining_infos = {info for info, _ in grouped if info is not None}
248+
249+
next_group = 0
250+
while grouped:
251+
if next_group >= len(grouped):
252+
# This should never happen, if there is an MRO cycle, it should be reported
253+
# and fixed during top-level processing.
254+
raise ValueError("Cannot order method targets by MRO")
255+
next_info, group = grouped[next_group]
256+
if next_info is None:
257+
# Trivial case, not methods but functions, process them straight away.
258+
yield from group
259+
grouped.pop(next_group)
260+
continue
261+
if any(parent in remaining_infos for parent in next_info.mro[1:]):
262+
# We cannot process this method group yet, try a next one.
263+
next_group += 1
264+
continue
265+
yield from group
266+
grouped.pop(next_group)
267+
remaining_infos.discard(next_info)
268+
# Each time after processing a method group we should retry from start,
269+
# since there may be some groups that are not blocked on parents anymore.
270+
next_group = 0
271+
272+
235273
def process_functions(graph: Graph, scc: list[str], patches: Patches) -> None:
236274
# Process functions.
275+
all_targets = []
237276
for module in scc:
238277
tree = graph[module].tree
239278
assert tree is not None
240-
analyzer = graph[module].manager.semantic_analyzer
241279
# In principle, functions can be processed in arbitrary order,
242280
# but _methods_ must be processed in the order they are defined,
243281
# because some features (most notably partial types) depend on
244282
# order of definitions on self.
245283
#
246284
# There can be multiple generated methods per line. Use target
247-
# name as the second sort key to get a repeatable sort order on
248-
# Python 3.5, which doesn't preserve dictionary order.
285+
# name as the second sort key to get a repeatable sort order.
249286
targets = sorted(get_all_leaf_targets(tree), key=lambda x: (x[1].line, x[0]))
250-
for target, node, active_type in targets:
251-
assert isinstance(node, (FuncDef, OverloadedFuncDef, Decorator))
252-
process_top_level_function(
253-
analyzer, graph[module], module, target, node, active_type, patches
254-
)
287+
all_targets.extend(
288+
[(module, target, node, active_type) for target, node, active_type in targets]
289+
)
290+
291+
for module, target, node, active_type in order_by_subclassing(all_targets):
292+
analyzer = graph[module].manager.semantic_analyzer
293+
assert isinstance(node, (FuncDef, OverloadedFuncDef, Decorator))
294+
process_top_level_function(
295+
analyzer, graph[module], module, target, node, active_type, patches
296+
)
255297

256298

257299
def process_top_level_function(
@@ -308,6 +350,11 @@ def process_top_level_function(
308350
str, Union[MypyFile, FuncDef, OverloadedFuncDef, Decorator], Optional[TypeInfo]
309351
]
310352

353+
# Same as above but includes module as first item.
354+
FullTargetInfo: _TypeAlias = tuple[
355+
str, str, Union[MypyFile, FuncDef, OverloadedFuncDef, Decorator], Optional[TypeInfo]
356+
]
357+
311358

312359
def get_all_leaf_targets(file: MypyFile) -> list[TargetInfo]:
313360
"""Return all leaf targets in a symbol table (module-level and methods)."""

test-data/unit/check-classes.test

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7007,11 +7007,10 @@ class C:
70077007
[case testAttributeDefOrder2]
70087008
class D(C):
70097009
def g(self) -> None:
7010-
self.x = ''
7010+
self.x = '' # E: Incompatible types in assignment (expression has type "str", variable has type "int")
70117011

70127012
def f(self) -> None:
7013-
# https://github.com/python/mypy/issues/7162
7014-
reveal_type(self.x) # N: Revealed type is "builtins.str"
7013+
reveal_type(self.x) # N: Revealed type is "builtins.int"
70157014

70167015

70177016
class C:
@@ -7025,7 +7024,7 @@ class E(C):
70257024
def f(self) -> None:
70267025
reveal_type(self.x) # N: Revealed type is "builtins.int"
70277026

7028-
[targets __main__, __main__, __main__.D.g, __main__.D.f, __main__.C.__init__, __main__.E.g, __main__.E.f]
7027+
[targets __main__, __main__, __main__.C.__init__, __main__.D.g, __main__.D.f, __main__.E.g, __main__.E.f]
70297028

70307029
[case testNewReturnType1]
70317030
class A:

test-data/unit/check-newsemanal.test

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3256,3 +3256,21 @@ class b:
32563256
x = x[1] # E: Cannot resolve name "x" (possible cyclic definition)
32573257
y = 1[y] # E: Value of type "int" is not indexable \
32583258
# E: Cannot determine type of "y"
3259+
3260+
[case testForwardBaseDeferAttr]
3261+
from typing import Optional, Callable, TypeVar
3262+
3263+
class C(B):
3264+
def a(self) -> None:
3265+
reveal_type(self._foo) # N: Revealed type is "Union[builtins.int, None]"
3266+
self._foo = defer()
3267+
3268+
class B:
3269+
def __init__(self) -> None:
3270+
self._foo: Optional[int] = None
3271+
3272+
T = TypeVar("T")
3273+
def deco(fn: Callable[[], T]) -> Callable[[], T]: ...
3274+
3275+
@deco
3276+
def defer() -> int: ...

0 commit comments

Comments
 (0)