Assert when type inference can't find a common supertype

[nchen63]

TypeScript Version: 2.2.2

Code

// A *self-contained* demonstration of the problem follows...
function isEqual<T>(a: T, b: T) { return a === b; };

type Gut = { shape: "defined" | undefined };
let gut: Gut;
gut = { shape: undefined };
isEqual(gut.shape, "defined");
// isEqual("defined", gut.shape); // uncomment this line and comment out line above to get invalid supertype error

Expected behavior:

Actual behavior:

/usr/local/lib/node_modules/typescript/lib/tsc.js:51296
                throw e;
                ^
Error: Debug Failure. False expression: If there is no common supertype, each type should have a downfallType
    at Object.assert (/usr/local/lib/node_modules/typescript/lib/tsc.js:1888:23)
    at reportNoCommonSupertypeError (/usr/local/lib/node_modules/typescript/lib/tsc.js:26948:26)
    at resolveCall (/usr/local/lib/node_modules/typescript/lib/tsc.js:30747:21)
    at resolveCallExpression (/usr/local/lib/node_modules/typescript/lib/tsc.js:30869:20)
    at resolveSignature (/usr/local/lib/node_modules/typescript/lib/tsc.js:31024:28)
    at getResolvedSignature (/usr/local/lib/node_modules/typescript/lib/tsc.js:31041:26)
    at checkCallExpression (/usr/local/lib/node_modules/typescript/lib/tsc.js:31057:29)
    at checkExpressionWorker (/usr/local/lib/node_modules/typescript/lib/tsc.js:32249:28)
    at checkExpression (/usr/local/lib/node_modules/typescript/lib/tsc.js:32205:42)
    at checkExpressionStatement (/usr/local/lib/node_modules/typescript/lib/tsc.js:33857:13)

or if uncommenting out last line and commenting out 2nd to last line:

test.ts(8,1): error TS2453: The type argument for type parameter 'T' cannot be inferred from the usage. Consider specifying the type arguments explicitly.
  Type argument candidate '"defined"' is not a valid type argument because it is not a supertype of candidate '"defined" | undefined'.
    Type 'undefined' is not assignable to type '"defined"'.

[sandersn]

I got it to repro on master, but I needed strictNullChecks: true.

[sandersn]

The cause of the crash is a mismatch between type inference and type inference error reporting. I'll use a smaller repro with slightly simpler identifiers (my head started hurting while reading "defined" vs undefined):

declare function f<T extends number | undefined>(a: T, b: T): boolean;
let gut: { n: 12 | undefined };
f(gut.n, 12); // error, invalid supertype

Type inference works with these candidates: [12 | undefined, number]. It can't find a common supertype for them. Type inference error reporting works with these candidates: [12 | undefined, 12]. It asserts because it can't find the type that was supposed to make 12 | undefined fail — doesn't expect to succeed where type inference already failed. (It was number in actual type inference).

It looks like there are 3 possible fixes.

1. Don't widen the argument 12 to number in type inference.
2. Try to widen all type inference candidates before looking for a common supertype.
3. Try to widen all type inference candidates before trying to report errors on them.

(2) is probably the best fix but I'm going to see where (1) happens first. If it's solely within type inference and doesn't leak out into something like contextual typing, it might be worth changing to see how much breaks.

(3) just makes the compiler not crash when reporting an incorrect error, so it's not a great fix.

[sandersn]

getInferredType already widens literals, it turns out. It just doesn't widen the type { n: 12 | undefined } because the 12 that comes from a type does not widen, according the rules as described in #10676.

@ahejlsberg should the rules for widening be special-cased here to widen 12 | undefined to number | undefined? The rules for literal widening in type argument inference are convoluted enough to destroy my intuitions about what should be correct. I do note that it's weird that it would infer a type that is neither of the parameter types, number | undefined.

[sandersn]

Alternatively, should we not widen the type of the second argument, 12, to number? I guess that some (a lot of?) existing programs rely on that widening, though.

[sandersn]

I wrote up the PR for (3) and realised that even if we change the spec, we would still need to unify the implementations of getInferenceCandidates since type inference does some additional widening that type inference error reporting does not. Even if we change the spec, there might be some other way to hit the assert as long as the two code paths differ.

@ahejlsberg I want to discuss the spec with you in person so I can understand the motivation for widening in arguments and not widening in type literals.

[sandersn]

The PR now fixes (2) as well. getCommonSupertype, which is called right after getInferenceCandidates, wasn't correctly discarding | undefined from the 1 | undefined type. Thanks @ahejlsberg for help finding the incorrect code.

[sandersn]

Fixed by #16439