Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'never' type #8652

Merged
merged 6 commits into from
May 18, 2016
Merged

Add 'never' type #8652

merged 6 commits into from
May 18, 2016

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented May 17, 2016

This PR introduces a never type that represents the type of values that never occur. Specifically, never is the return type for functions that never return and never is the type of variables under type guards that are never true. The never type has the following characteristics:

  • never is a subtype of and assignable to every type.
  • No type is a subtype of or assignable to never (except never itself).
  • In a function expression or arrow function with no return type annotation, if the function has no return statements, or only return statements with expressions of type never, and if the end point of the function is not reachable (as determined by control flow analysis), the inferred return type for the function is never.
  • In a function with an explicit never return type annotation, all return statements (if any) must have expressions of type never and the end point of the function must not be reachable.

Because never is a subtype of every type, it is always omitted from union types and it is ignored in function return type inference as long as there are other types being returned.

The never type replaces the nothing type introduced in #8340 and it is effectively the same as the bottom type proposed in #3076.

Some examples of functions returning never:

// Function returning never must have unreachable end point
function error(message: string): never {
    throw new Error(message);
}

// Inferred return type is never
function fail() {
    return error("Something failed");
}

// Function returning never must have unreachable end point
function infiniteLoop(): never {
    while (true) {
    }
}

Some examples of use of functions returning never:

// Inferred return type is number
function move1(direction: "up" | "down") {
    switch (direction) {
        case "up":
            return 1;
        case "down":
            return -1; 
    }
    return error("Should never get here");
}

// Inferred return type is number
function move2(direction: "up" | "down") {
    return direction === "up" ? 1 :
        direction === "down" ? -1 :
        error("Should never get here");
}

// Inferred return type is T
function check<T>(x: T | undefined) {
    return x || error("Undefined value");
}

Because never is assignable to every type, a function returning never can be used when a callback returning a more specific type is required:

function test(cb: () => string) {
    let s = cb();
    return s;
}

test(() => "hello");
test(() => fail());
test(() => { throw new Error(); })

Fixes #3076.
Fixes #8602.

@DanielRosenwasser
Copy link
Member

I really don't think this name is good for the general user experience. My feeling is that it seems to model several different things which the name doesn't reflect very well on. At least nothing reflected the domain of values a little bit better.

@ahejlsberg ahejlsberg changed the title Add 'never type Add 'never' type May 17, 2016
@ahejlsberg
Copy link
Member Author

ahejlsberg commented May 17, 2016

@DanielRosenwasser The primary use for never is as the return type of functions that never return. I'm hard pressed to come up with a more concise and descriptive name. Certainly nothing isn't in my mind.

@RyanCavanaugh
Copy link
Member

The problem with nothing is that it's very confusing to then say "void is the return type of functions that return nothing". never also parses well with quick info in typeguarded code:

function f(x: A | B) {
  if(isA(x)) {
  } else if(isB(x)) {
  } else {
    x; // appears as type 'never'
  }
}

@sandersn
Copy link
Member

My objection to never is that it reads strangely: "f returns never" wants to be "f never returns". But "f returns nothing" reads fine.
It would help to show some examples of never actually used in some of the tests.

Other than that, and bike shedding on the name, the code looks good.

@DanielRosenwasser
Copy link
Member

Agreed with Nathan - the code looks fine, but I have those gripes with the naming.

@ahejlsberg
Copy link
Member Author

Yes, but "f returns nothing" also strongly implies that f actually returns. Which is precisely the intuition we want to avoid!

@sandersn
Copy link
Member

Interesting. That makes it sounds intended that none of the tests explicitly mention their return type is never. Could we get away with not making never a keyword and having it always be inferred?

@ahejlsberg
Copy link
Member Author

No, we can't get away with never always being inferred. We need it in declaration files, for example. Several of the tests in the PR already explicitly specify never as their return type.

@sandersn
Copy link
Member

OK, that makes sense. The d.ts is a good enough reason to have the keyword, although I noticed the rest -- outside a d.ts -- are all error cases where the annotation specifies : never and then fails to get it.

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

This will be a breaking change for back compat for .d.ts emit. in the past we inferred the type of these methods/functions as void. some one using nightly builds will generate .d.ts files that have the new type never that is not consumable by TS 1.8 users.

@ahejlsberg
Copy link
Member Author

@mhegazy Yes, but an easy fix is to add a void type annotation.

