[ty] Avoid ever-growing default types

[sharkdp]

Summary

We currently panic in the seemingly rare case where the type of a default value of a parameter depends on the callable itself:

class C:
    def f(self: C):
        self.x = lambda a=self.x: a

Types of default values are only used for display reasons, and it's unclear if we even want to track them (or if we should rather track the actual value). So it didn't seem to me that we should spend a lot of effort (and runtime) trying to achieve a theoretically correct type here (which would be infinite).

Instead, we simply replace nested default types with Unknown, i.e. only if the type of the default value is a callable itself.

closes astral-sh/ty#1402

Test Plan

Regression tests

[github-actions]

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

[github-actions]

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

[AlexWaygood]

Types of default values are only used for display reasons,

We do also check whether the type of the default value is assignable to the type of the annotation (if there is an annotation), right?

It looks like we still emit an error for something like this on your branch, but it might be worth adding a test for it:

class A:
    def __init__(self: "A"):
        def f(a: int = self.f): ...
        self.f = f

[sharkdp]

We do also check whether the type of the default value is assignable to the type of the annotation (if there is an annotation), right?

Yes, correct. Type inference for the actual default value did not change. And we still perform that check. Only the type inside the generated signature changes.

[sharkdp]

but it might be worth adding a test for it

Done

[MichaReiser]

We currently panic in the seemingly rare case where the type of a default value of a parameter depends on the callable itself:

Is this a too-many-iterations panic?

[sharkdp]

We currently panic in the seemingly rare case where the type of a default value of a parameter depends on the callable itself:

Is this a too-many-iterations panic?

Yes

[MichaReiser]

I don't have a good sense for whether there's a better solution but i agree with the sentiment, that it's probably not worth spending too much time on.

I was worried that it could regress signature completions in the LSP but this isn't the case

[AlexWaygood]

I think the better solution (which David gestures at in his PR description) would be not to store the type of the parameter default as part of the function signature at all. I think our current UX here is somewhat bad: if you have a function like this in your source code:

def f(x: bool = True) -> None: ...

Why do we display the signature on hover as (x: bool = Literal[True) -> None? That honestly just seems incorrect -- it should just be (x: bool = True) -> None, as it is in the source code (that's what pyright does -- try hovering over print in VSCode). This implies that rather than storing the default value's type in the signature, we should ideally just grab the slice of the source code that the parameter default occupies and store it as a Box<str> in the function's signature.

This PR doesn't regress UX, so I'm okay with it as a temporary measure. But it does also add some additional complexity to our codebase (by introducing a new TypeMapping variant), and I do wonder how hard it would be to just implement the "better solution" (which feels like it would also solve a significant UX painpoint).

[sharkdp]

This implies that rather than storing the default value's type in the signature, we should ideally just grab the slice of the source code that the parameter default occupies and store it as a Box<str> in the function's signature.

That's not ideal either, I think. Consider:

def outer(x: int):
    def inner(y: int=x):
        pass

Do you want to see a default value of x there? I think what we could do instead would be to see if the default type is a Literal[…] type (that could be generalized to single-value types probably). If so, we turn it into a value and show param = True instead of param = Literal[True]. If not, we simply show param = .... What do you think?

I can attempt to implement that instead of this PR.

[AlexWaygood]

Do you want to see a default value of x there?

Not sure I understand the example -- none of the parameters have default values in this snippet? Did you mean y=x rather than y: x?

[sharkdp]

Not sure I understand the example -- none of the parameters have default values in this snippet? Did you mean y=x rather than y: x?

yes, fixed

[AlexWaygood]

I think what we could do instead would be to see if the default type is a Literal[…] type (that could be generalized to single-value types probably). If so, we turn it into a value and show param = True instead of param = Literal[True]. If not, we simply show param = .... What do you think?

I don't think this will do a great job for things like tuple.index, where sys.maxsize (a value that varies depending on the specific build of Python you're running) is used as the parameter default:

ruff/crates/ty_vendored/vendor/typeshed/stdlib/builtins.pyi

Lines 2739 to 2743 in e1cada1

	def index(self, value: Any, start: SupportsIndex = 0, stop: SupportsIndex = sys.maxsize, /) -> int:
	"""Return first index of value.
	Raises ValueError if the value is not present.
	"""

And what about parameter defaults that typeshed faithfully gives to us in octal, so that they will be nicely readable for IDE users?

ruff/crates/ty_vendored/vendor/typeshed/stdlib/dbm/dumb.pyi

Line 57 in e1cada1

def open(file: StrOrBytesPath, flag: str = "c", mode: int = 0o666) -> _Database:

While I don't think it ever occurs in typeshed, I think things like x=(foo if some condition else bar) are also not unusual in user code, and it's probably useful to have the full condition printed in the on-hover signature so that you can see how the default value changes under certain conditions.

[AlexWaygood]

A compromise that might work quite well: if the function comes from a stub file, just grab a string slice from the source code (typeshed, for example, actually has pretty strict lint rules to stop us from using arbitrary expressions in default values; we just use =... for anything complicated at runtime). If it's not a stub file, we can implement your Literal heuristic, to avoid us possibly printing strange default values that reference global variables the user has no context on.

[MichaReiser]

The issue isn't just about values the user doesn't have control over. It also means that comments, or line breaks within the default value are preserved.

I'm not sure if this is worth prioritizing right now. It seems there are enough details at play that are worth considering in a separate PR (and later)

[AlexWaygood]

I'm not sure if this is worth prioritizing right now. It seems there are enough details at play that are worth considering in a separate PR (and later)

Yes, it does seem like pyright/pylance is doing something quite sophisticated here. For this function:

def f(x=[
    1,
    2,
    # some comment,
    4
]): ...

Pyright/pylance gives this signature when I hover over it:

Screenshot

But pyright/pylance doesn't do a great job for the sys.maxsize or octal defaults that I showed from typeshed above (the sys.maxsize defaults just become ..., and the 0o666 default becomes 438), so there's arguably room for improvement over what pyright/pylance does too.

[sharkdp]

Seeing this discussion, I feel like this change should be done separately, and probably at a later time. I'd love to work on this, but it's honestly not high priority. And it seems like there will be some things to decide. So I'm going to merge this (because solving that panic is a high priority), even if I fully agree with Alex's comment above. It looks to me like it'll be easy to rip this out once we switch to the value-based representation. I'll write down a todo task for me and/or create a ticket.

[AlexWaygood]

Thank you!