microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.26k stars 12.38k forks source link

Union types and overloads acts a bit weirdly #33101

Open thetutlage opened 5 years ago

thetutlage commented 5 years ago

Acknowledgement

I have asked the question on Gitter and most of the users believe it's a bug and I have search for existing issues and didn't found any.

Issue

Let's start with the code. Playground link

declare class Foo {
    public red(): this
    public red(text: string): string
}

declare class Bar {
    public red(): this
    public red(text: string): string
}

const foo: Foo | Bar = new Foo()
const a = foo.red('text')

In the above code, I expect the variable a to be a string. However, it is string | Bar.

Now, things get even weird when I replace string with object in the return types of red method.

declare class Foo {
    public red(): this
    public red(text: string): object
}

declare class Bar {
    public red(): this
    public red(text: string): object
}

const foo: Foo | Bar = new Foo()
const a = foo.red('text')

Now suddenly, a is an object (which is correct)

fatcerberus commented 5 years ago

More fun:

declare class Foo<T> {
    public red(): this
    public red(text: string): T
}

declare const foo: Foo<string> | Foo<number>
const a = foo.red('text'); // string | Foo<number>

It picks one overload from each member of the union, and both are incompatible with the call site.

webstrand commented 5 years ago

I can confirm this is an issue on ts version 3.7.0-dev.20190823.

If Foo and Bar are given conflicting return types for the second overload:

declare class Foo {
    public red(): this
    public red(text: string): string
}

declare class Bar {
    public red(): this
    public red(text: string): number;
}

const foo: Foo | Bar = new Foo()
const a = foo.red('text'); // string?!

a is set to an invalid type string when it should be string | number.

AnyhowStep commented 5 years ago
declare class Foo {
    public red(): this
    public red(text: string): string
}

declare class Bar {
    public red(): this
    public red(text: string): number;
}

declare const foo: Foo | Bar;
//const a: string | Bar
const a = foo.red('text');

Playground

RyanCavanaugh commented 5 years ago

There are no generics in this example

RyanCavanaugh commented 5 years ago

This isn't really ideal but the overloads are in the "wrong" order and the Foo | Bar type is subject to subtype resolution, so some strangeness is bound to happen. Fixing either of those problems independently is sufficient to make things behave as expected.

keithlayne commented 5 years ago

Some additional fun stuff from our gitter exploration:

Take the original example:

declare class Foo {
    public red(): this
    public red(text: string): string
}

declare class Bar {
    public red(): this
    public red(text: string): string
}

declare const foo: Foo | Bar
const a = foo.red('text') // string | Bar
  1. Rearranging the overloads in Bar fixes a
  2. Making foo: Bar | Foo makes a: string | Foo
  3. Adding an identical class Baz (or more) and appending the type to foo's union also appends it to the type of a

I think that's all I had. But it does trigger one peeve of mine: conceptually, I don't expect the ordering within a union or intersection to matter. It's been pointed out that overloads are represented as intersections, so ordering matters there. So I think this may be the smallest example that shows ordering sensitivity in both a union and an intersection.

😠

Also @RyanCavanaugh can you elaborate on the 'right' order of overloads since I don't know of it being documented anywhere? I was thinking that it makes sense to order from more specific to more generic. My first thought was that the nullary version was most specific, but then it occurred to me that 1 arg is more 'specified' than no args, so maybe I had it backwards. If you could help my intuition there that would be great.

thetutlage commented 3 years ago

Its kind of sad that verified bugs are not getting fixed for years. :(