microsoft / TypeScript

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

type `nothing` emitted in generated .d.ts files #8345

Closed mhegazy closed 8 years ago

mhegazy commented 8 years ago
function f() {
    var x: string | undefined = undefined;
    y = 0;
    if (x) {
       var y: typeof x;
    }
    return y; // y has type nothing
}

generates invalid .d.ts file

declare function f(): nothing;

this is a regression introduced by https://github.com/Microsoft/TypeScript/pull/8340

ahejlsberg commented 8 years ago

I'm tempted to say just don't do that. But I won't. :wink:

I definitely don't think the solution is to make nothing a manifest type that you can reference in a type annotation. That leaves rewriting (but to what?) or issuing an error. The error is probably the better option.

DanielRosenwasser commented 8 years ago

I get why {} was not appropriate. But nothing sounds a heck of a lot like what I'd associate void with, so what exactly is the intended difference semantically?

mihailik commented 8 years ago

Can it return any and for those interested in picking at those little tiny details let noImplicitAny generate an error?

ahejlsberg commented 8 years ago

@DanielRosenwasser There are a couple of important differences between void and nothing.

I do agree we have bit of a profusion of nothingness types (undefined, null, void, nothing), but they all have important differences. If anything had to change, we could possibly consider removing the undefined type and having void be the type of the undefined literal. But that would allow silly stuff like assigning the result of a void function to a variable can be undefined, which really should be an error.

mhegazy commented 8 years ago

here is a less artificial example:

function bar() {
    var x: string | undefined;
    if (x) {
        return x;
    }
    throw new Error();
}
mhegazy commented 8 years ago

so for .d.ts purposes, should we 1. write void, 2. undefined, 3. nothing and make it legal to name this type.

ahejlsberg commented 8 years ago

(1) and (2) aren't correct and (3) really doesn't help anyone. I think we should (4) issue an error.

ahejlsberg commented 8 years ago

Adding some rationale for (4): There's really no situation ever where you want nothing to be the declared type of an entity. It is typically an indicator of a logic error, but we don't want to eagerly issue the error because you may be writing "defensive" code that deals with incorrect parameters passed from code that doesn't follow the stated contract (such as plain JavaScript).

mihailik commented 8 years ago
function bar() {
    throw new Error();
}

Under these reasoning this should produce error too.

mhegazy commented 8 years ago

no that is void.

mihailik commented 8 years ago

Exactly, and that's the same code as in your example (sans no-op if).

Do we end-users of the compiler need this level of precision? By all means it may add value internally, but best reduce peculiarity on the outside, coercing nothing to void or unknown in codegen/diagnostic messages.

mhegazy commented 8 years ago

void means it returns no value. nothing means it returned a value, but its type was narrowed down by the compiler to nothing. The assumption is that is never the intention of the user. so if you did not mean for this to happen, so you might want to take a look at the source and see where did the nothing came from, and the error does that.

aluanhaddad commented 8 years ago

Do we end-users of the compiler need this level of precision? By all means it may add value internally, but best reduce peculiarity on the outside, coercing nothing to void or unknown in codegen/diagnostic messages.

The TypeScript compiler is a TypeScript program so I do not think that would be either possible or valuable.

👍 for making it an error.

mihailik commented 8 years ago

@mhegazy unless this detection generates false positives, as @ahejlsberg pointed out above.

function validateType(val: number | string) {
    if (typeof val === 'string') return null;
    else {
        if (typeof val === 'number') return null;
        else return val; // validation fails at runtime, but this code is valid
    }
}

function validateTypeMoreDetails(val: number | string) {
    if (typeof val === 'string') return null;
    else {
        if (typeof val === 'number') return null;
        else return 'Incorrect type ' + typeof val+' ('+val+')'; // should 'nothing' support plus operator with string?
    }
}
mihailik commented 8 years ago

Set theory (empowered by union and intersection types) is bound to produce oddities -- hence a need to limit that power in places where it becomes too exotic and impractical.

Coercing nothing to void or similar may be one of those intentional trivializations for the sake of sanity and practicality. You know saying 'the buck stops with me'? Make that nothing buck stay within the compiler, don't pass it out in the world.

DanielRosenwasser commented 8 years ago

I don't think the nothing type is actually that big of a deal, I just think it's odd to make it an inexpressible type. I don't see a reason to limit users from writing it out like with how null and undefined use to be disallowed.

jeffreymorlan commented 8 years ago

There's really no situation ever where you want nothing to be the declared type of an entity.

A function that always throws an exception, never returning normally, logically has a return value of nothing. Unlike a void-returning function, a nothing-returning function could safely be assigned to any other return type:

function stub() { throw Error("Unimplemented"); }
interface SomeInterface {
    someMethod(): number;
    someIrrelevantMethod1(): number;
    someIrrelevantMethod2(): string;
}
var obj: SomeInterface = {
    someMethod() { ... },
    someIrrelevantMethod1: stub,
    someIrrelevantMethod2: stub
};

Currently this is an error, despite being perfectly type-safe - making stub return nothing would fix that. (Or any, but that makes the type less self-documenting)

aluanhaddad commented 8 years ago

@jeffreymorlan perfectly typesafe, but known to produce an error when invoked.

mhegazy commented 8 years ago

should be fixed by https://github.com/Microsoft/TypeScript/pull/8652

ahejlsberg commented 8 years ago

Fixed in #8652.