@ahejlsberg ahejlsberg added this to the TypeScript 2.0 milestone May 18, 2016
@mhegazy
Copy link
Contributor

mhegazy commented May 18, 2016

we said we will be more careful in the future with such changes.

@ahejlsberg
Copy link
Member Author

I don't see a big issue here. We're being more precise in our type analysis and that may change the outcome of inference. There are other such issues already resulting from the control flow based type analysis (e.g. narrowing occurs in more places which might affect inferred return types). Effectively we can only replicate the old results by keeping the old code and not changing anything. I think it is reasonable to recommend explicit type annotations if you want to freeze the shape of an API.

@mhegazy
Copy link
Contributor

mhegazy commented May 18, 2016

Being more precise is one issue. Generating a .d.ts that is not consumable is another.

@ahejlsberg
Copy link
Member Author

I'm not sure where you're going with this. What are you proposing?

@yortus
Copy link
Contributor

yortus commented May 18, 2016

@ahejlsberg I've just submitted an issue (#8655) about better type analysis around assert(...) calls. Could this never type be a step toward that kind of analysis becoming possible in future? assert functions might require a return type annotation like void | never or x is T | never to tell consumers they either prove a type assertion or don't return at all.

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented May 18, 2016

Sorry for not minding my own business. The official 1.8. has its *.d.ts fixed and shipped, whoever needs it is welcome to get it anytime. Technically speaking nightly build is 1.9.+ with a bump in the major version number which means breaking changes are likely. Can't see any crime here. People who are scared to break their code are warned to proceed with caution or stay where they are. Am I missing the point?

@kitsonk
Copy link
Contributor

kitsonk commented May 18, 2016

The official 1.8. has its *.d.ts fixed and shipped, whoever needs it is welcome to get it anytime. Technically speaking nightly build is 1.9.+ with a bump in the major version number which means breaking changes are likely. Can't see any crime here. People who are scared to break their code are warned to proceed with caution or stay where they are. Am I missing the point?

It isn't only the TypeScript team that generates .d.ts files. 😄 Meaning that there is a fracture in the entire ecosystem. For example if Angular 2, Dojo 2, RxJS, etc. migrate then it causes them to generate definition files that are not consumable by those that haven't upgraded, continuing to fracture and annoy the wider TypeScript community. Many large entities don't have the time or resources to keep upgrading every 6 weeks.

Both @ahejlsberg and @mhegazy have points. My opinion is if this lands for 2.0, there is the readonly breaking change. Arguably that is opt-in, while in theory, this would likely end up not being opt-in, but invariably people are going to start producing .d.ts files with readonly anyways and stranding < 2.0 users. So I would say 2.0 would be the point to break things from an interface perspective.

@kitsonk
Copy link
Contributor

kitsonk commented May 18, 2016

Not quite seeing the issue in the example.

These people (for disclosure, I am the lead for Dojo 2) issue .d.ts which are then consumed by other people to build applications. Your original statement was:

The official 1.8. has its *.d.ts fixed and shipped, whoever needs it is welcome to get it anytime.

But instead of acknowledging that there is potentially more breakage than just what version of the lib.d.ts someone is using, you suggest that project like Dojo 2 (of which I am the lead) can start issuing versions of their .d.ts files without regard for the fact that these projects have a downstream user base. There is a risk with a non-optional breakage like this that we could have serious impacts downstream.

I clearly indicated that both Anders and Mohammad had valid points, of which you seemed to suggest in your original statement confusion about why Mohammad was concerned. I am glad he is concerned. I know both Anders and Mohammad consider downstream projects like Dojo 2 and others in their decisions. Ryan further added though that valid point that this isn't the first breakage of definition files that force the larger community to upgrade. I would argue though that intersection and union types were opt-in, so in theory downstream projects could have chosen not to use them, but they were actually key enablers and so I know we chose to opt-in to them.

Clearly the compromise of emitting a void in declaration files would be perfect and cause a whole lot less downstream carnage. I am really glad to know that the team thinks about these things instead of going "oh, whats the big deal, people can just upgrade"...

@zpdDG4gta8XKpMCd
Copy link

you suggest that project like Dojo 2 (of which I am the lead) can start issuing versions of their .d.ts files

Where did I say that?

What I said is: it's a purely engineering problem, and this is what we, engineers, do for living: solve them, and not to seem too arrogant I even outlined how it can be done in typical cases.

