Skip to content

Bug: [no-unnecessary-type-assertion] false positive on evolving type #10334

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
4 tasks done
OpportunityLiu opened this issue Nov 15, 2024 · 6 comments
Closed
4 tasks done
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@OpportunityLiu
Copy link

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Playground Link

https://typescript-eslint.io/play/#ts=5.6.3&fileType=.tsx&code=LAKAJgpgxgNghgJwgAgGYFcB2UAuBLAe0zQIIAoBKALmU3QFsAjCBZAH2S0lT0wjFCRYiFBmz4iyRoko06TFoOjwkaLLkLFpALzKY5DZgmrIAbgTxgA3KFBiNk%2BnF4BGSsgDeoZMhgQcyAAeNiA%2BgcgAvFIyFCFhkSTksd7RuoEAhMkgAL62IPYSxE68AEzuXqG%2B-kFxQQnSCJS14VGopE0pOmSBWbkgQA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tiacTJTIAhtEK0ipWkOTJE0fJQ5N0UOdA7RI4MAF8QOoA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eFYDArugDTg10NM8AOXapUAYQAW6aAGsylDul4BfECqA&tokens=false

Repro Code

declare function foo(): number | undefined
declare function bar(): number
declare function baz(n: number): void;

function main1() {
  let x;
  x = bar();
  x = foo();
  baz(x!);
}

function main2() {
  let x;
  x = bar();
  x = foo();
  baz(x);
}

ESLint Config

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/no-unnecessary-type-assertion": "error"
  },
};

tsconfig

{
  "compilerOptions": {
    "strict": true
  }
}

Expected Result

The linter should not treat baz(x!) as an unnecessary type assertion

Actual Result

@typescript-eslint/no-unnecessary-type-assertion rule warns about the assertion baz(x!) as unnecessary, suggesting it could be removed. Removing the assertion causes TypeScript to raise an error.

Additional Info

No response

@OpportunityLiu OpportunityLiu added bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Nov 15, 2024
@OpportunityLiu
Copy link
Author

OpportunityLiu commented Nov 15, 2024

Maybe related: #7652 #8721 #10257

@bradzacher
Copy link
Member

This is a case of strange behaviour from TS.
The type of your variable is a special kind of any that evolves and changes as you assign and use the variable.

here is a playground with some annotations to show how things are weird

The important bit is that if you inspect the type of the identifier, x with and without a non-null assertion you'll see that TS itself reports weird types. When there is not a non-null assertion operator then the type of x is number | undefined, but when there is a non-null assertion operator then the type of x is just number.

To clarify -- it's specifically the identifier x that changes type, not the result of the non-null assertion operator!
This is why the rule reports - because we see a non-nullish type and thus based on the types the operator is unnecessary.

This might be a bug in TS, TBH. It certainly seems like very strange behaviour from TS.

@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented Nov 15, 2024

I agree with @bradzacher that this looks like a TS bug. I would add, from playing with different TS versions in the playground, it appears that this behavior was introduced with 5.1.

Seems like the next step here would be to get input from TS team to see if an unintentional regression occurred that can be fixed upstream... The keywords for this are "evolving any"

@kirkwaiblinger kirkwaiblinger changed the title Bug: [no-unnecessary-type-assertion] Incorrect handling of unnecessary type assertions and undefined type checks Bug: [no-unnecessary-type-assertion] false positive on evolving type Nov 15, 2024
@bradzacher
Copy link
Member

There's another weird bit of behaviour I just noticed - when noExplicitAny is false, then TS never evolves the any and it's always any!!! playground

@bradzacher bradzacher added external This issue is with another package, not typescript-eslint itself and removed triage Waiting for team members to take a look labels Nov 16, 2024
@kirkwaiblinger
Copy link
Member

@bradzacher do you mean noImplicitAny? This is expected https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-1.html#improved-any-inference

@bradzacher
Copy link
Member

Filed microsoft/TypeScript#60514
Will close this for now.
We can reopen this if the result of that issue is working as intended and we can figure out a path forward.

@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Nov 24, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

3 participants