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

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

Open
bradzacher opened this issue Nov 16, 2024 · 3 comments Β· May be fixed by #60523
Open

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

bradzacher opened this issue Nov 16, 2024 · 3 comments Β· May be fixed by #60523
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@bradzacher
Copy link
Contributor

bradzacher commented Nov 16, 2024

πŸ”Ž Search Terms

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

πŸ•— Version & Regression Information

  • This changed in version 5.1

⏯ 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: typescript-eslint/typescript-eslint#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
Copy link
Member

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

let y2: number = x;

there's an error as expected

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Nov 17, 2024
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Nov 17, 2024
@bradzacher
Copy link
Contributor Author

@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
Copy link

bgenia commented Nov 17, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants