Return type inference

[sharkdp]

It would be great if we could infer return types for functions where it is not annotated

def f(x: int, y: str):
    if x > 10:
        return x
    elif x > 5:
        return y

reveal_type(f(some_int, "a"))  # ideally: int | str | None

It seems like it would simply be a union of all reachable return expressions (and possible None, if the function can implicitly return it — we already have functionality to detect that).

[MichaReiser]

This gets more interesting when parameters aren't annotated, but doing it for functions with annotated parameters is a good start.

I think this is an important feature for a good IDE (and later linter) experience. Without it, Red Knot won't be able to provide many IDE features, such as go-to definitions, completions, on-hover, and useful inlays for any function without an annotated return type. That means, Red Knot won't be very useful on unannotated code.

[sharkdp]

This gets more interesting when parameters aren't annotated

Yes. This is where Pyright's inlining strategy (for small functions) comes into play, for example.

[carljm]

I am skeptical of this feature. It can easily lead to false positives on un-annotated code with inheritance, which can only be resolved by adding annotations. This violates the gradual guarantee, and potentially makes red-knot less useful on un-annotated code, because of false positives.

In the scenario below, what type should red-knot infer for the return types of A.method and B.method, and should it issue any error for Liskov violation?

class A:
    def method(self):
        return None

class B(A):
    def method(self):
        return 1

(Edit: perhaps it would be more accurate to say, I see the value in this feature, but I also think there are significant design problems with it that we need to sort out our answers to before we merge an implementation of it. I also think it is low priority for the alpha.)

[carljm]

I also think it's unclear how much this improves the overall utility of red-knot an totally un-annotated code. I'm sure it will help some, but if function parameters are not annotated, then we still have very limited type information inside functions. And inferring types for function parameters is much more difficult (it means that checking any function depends on every module in the codebase.)

[mtshiba]

It seems that mypy and pyright behave differently in this regard.
In mypy, if the return type of a function is not annotated, it is simply set to Any, while in pyright it infers it.

There is also a discussion on mypy about inferring function's return type, and the main argument against it was that there are "functions with an obvious return type" and "functions with a less obvious return type", and that inferring the return type may cause false positive errors in the latter, and that the distinction between the two is not so clear.

In my personal opinion, I still think that this feature has a lot of utility. Inferring only the return type of functions that can be inferred "confidently" rather than that of all functions is an arbitrary but not so bad solution.

[carljm]

My issue with it is not solved by distinguishing "obvious" vs "non-obvious" return types. IMO that would simply make the feature more confusing, because it would be arbitrary when you make some change to your function and suddenly your inferred return type goes away.

My issue is summarized by my question above, and we need some answer to that question before we move ahead with the feature.

Pyright's answer is that A.method is inferred as returning None, and B.method is inferred as returning Literal[1], and pyright then emits a diagnostic because B.method is an invalid override for A.method (Literal[1] is not a subtype of None).

I don't think this is a good answer, because it violates the gradual guarantee. We now have a (possibly false positive) diagnostic on (possibly valid and working) unannotated code, which can only be removed by adding an annotation -> int | None on A.method.

There is nothing wrong with the original un-annotated code, if the author assumes a possible return type of int | None when they call .method(). Their code only becomes wrong when we assume (without the author having said so) that A.method defines a method which must always (including in all subclass overrides) return None.

[mtshiba]

While thinking about this issue, I came across the following comment:

The union Unknown | Literal[1] expresses the gradual type "some type at least as large as Literal[1] but possibly larger", which has the nice property that if you ever try to use it somewhere you can't use an integer, it'll error -- but it'll allow things other than Literal[1] to be assigned to it.

What about using Unknown | <inferred return type> when the return type of a function is not annotated?

Since is_assignable_to(Unknown | T, Unknown | U) holds for all types T, U, the gradual guarantees regarding method overriding should hold.

[sharkdp]

@carljm Sorry for not responding earlier here. I discussed this with Micha in one of our 1:1s, and we weren't sure if this would always be safe to do. I did not think about inheritance when writing this issue. Are there also issues with free functions, or would it be okay to use the union of all return expressions there?

I guess overloaded functions could also complicate the situation.

What about using Unknown | <inferred return type> when the return type of a function is not annotated?

I think that's a great idea. We would infer Unknown | None for the return type of A.method, which would reflect the fact that the "correct" annotation could be None or anything larger, like int | None. The return type can not be smaller than None though; otherwise the function body would be incorrect.

I also think it's unclear how much this improves the overall utility of red-knot an totally un-annotated code. I'm sure it will help some, but if function parameters are not annotated, then we still have very limited type information inside functions.

I agree that we would probably still infer Unknown as the return type of most un-annotated functions. But I think it can still be useful when calling into un-annotated code from a typed code base, for example:

number: int = untyped_library.function_that_I_expected_to_only_return_int()

where we could raise a diagnostic if we would see that function_that_I_expected_to_only_return_int can (implicitly or explicitly) return None, for example.

[carljm]

I think that inferring the union of return statements is safe for unannotated free functions, and that unioning with Unknown is a great solution for unannotated methods! I'd be happy moving forward with that plan.

[Avasam]

Also consider that final classes can't be subclassed.

Other cases that may be considered relatively safe are:

• the many dunders/special methods that have a documented expectation (so if the inferred return type is the same, it should be good)
• methods that override superclass' where the parent method is already "safely inferred" (using a larger type/union already violates the LSP)

[mtshiba]

I think that's a great idea. We would infer Unknown | None for the return type of A.method, which would reflect the fact that the "correct" annotation could be None or anything larger, like int | None. The return type can not be smaller than None though; otherwise the function body would be incorrect.

I think that inferring the union of return statements is safe for unannotated free functions, and that unioning with Unknown is a great solution for unannotated methods! I'd be happy moving forward with that plan.

Thanks, I will make a correction to astral-sh/ruff#17371 with the policy that for non-final class methods, the union with Unknown is the inferred return type.

• the many dunders/special methods that have a documented expectation (so if the inferred return type is the same, it should be good)
• methods that override superclass' where the parent method is already "safely inferred" (using a larger type/union already violates the LSP)

It seems good to use the explicitly specified return type of the superclass method.

But I'm not sure if this is OK to apply to dunders/special methods. Even if the documentation specifies the return type, that "gentlemen's agreement" is often ignored (e.g. numpy.ndarray.__eq__, pathlib.Path.__truediv__).

[carljm]

Yes, I agree that we should probably not implement special cases for dunder methods right now, just for superclass method with explicit return type.