Skip to content
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

Class definition for D depends on itself - unable to figure out types #9391

Closed
ljw1004 opened this issue Nov 5, 2024 · 3 comments
Closed
Labels
as designed Not a bug, working as intended bug Something isn't working

Comments

@ljw1004
Copy link
Contributor

ljw1004 commented Nov 5, 2024

Pyright gets "Unknown" for the return-type of f, but Mypy and Pyre get "D.A".

from __future__ import annotations
from typing import Generic, Type, TypeVar
from abc import ABC

T = TypeVar("T")

class C(Generic[T], ABC):
    A: Type[T]

class D(C["D.A"]): # "class definition for D depends on itself" (pyright), no error (pyre, mypy)
    class A:
        pass

def f(_: Type[C[T]]) -> T: ...

x = f(D)
reveal_type(x)  # Unknown (pyright), D.A (pyre, mypy)

Playgrounds: pyright, mypy, pyre

I also see #4518 "Class definition for "MyClass" depends on itself", but the repros there were non-deterministic and it seemed to be related to evaluation order. That issue was closed because of no clear repro.

I also see https://microsoft.github.io/pyright/#/mypy-comparison?id=circular-references "Because mypy is a multi-pass analyzer, it is able to deal with certain forms of circular references that pyright cannot handle." I don't know if this is an additional case of circularity that pyright can't handle?

My hope is that even if there's a type error in the declaration of D, I'd hope that the return type of f should still be solvable by pyright, so that hover and go-to-def on x.foo() will succeed. In other words some way to still retain useful information about D even in the presence of a type error, maybe even one that we suppress.

(The code in this bug is a minimal repro of an idiom that's widely used in generated code at my company).

@ljw1004 ljw1004 added the bug Something isn't working label Nov 5, 2024
@erictraut
Copy link
Collaborator

This is a form of circular reference that pyright doesn't (and is unlikely to ever) support. Sorry.

@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Nov 5, 2024
@erictraut erictraut added the as designed Not a bug, working as intended label Nov 5, 2024
@erictraut
Copy link
Collaborator

erictraut commented Nov 5, 2024

I'll note that mypy has not yet implemented support for class decorators. It completely ignores them. I think pyre may do the same. If and when they add support for class decorators, they'll probably also reject the above code. The expression "D.A" refers to the post-decorated type, and that can affect the evaluation, but applying the pre-decorated type requires an evaluation of the base class C["D.A"]. That makes this an unresolvable circular dependency.

@ljw1004
Copy link
Contributor Author

ljw1004 commented Nov 5, 2024

I'll note that mypy has not yet implemented support for class decorators. It completely ignores them.

I think that might actually be the best choice for the sake of codenav (gotodef, hover). What I've observed: (1) there are few typesafe ways to annotate a decorator correctly which changes function signature, and I haven't seen any for decorators which add/remove methods in a class. (2) Users usually get decorators wrong. (3) If we ignored decorators entirely, then Go-to-def and Hover would more often give the user what they wanted, i.e. reflecting the eventual runtime behavior of their program.

Here, I picked a random example of a decorator that you wrote #774 (comment)

#!/usr/bin/python3

from typing import Callable, TypeVar
from typing_extensions import ParamSpec

_P = ParamSpec("_P")
_R = TypeVar("_R")


def decorator(f: Callable[_P, _R]) -> Callable[_P, _R]:
    def wrapper(*args: _P.args, **kwargs: _P.kwargs):
        print("wrapper", *args, **kwargs)
        return f(*args, **kwargs)
    return wrapper
    
@decorator
def foo(x1:int, y1:str) -> str:
    return str(x1) + y1

@mistyped_decorator
def bar(x2:int, y2:str) -> str:
    return str(x2) + y2

print(foo(x1=1, y1=""))
print(bar(x2=1, y2=""))

If you cmd+click on the named arguments "x1" or "y1" then pyright doesn't find a definition; if you cmd+click on the named arguments "x2" or "y2" then it does.

So for cases where users get decorators right (like this one), and where they get them wrong, codenav would be better served by assuming the decorator doesn't exist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants