microsoft / TypeScript

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

Weird behaviour with an "evolving any" and a non-null assertion operator #60514

Open bradzacher opened 1 week ago

bradzacher commented 1 week ago

🔎 Search Terms

"evolving any", "non-null assertion", "compiler api"

🕗 Version & Regression Information

⏯ Playground Link

https://typescript-eslint.io/play/#ts=5.5.2&fileType=.tsx&code=CYUwxgNghgTiAEAzArgOzAFwJYHtVJxwAoBKALnlWQFsAjEGeAH3jVES1RGAFgAoUJFgIU6bHni1YpClToN%2B-UZlz5qUTgEZS8AN7948CCAzwAHgG4D5%2BAF5J0klb6GzdgsSf8A9N4B6APzWxqYAnpp21r6GgdaG8QmWUd4J8bEuRibwoQBMkRnR8OmpJWYAhM6GhanFmWEAzPlVKUVBGSXxblAAzpQ09DCV8NUJ6QC%2B-EA&eslintrc=N4KABGBEBOCuA2BTAzpAXGYBfEWg&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3QAacDUhFYASSSomyPAEEiATwphR6KQF8Q%2BoA&tokens=false

NOTE: I linked to the typescript-eslint playground just because it both supports // ^? and also provides access to the TS types so you can easily see the issue. You can ofc reproduce this in the TS playground too.

💻 Code

declare function foo(): number | undefined
declare function bar(): number

function main1() {
  let x;
  x = bar();
  x = foo();
//^?
  let y1 =
  //  ^?
           x;
  //       ^?
  let y2 =
  //  ^?
           x!;
  //       ^?
  let y3 =
  //  ^?
           x as number;
  //       ^?
}

🙁 Actual behavior

declare function foo(): number | undefined
declare function bar(): number

function main1() {
  let x;
  x = bar();
  x = foo();
//^? any ✅
  let y1 =
  //  ^? number | undefined ✅
           x;
  //       ^? number | undefined ✅
  let y2 =
  //  ^? number ✅
           x!;
  //       ^? number ❌
  let y3 =
  //  ^? number ✅
           x as number;
  //       ^? number | undefined ✅
}

🙂 Expected behavior

declare function foo(): number | undefined
declare function bar(): number

function main1() {
  let x;
  x = bar();
  x = foo();
//^? any ✅
  let y1 =
  //  ^? number | undefined ✅
           x;
  //       ^? number | undefined ✅
  let y2 =
  //  ^? number ✅
           x!;
  //       ^? number | undefined ❓❓❓❓
  let y3 =
  //  ^? number ✅
           x as number;
  //       ^? number | undefined ✅
}

Additional information about the issue

Reference: https://github.com/typescript-eslint/typescript-eslint/issues/10334

TypeScript-ESLint has a rule called no-unnecessary-type-assertion which, as the name implies, flags unnecessary type assertions to help keep the code clean.

One of the checks done by the rule involves flagging unnecessary non-null assertions. The basic logic for this check is "if the type of the thing does not include null | undefined then the non-null assertion is unnecessary". By "the thing" I am referring to the expression being non-null asserted, eg the x in x!.

In the case of an "evolving any" the type of the variable is shown to be incorrect by the TS APIs. As shown in the example code (specifically the y2 case) - the type of x is presented as number - even though it should be presented as number | undefined. It appears that for some reason the non-null assertion modifies the type of the variable to remove the | undefined, rather than just emitting a non-nullish type from the non-null assertion expression.

This is made even weirder from the fact that writing x as number behaves correctly and the type of x is correctly presented as number | undefined.

This discrepancy is problematic for our lint rule because it means we currently false-positive on this case because when we query for the type of x it looks non-nullish -- which suggests the non-null assertion is unnecessary. But if the user removes the non-null assertion then the type of x changes to number | undefined and this causes errors.

RyanCavanaugh commented 1 week ago

It seems like this is just a type display bug? If you write

let y2: number = x;

there's an error as expected

bradzacher commented 1 week ago

@RyanCavanaugh it's not just a display bug! If you inspect the type object that TS returns for the Identifier node within the NonNullExpression node then you will see the number type. Not just displayed as number, but it's not a union -- it's a type with flags TypeFlags.Number and intrinsicName: 'number'.

Whereas if you inspect the type object that TS returns for the Identifier node within the AsExpression node then you will see a union type whose constituents are the undefined type and the number type.

This was why I linked the typescript-eslint playground because it has a "Types" tab to inspect the types from TS -- but you can reproduce this via the TS APIs directly or via ts-ast-viewer.com too

bgenia commented 1 week ago

This is because checkIdentifier applies getNonNullableType if the type is automatic and the parent node is a non-null assertion, which causes type queries for that identifier to return non-nullable types. This was introduced in #50092.