Skip to content

Process superclass methods before subclass methods in semanal #18723

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

Merged
merged 4 commits into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 55 additions & 8 deletions mypy/semanal_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@

from __future__ import annotations

from collections.abc import Iterator
from contextlib import nullcontext
from itertools import groupby
from typing import TYPE_CHECKING, Callable, Final, Optional, Union
from typing_extensions import TypeAlias as _TypeAlias

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


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:
# 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]
)

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(
analyzer, graph[module], module, target, node, active_type, patches
)


def process_top_level_function(
Expand Down Expand Up @@ -308,6 +350,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)."""
Expand Down
7 changes: 3 additions & 4 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
18 changes: 18 additions & 0 deletions test-data/unit/check-newsemanal.test
Original file line number Diff line number Diff line change
Expand Up @@ -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: ...