Now if the budget of TS is big enough to take on supporting backward compatibility for every team that might benefit of it, I am all for it. Doubtfully so.

More realistically the budget of TS can be better spent on developing new features merits of which far outweigh the troubles of adopting them. And this is what I am rooting for.

Lastly please give an example how v.2.0/lib.d.ts has anything to do with v.1.8/dojo.d.ts?

@ahejlsberg
Copy link
Member Author

I'm not sure generating : void instead of : never in .d.ts files until we ship 2.0 will materially change anything. Aside from appearing to be a bug, it just delays the realization that something has changed. In fact, if we're going to break something then we should break it early in a nightly build to give folks time to adapt. Introducing the actual break right as we ship seems really bad.

We already have a breaking change in 2.0 with readonly because we sometimes infer it (e.g. when you have a get accessor without a set accessor). I think this one is very similar. I also think it is quite reasonable to ship these changes as long as we document them and the appropriate workarounds.

Alternative solutions are:

  • Never infer never as a return type and instead require an explicit type annotation.
  • Introduce (yet another) switch to enable or disable inference of never return types.

I think both of those are worse than just introducing the change. There is real value in having inference produce never as it calls out an important behavioral distinction. We lose that with an opt-in model. And we really don't want more switches.

@mhegazy
Copy link
Contributor

mhegazy commented May 18, 2016

In #8252 we said we will add this to our "feature checklist". i am just bringing this up as a violation of this.

Given that we rejected the sourceVersion suggestion earlier this weeks, I believe there are two options here, the first is the one you mentioned (that also should apply to undefined and null now that i think about it), and the second is to say this is too much of a restriction, and it is not supported.
My vote would be for the second.

@ahejlsberg
Copy link
Member Author

Discussed with @mhegazy. We're good with documenting this as a breaking change for .d.ts emit and will also document the workaround (add void type annotations).

@ahejlsberg ahejlsberg merged commit 59f269c into master May 18, 2016
@ahejlsberg ahejlsberg deleted the neverType branch May 18, 2016 19:55
@JsonFreeman
Copy link
Contributor

What a nice feature! I also quite like the name.

@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label May 20, 2016
@mhegazy
Copy link
Contributor

mhegazy commented May 20, 2016

Note for breaking changes docs:
A common break would be a base class with an implementation to always throws. previously the return was inferred as void, now it is never.

Example:

class Base {
    method() {
        throw new Error("Not Implemented!");
    }
}

class Derived extends Base {
    method() {
        return 0; 
    }
}

// error TS2415: Class 'Derived' incorrectly extends base class 'Base'.
//   Types of property 'method' are incompatible.
//     Type '() => number' is not assignable to type '() => never'.
//       Type 'number' is not assignable to type 'never'.

Recommendation:

Give the base class method a type annotation of the expected return type, be it void or any other concrete type.

@ahejlsberg
Copy link
Member Author

ahejlsberg commented May 23, 2016

With #8767 we have modified the inference rules to fix the breaking change above. I think we can just consider this a new feature and remove the breaking change label.

@usta
Copy link

usta commented Jun 2, 2016

how about naming it as "impossible" instead of "never"

@kostat
Copy link

kostat commented Jun 2, 2016

I understand that the primary reason to call the type "never" derives from the desire to facilitate the reading the never returning function declaration. For me, it does not reflect the essence of the type. Look:

var x:never = IAlwaysThrow();

On the other hand the common definition of such code is unreachable code block. Wouldn't it be better to name the type "unreachable"?

function IAlwaysThrow() : unreachable {
    throw Error();
}

var x:unreachable = IAlwaysThrow();

I understand that no one will ever declare a variable of 'unreachable' type, but type is type. When I see var y:number, I attribute some properties to y. On the same way when I see x:unreachable I understand that I will never reach this point in code. "Never" is close, yet it's uncommon term for this case.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 2, 2016

Considering this is already merged and documented, all we are doing is 🚲 🏠 and navel gazing something is a semantic opinion. The naming was already debated and settled.

@fightingcat
Copy link

What's the point?

@kitsonk
Copy link
Contributor

kitsonk commented Jul 21, 2016

@fightingcat the point is that TypeScript lacked a bottom type and that made it difficult to properly enforce parts of a type system.

@RyanCavanaugh
Copy link
Member

@remojansen please use Stack Overflow for questions -- thanks!

@remojansen
Copy link
Contributor

@RyanCavanaugh sorry my mistake will move it to SO.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet