Skip to content

use is_same_type when resolving decorators in suggestions #19319

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
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

asottile
Copy link
Contributor

@asottile
Copy link
Contributor Author

this does seem to have a strange side-effect that I can't quite wrap my head around

using this sample code:

from __future__ import annotations

from typing import Callable, Protocol, TypeVar, Any
from typing_extensions import ParamSpec, TypeAlias

def dec(f: Callable[..., Any]) -> Callable[..., Any]:
    return f

@dec
def f(x, y):
    return x + y

f(1, 2)

before this produced "Object t.f is a decorator we can't handle"

but now it produces:

(int, Any) -> Any

(a correct decorator allows it to infer (int, int) -> int but I am quite confused how it figured out one of the parameters but not both and then not the return value 🤔)

@JelleZijlstra
Copy link
Member

Is the code sample in your last message correct? I don't get that error in the playground when I try the code in your message, but maybe you meant something else. You imported ParamSpec but didn't use it.

@asottile
Copy link
Contributor Author

Is the code sample in your last message correct? I don't get that error in the playground when I try the code in your message, but maybe you meant something else. You imported ParamSpec but didn't use it.

yeah the imports are left over from making the testcase I added in this PR. weird that you don't get the same results though 🤔

@asottile
Copy link
Contributor Author

oh to be clear the code doesn't produce an error -- I'm specifically talking about dmypy suggest

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@sterliakov
Copy link
Collaborator

Hm, this seems to be the reason why dmypy suggest didn't allow decorators that don't preserve signature explicitly. In your example the argument types for the call site are not mapped correctly: the first argument is Literal[1] | Literal[2] and the second argument has no type information at all, as if the signature was def fn(*args) - consistent with Callable[..., T] interpretation as def (*args: Any, **kwargs: Any) -> T. This mapping comes from straight from FunctionContext, so that's a limitation of how mypy treats calls of Callable[..., T] internally.

Dumping the call sites, we have this single entry there:

Callsite(
    path='b.py', 
    line=12, 
    arg_kinds=[[<ArgKind.ARG_POS: 0>, <ArgKind.ARG_POS: 0>], []], 
    callee_arg_names=[None, None], 
    arg_names=[[None, None], []], 
    arg_types=[[Literal[1]?, Literal[2]?], []]
)

Where i-th element of a 2D list contains types mapped to i-th formal argument of the function. f at that moment is def (*Any, **Any) for the type checker but (x: Any, y: Any) for the suggestion engine, and this discrepancy manifests in quite weird results. It's possible to update your example to make it exhibit even weirder behavior - if you do f(1, ''), for example, the inferred signature would be (x: object, y: Any) -> Any. And this will crash altogether with an IndexError:

from __future__ import annotations

from typing import Callable, Any

def dec(f: Callable[..., Any]) -> Callable[..., Any]:
    return f

@dec
def f(x, y, z):
    return x + y

f('', 0, 1)

It might be better to disallow such decorators again and revert/update #19072... But this change (using is_same_type) is certainly good on its own.

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

Successfully merging this pull request may close these issues.

3 participants