Literal type narrowing regression

TypeScript Version: typescript@2.1.0-dev.20160913

Code

type FAILURE = "FAILURE";
const FAILURE = "FAILURE";

type Result<T> = T | FAILURE;

function doWork<T>(): Result<T> {
  return FAILURE;
}

function isSuccess<T>(result: Result<T>): result is T {
  return !isFailure(result);
}

function isFailure<T>(result: Result<T>): result is FAILURE {
  return result === FAILURE;
}

function increment(x: number): number {
  return x + 1;
}

let result = doWork<number>();

if (isSuccess(result)) {
  increment(result);
//          ~~~~~~
//          Argument of type 'string | number' is not assignable to parameter of type 'number'.
//            Type 'string' is not assignable to type 'number'.
}

Expected behavior:

result (before the narrowing) should be "FAILURE" | number
result (after the narrowing) should be number

Actual behavior:

result (before the narrowing) is string | number
result (after the narrowing) is string | number

This is working on 20160911 but broke in 20160912, most likely related to #10676

It seems like the change is somewhat intentional – that let is getting the "base primitive types" now. However, since the function (doWork, and also isSuccess and isFailure have explicit annotations), I suspect it is possible to keep this pattern working without breaking the other goals?

the change is intentional. see #10676 for more details. I do not think this is a pattern we explicitly thought about; so we need to go back and talk about this.

a work around would be an explicit type annotation to result: let result: Result<number>

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) keep me posted! 👍 I did try to read #10676, and there seems to be some thoughts/effort in making functions with explicit return type annotation work, so I am crossing my fingers :P

Anders Hejlsberg (@ahejlsberg)

//cc: Anders Hejlsberg (@ahejlsberg)

The issue here is that when we infer the type number | "FAILURE" for a mutable location, we can't tell that "FAILURE" is a special type that you don't want widened to string. Indeed, had the type been 1 | "FAILURE" you probably would have wanted the 1 to be widened to number.

One possible solution is to change the first two lines to:

type FAILURE = "FAILURE" & {};
const FAILURE: FAILURE = "FAILURE";

This turns the FAILURE type into an intersection that won't be widened, but otherwise is similar to the literal type "FAILURE". But I think an even better change would be to use an object as the sentinel failure value. That way Result<T> would also work when T is a string.

type FAILURE = { __kind__: "FAILURE" };
const FAILURE: FAILURE = { __kind__: "FAILURE" };

I agree the last solution would work and arguably better, but there are still a few things that doesn't feel right to me:

When there is an explicitly typed return value, it feels wrong to infer the type on the let, instead of just taking the returned type verbatim. IMO, the explicit return type is a pretty strong signal that you know what you are expecting, and the widening caused by the inference is probably just going to invite bugs.
Even with the widening the error still seems wrong. I would expect that it fails on the call to isSuccess because the argument type doesn't match the signature (which really highlights my problem with 1). If that's somehow okay, then the narrowing should have worked - in the current version it seems like the value is T assertion is just silently dropped, making the isResult a noop.

When there is an explicitly typed return value, it feels wrong to infer the type on the let, instead of just taking the returned type verbatim. IMO, the explicit return type is a pretty strong signal that you know what you are expecting, and the widening caused by the inference is probably just going to invite bugs.

Consider this modified example:

function doWork<T>(x: T): Result<T> {
    return x;
}

const c = doWork(1);  // Result<1>
let v = doWork(1);  // Result<1> widened to Result<number>

In both cases doWork(1) returns a Result<1>, preserving the fact that the argument was the literal 1. The type is widened to Result<number> when we infer it as the type for v because we're inferring for a mutable location (e.g. you might later want to assign a Result<123> to v). The explicitly typed return value isn't really an indicator of intent.

The modified example above works when FAILURE is changed in one of the ways I proposed. However, in your original example, the widening of Result<number> (which is equivalent to number | "FAILURE") ends up producing number | string because the "FAILURE" type is just a string literal type like any other.

Even with the widening the error still seems wrong. I would expect that it fails on the call to isSuccess because the argument type doesn't match the signature (which really highlights my problem with 1). If that's somehow okay, then the narrowing should have worked - in the current version it seems like the value is T assertion is just silently dropped, making the isResult a noop.

What happens is that when inferring from number | string to T | "FAILURE", we infer number | string for T, just as we would have inferred number if you had passed a plain number. Effectively, any type is a valid argument for isSuccess because anything will match T | "FAILURE" as there is no constraint on T.

I'm re-labeling as Breaking Change as this is working as intended and is an effect of our new approach to better preserving literal types.

I was thinking about this issue and what dawned on me is that maybe it will be most intuitive if explicit literal, non-template types are not widened. It's subtle, but the purpose of the union is that the literal options are something you expect and prepare to discriminate on even in mutable locations. Maybe I'm wrong, but I wasn't able to come up with a counter example.

Godfrey Chan (@chancancode)

Godfrey Chan (@chancancode) Yehuda Katz (@wycats) We have a fix for this in #11126. Explicit type annotations are now respected and your examples behave as expected. Thanks for the well considered feedback (even if it did arrive as a wall of text 😃).

I'm glad that #11126 helps with examples above - but wow - rules got complicated now. One need to think now in terms of literal types and widening literal types and combination of them and some cases that cancel the widening if there is non-widening form that gets priority :)

This example

declare let cond: boolean;
const c1 = cond ? "foo" : "bar";
const c2: "foo" | "bar" = c1;
const c3 = cond ? c1 : c2;
const c4 = cond ? c3 : "baz";
const c5: "foo" | "bar" | "baz" = c4;

let v1 = c1;  // string
let v2 = c2;  // "foo" | "bar"
let v3 = c3;  // "foo" | "bar"
let v4 = c4;  // string
let v5 = c5;  // "foo" | "bar" | "baz"

reminds me one of this famous CoffeeScript question - "What is the result of :"

foo bar and hello world

where users might need to compile the code first to check if it is foo(bar && hello(world)) or foo(bar) && hello(world)

I wish there was a way to satisfy both:

use cases above and
keeping the language easy to reason about - but maybe there is no such way.

One request though would be to add support to TSServer so that it could show users if literal type is detected as widening literal type, because right now for example above it will show:

and one could be surprised that v4 is of type string

so instead of

const c4: "foo" | "bar" | "baz"

if it could show:

const c4: "foo" | "bar" | widening "baz"

Thanks a lot for awesome TS

@wallverb Yeah, I specifically tried to capture as many subtleties as possible in a short example, and it definitely adds another layer to the rules. I think the intuition is reasonably simple though--as long as an explicit type annotation hasn't been encountered for a particular literal in an expression, that literal type will widen to string when inferred for a mutable location.

@wallverb I'll think about the suggestion of showing widening (or some other indicator) in the hints. In general I'm not crazy about showing syntax that isn't supported in the language, and I don't think the distinction merits elevation to an actual type modifier.

Yehuda Katz (@wycats)

I find it really unfortunate that now more annotations are needed for use-cases where the implicit intent is to use a literal union as a type. I can't formulate a formal proposal, but my strong feeling is that wherever an explicit literal type annotation is given, it should be preserved regardless of mutable or immutable locations.

For example:

type Rel = 'lt' | 'eq' | 'gt';

function compare(x: number, y: number): Rel {
    return (x === y) ? 'eq' : (x > y) ? 'gt' : 'lt';
}

const cmpRes = compare(1, 2); // `Rel`, ok
let cmpResMut = compare(1, 2); // `string`, what? when did I say that?
                               // for what reason will I ever assign 'hello' to a comparison result?

The second widening is like widening a subclass to the base or any value to Object (as Yehuda Katz (@wycats) pointed out). It allows for unintended values to be assigned and is arguably not intuitive. If this was the intent, than a natural (and a bless for the reader/maintainer) solution is to add the widened annotation by hand.

The same applies to object literals:

const x = {
    left: 1,
    right: 2,
    cmp: compare(1, 2) // why string?
}

class A {}
class B extends A {}
const y = { instance: new B() }; // it is correctly `{ instance: B }`

As a side note, this is also a regression in the display of types in IDEs, cmpRes used to be Rel now it is expanded to its members.

Marin Marinov (@gcnew)

Marin Marinov (@gcnew) Agreed. That is exactly the fix we have in #11126.

Well if that's the case then it's good news, I couldn't get it from the examples. I still have the feeling that apparent literal types are inferred less often after #10676. With #11126 we'll have a mechanism for forcing them at the expense of verbosity (and unfortunately they are not transitive :().

type Rel = 'lt' | 'eq' | 'gt';

function compare(x: number, y: number): Rel {
    return (x === y) ? 'eq' : (x > y) ? 'gt' : 'lt';
}

const cmpRes = compare(1, 2); // `Rel`, ok
let cmpResMut = compare(1, 2); // `string`, what? when did I say that?
                               // for what reason will I ever assign 'hello' to a comparison result?

Unless I understand wrong, both cmpRes and cmpResMut would have the type Rel under #11126, because compare() explicitly expresses a return type of Rel.

Marin Marinov (@gcnew)

Marin Marinov (@gcnew) Not sure what you mean by not transitive. One thing is certain though: With #10676 and #11126 in place, we'll never infer less specific types for string and numeric literals than we used to. Specifically, we behave just like before when you have explicit type annotations, and whereas we used to immediately widen un-annotated const declarations we now preserve string and numeric literal types from un-annotated const declarations for as long as possible. It's strictly a better thing.

My apologies. Literal types now work as expected, thanks :)

Navigation Menu

Repository navigation

Literal type narrowing regression #10898

8 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Literal type narrowing regression #10898

Description

Activity

chancancode commented on Sep 13, 2016

chancancode commented on Sep 13, 2016

mhegazy commented on Sep 13, 2016

chancancode commented on Sep 13, 2016

mhegazy commented on Sep 13, 2016

ahejlsberg commented on Sep 14, 2016

chancancode commented on Sep 14, 2016

ahejlsberg commented on Sep 14, 2016

ahejlsberg commented on Sep 14, 2016

gcnew commented on Sep 15, 2016

8 remaining items

ahejlsberg commented on Sep 24, 2016

normalser commented on Sep 24, 2016

ahejlsberg commented on Sep 24, 2016

ahejlsberg commented on Sep 24, 2016

gcnew commented on Sep 25, 2016

ahejlsberg commented on Sep 25, 2016

gcnew commented on Sep 26, 2016

wycats commented on Sep 26, 2016

ahejlsberg commented on Sep 26, 2016

gcnew commented on Sep 27, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions