Skip to content

API: getTypeAtLocation of node inside non-null assertion returns apparent type of TypeParameter with nullable constraint #21317

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

Closed
ajafff opened this issue Jan 20, 2018 · 5 comments
Labels
API Relates to the public API for TypeScript Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@ajafff
Copy link
Contributor

ajafff commented Jan 20, 2018

TypeScript Version: 2.7.0-dev.20180120 (20180119 works as expected)

Search Terms:

Code
--strictNullChecks is enabled

function test<T extends string | undefined, U extends string>(param1: T, param2: U) {
    param1;
    param1!;
    param2;
    param2!;
}

Expected behavior:
checker.getTypeAtLocation(node) where node is the Identifier param1 or param2 always returns a ts.TypeParameter.

Hovering param2 in both locations shows (parameter) param2: U extends string
Hovering param1 in both locations shows (parameter) param1: T extends string | undefined

Actual behavior:
result of checker.getTypeAtLocation(node):

  • on both uses of param2 returns ts.TypeParameter
  • on the first use of param1 returns ts.TypeParameter
  • on the second use of param1 (passing the Identifier as argument, not the NonNullExpression) returns ts.UnionType (string | undefined)

Hovering param2 in both locations shows (parameter) param2: U extends string
Hovering the first use of param1 shows (parameter) param1: T extends string | undefined
Hovering the second use of param1 shows (parameter) param1: string | undefined
The result of param1! is type string which is not valid according to #20974.

This change was introduced in #20995 as a bugfix. Looking at the baseline changes this might even be intended.

IMO this is pretty inconsistent if it only applies to TypeParameters with nullable constraint.
According to the reasoning in #20974 (comment) this new behavior is invalid.
And it's a breaking change for API users.

Playground Link: Playground doesn't use the correct nightly

Related Issues:
#20995

@mhegazy mhegazy added Bug A bug in TypeScript API Relates to the public API for TypeScript labels Jan 20, 2018
@mhegazy mhegazy added this to the TypeScript 2.8 milestone Jan 20, 2018
@mhegazy mhegazy assigned ghost Jan 20, 2018
ghost pushed a commit that referenced this issue Jan 22, 2018
@ghost ghost mentioned this issue Jan 22, 2018
@weswigham
Copy link
Member

weswigham commented Jan 23, 2018

Looking at the baseline changes this might even be intended.

Yes, it is intended. We wanted to allow people to assert nonullness of type variables, since otherwise you can perform no meaningful operations on them. The nonull operator is effectively a shorthand cast, so it losing a bit of information was deemed acceptable until such time as we can represent it completely using a negated type.

@ajafff
Copy link
Contributor Author

ajafff commented Jan 23, 2018

Thank you for the explanation. This actually makes sense.
This is still labeled as bug because the API returns the wrong type?

@weswigham
Copy link
Member

weswigham commented Jan 24, 2018

@ajafff This is likely still labeled as a bug because we haven't gotten a chance to talk about it in the office and have it retriaged yet 😉

@ajafff
Copy link
Contributor Author

ajafff commented Jan 24, 2018

Continuing discussion from #21349 here:

The type before the assertion is going to be the constraint (string | undefined, because it's an apparent position thanks to the assertion), and then after the assertion is applied it should just be string. This isn't an API issue, we legitimately interpret the type at that position as just the constraint now, that way the bang after it functions somewhat.

Let me share my use case:
I want to detect if a variable may be used before being assigned. Until #20221 is implemented, I need a workaround. One part of that workaround is checking if the type before the assertion is exactly the same as the declared type of the variable.
Now that I get the apparent / constrained type, I can no longer distinguish the following:

function fn<T extends string | undefined>(param: T) {
    let foo: T;
    let bar: T | undefined;
    if (Boolean()) {
        foo = bar = param;
    }
    foo!; // expected 'foo' to have type 'T', got 'string | undefined'
    bar!; // expected 'bar' to have type 'T | undefined', got `string | undefined`
}

Now there is no way for me to know that banging foo (foo!) is necessary to avoid a compile error while the assertion in bar! is redundant.

we legitimately interpret the type at that position as just the constraint now, that way the bang after it functions somewhat.

Can't you just do that getConstrainedType magic inside checkNonNullAssertion? That way the API still returns what I requested.

@weswigham
Copy link
Member

weswigham commented Feb 14, 2018

Now that conditional types have landed, we're probably doing to change the assertion operator to narrow to NonNullable<T>, rather than the overeager constraint things we're doing in nightly. Just a heads-up.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Mar 5, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Relates to the public API for TypeScript Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

3 participants