python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.2k stars 2.78k forks source link

(🐞) No error when overload implementation has incompatible return type #12401

Open KotlinIsland opened 2 years ago

KotlinIsland commented 2 years ago
from typing import overload

@overload
def foo(a: int) -> int: ...

@overload
def foo(a: float) -> int: ...

def foo(a: object) -> object:  # no error
    return object()

playground

While the strictest of solutions would be to enforce the return type be an intersection of the overload return types, that would result in being overly strict to the point of being unusable. I suggest enforcing the return type to be a union of the overloads, this is not typesafe, but is much safer than not checking it at all.

JelleZijlstra commented 2 years ago

Not sure why this should be an error. object is a supertype of int, and it's generally not possible for mypy to check that overload implementations are correct.

KotlinIsland commented 2 years ago

@JelleZijlstra It's not to do with the implementation, object isn't a subtype of int, it's the same as the following scenario:

class A:
    def foo(self) -> int: ...

class B(A):
    def foo(self) -> object: ...

In the OP only subtypes of int would be valid, eg int or bool.

KotlinIsland commented 2 years ago

I think this is intentional, as there is no way to correctly annotate an overload like the following due to overload signatures being intersections in the return type.

The following:

from typing import overload

@overload
def foo(a: int) -> int: ...

@overload
def foo(a: str) -> str: ...

def foo(
    a: int | str  # correct
) -> int | str: ...  # incorrect, should be intersection of int and str

It is at least, much more close to being correct, and I would expect mypy to enforce this tighter(unionized) type.

That being said, the true nature of overloads is far more complicated than just unionizing the inputs and intersecting the outputs, their final form is a complex conditional type with no currently existing method of describing or checking them.

DetachHead commented 2 years ago

i believe this is related to https://github.com/microsoft/TypeScript/issues/47669. specifically, the issue is that the function implementation is not checked to be assignable to its type (the overloads).

in typescript, function interfaces/intersections can be used to create overloads on arrow functions as they do not have their own dedicated overload syntax, however this results in so many false positives that it's pretty much unusable.

take this example:

// Type '(value: string | number) => string | number' is not assignable to type '{ (value: number): number; (value: string): string; }'.
//   Type 'string | number' is not assignable to type 'number'.
//     Type 'string' is not assignable to type 'number'
const foo: {
    (value: number): number
    (value: string): string
} = (value) => value

there are two workarounds for this, both of which are pretty annoying:

const foo: {
    (value: number): number
    (value: string): string
} = (value: number | string) => value as number & string

or more accurately (which almost defeats the point of the overload in the first place):

const foo: {
    (value: number): number
    (value: string): string
} = <T extends number|string>(value: T): T => value

so i'm skeptical of making overloads work this way. while it is stricter, it makes overloads pretty much unusable in most cases (in typescript it's not as much of a problem because you can get around it with generics and conditional types for some of the more complicated overloads)

i think @KotlinIsland's idea of enforcing that the impl's return type is at least a union of the return types in the overloads is a decent compromise tho

KotlinIsland commented 2 years ago

or more accurately (which defeats the point of the overload in the first place):

const foo: {
    (value: number): number
    (value: string): string
} = <T extends number|string>(value: T): T => value

This is almost identical to a constrained TypeVar!

T = TypeVar("T", int, str)

def foo(value: T) -> T:
    return value