Skip to content

Non-null assertion operator confuses flow analysis with type guards #25802

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
rubenlg opened this issue Jul 19, 2018 · 6 comments
Closed

Non-null assertion operator confuses flow analysis with type guards #25802

rubenlg opened this issue Jul 19, 2018 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@rubenlg
Copy link

rubenlg commented Jul 19, 2018

TypeScript Version: 3.1.0-dev.20180717

Search Terms: null assert

Code

// A *self-contained* demonstration of the problem follows...
interface CaseA {
  type: 'A';
  data: number;
}

interface CaseB {
  type: 'B';
  b: string;
}

function isA(x: CaseA|CaseB): x is CaseA {
  return x.type === 'A';
}

interface Test {
  c: CaseA|CaseB;
}

function testWorks(t: Test) {
  if (isA(t.c)) {
    console.log(t.c.data);
  }
}

function testFails(t: Test) {
  if (isA(t!.c)) {
    console.log(t!.c!.data);
  }
}

Expected behavior: Both functions compile correctly.

Actual behavior: The second function does not compile, because after checking that t!.c is of type CaseA, the compiler forgets.

Note that t is not nullable, but that doesn't matter. This also fails with nested optional properties, or even if you make the function parameter optional.

A workaround is to pull t! to a variable, and then run the rest of the function on that variable.

Playground Link: Here

Related Issues:

@ghost
Copy link

ghost commented Jul 19, 2018

Duplicate of #15655

@ghost ghost marked this as a duplicate of #15655 Jul 19, 2018
@ghost ghost added the Duplicate An existing issue was already created label Jul 19, 2018
@rubenlg
Copy link
Author

rubenlg commented Jul 19, 2018

I actually think there is a deeper issue here, not just related to the assertion operator. I get the same odd behavior when using &&:

function testFails(t?: Test) {
  if (isA(t && t.c)) {
    console.log(t && t.c.data);
  }
}

Playground: here

Which probably means that the null propagation operator in #16 will have the same problem when it lands, since it's just syntactic sugar around this latest example.

@ghost
Copy link

ghost commented Jul 19, 2018

t!.c isn't shorthand for t && t.c, it's just shorthand for t.c. You're asserting to the type checker that t will always be defined at runtime.

@rubenlg
Copy link
Author

rubenlg commented Jul 20, 2018

Thanks @Andy-MS, I understand what the non-null assertion operator does. t!c is a shorthand for (t as NonNullable<typeof t>).c (regardless of how it's implemented internally which I don't know). What I'm saying is that this is not the only operator affected by this bug. t && t.c, which means something different, is also affected by the same bug (the compiler is not able to remember that the expression has been verified already with a type guard), and in that case there is no type casting involved.

This bug is not a huge deal for manually-written code, but it's a pain for code generators, because generating these variable extractions adds a lot of complexity to the generator.

My side concern is that the upcoming t?.c might also be affected, since it's the shorthand for t && t.c, and its value will be diminished by having to add boilerplate to extract variables before running the type guards.

Should I file a separate, more generic issue that doesn't focus on any specific operator, and just on the fact that some expressions get their type narrowed by type guards (e.g. isA(t.c)) while others don't (e.g. isA(t && t.c) or isA(t!.c) or isA((t as NonNullable<typeof t>).c)) ?

@ghost
Copy link

ghost commented Jul 20, 2018

We should probably support t?.c for type guards, but not arbitrary expressions like f(t), which might not be the same value each time. And testing every arbitrary expression to see if it has appeared before in a type guard would tax performance.

@rubenlg
Copy link
Author

rubenlg commented Jul 24, 2018

Thanks @Andy-MS. In that case let's close this. I think another duplicate candidate is #23662. They are all probably related.

@rubenlg rubenlg closed this as completed Jul 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

1 participant