Skip to content

1.11 regression: "generic" mapping failing with confusing error message #17566

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

Open
asottile opened this issue Jul 23, 2024 · 4 comments
Open
Labels
bug mypy got something wrong

Comments

@asottile
Copy link
Contributor

Bug Report

I've boiled down a minimal example -- this pattern is used by pyupgrade (and reorder-python-imports) to register ast functions and has been working with mypy since version 0.710 -- but was broken with the last update sadly

To Reproduce

this is a simplified version of pyupgrade/_data.py -- FUNCS acts as a mapping from ast types to their callback functions

import ast
import collections
from typing import Callable, Protocol, TypeVar

AST_T = TypeVar('AST_T', bound=ast.AST)
ASTFunc = Callable[[AST_T], None]

FUNCS = collections.defaultdict(list)

def register(tp: type[AST_T]) -> Callable[[ASTFunc[AST_T]], ASTFunc[AST_T]]:
    def register_decorator(func: ASTFunc[AST_T]) -> ASTFunc[AST_T]:
        FUNCS[tp].append(func)
        return func
    return register_decorator


class ASTCallbackMapping(Protocol):
    def __getitem__(self, tp: type[AST_T]) -> list[ASTFunc[AST_T]]: ...

def visit(funcs: ASTCallbackMapping) -> None: ...


def f() -> None:
    visit(FUNCS)

Expected Behavior

the behaviour pre-mypy-1.11:

(no errors)

Actual Behavior

$ mypy t.py 
t.py:24: error: Argument 1 to "visit" has incompatible type "defaultdict[type[AST_T], list[Callable[[AST_T], None]]]"; expected "ASTCallbackMapping"  [arg-type]
t.py:24: note: Following member(s) of "defaultdict[type[AST_T], list[Callable[[AST_T], None]]]" have conflicts:
t.py:24: note:     Expected:
t.py:24: note:         def [AST_T: AST] __getitem__(self, type[AST_T], /) -> list[Callable[[AST_T], None]]
t.py:24: note:     Got:
t.py:24: note:         def __getitem__(self, type[AST_T], /) -> list[Callable[[AST_T], None]]
Found 1 error in 1 file (checked 1 source file)

the error message seems suspicious because I think those two are the same ?

Your Environment

  • Mypy version used: 1.11.0
  • Mypy command-line flags: none
  • Mypy configuration options from mypy.ini (and other config files): none
  • Python version used: 3.10.12 -- though really anything modern
@hauntsaninja
Copy link
Collaborator

Bisects to #17311 , looks like some weird partial type situation where mypy gets a partial type from the FUNCS[tp].append but the type var scope there is different than the scope in ASTCallbackMapping.__getitem__.

Definitely should at least have a better error message, a possible workaround is FUNCS = cast(ASTCallbackMapping, collections.defaultdict(list))

@erictraut
Copy link

Another workaround is to make AST_T a class-scoped type variable in the protocol definition:

class ASTCallbackMapping(Protocol[AST_T]):
    def __getitem__(self, tp: type[AST_T]) -> list[ASTFunc[AST_T]]: ...

@asottile
Copy link
Contributor Author

making the Protocol generic isn't right either because then visit can't be typed properly -- the actual __getitem__ is what's generic, not the class itself

asottile added a commit to asottile/pyupgrade that referenced this issue Jul 28, 2024
asottile added a commit to asottile/add-trailing-comma that referenced this issue Jul 28, 2024
@sterliakov
Copy link
Collaborator

Sorry if I'm missing something, but isn't this error expected?

The protocol defines a "mapping from given type AST_T to a list of AST_T handlers corresponding to it". defaultdict does not guarantee that - there is no item-level type mapping. There's no way to specify the type of FUNCS that you want explicitly: defaultdict and dict are generic classes, their __getitem__ methods aren't generic.

The problem:

def visit(funcs: ASTCallbackMapping) -> None:
    # Adding some exploding body
    for callback in funcs[ast.And]:
        callback(ast.And())

def handle_or(p: ast.Or) -> None:
    assert isinstance(p, ast.Or)

store = collections.defaultdict(list)
store[ast.And] = [handle_or]

visit(store)

Now mypy refuses to visit(store) as in your post, and also fails to infer store type wide enough.

There's nothing in static types of your original snippet preventing FUNCS[ast.And] = [handle_or], actually, and "it's only done by that function" is impossible to explain to mypy.

Note that you can't do

FUNCS: collections.defaultdict[type[AST_T], list[Callable[[AST_T], None]]] = ...

because AST_T is unbound there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

4 participants