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

[no-unnecessary-type-constraint] fixing as suggested can lead to a TS error #3509

Closed
3 tasks done
nikolai-katkov opened this issue Jun 9, 2021 · 3 comments
Closed
3 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@nikolai-katkov
Copy link

nikolai-katkov commented Jun 9, 2021

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
    "@typescript-eslint/no-unnecessary-type-constraint": "error"
  }
}

Consider the following example:

const maybe = <T extends any>(whatever: T): T => {
  console.log(Object.keys(whatever)) // TS is complaining, which is good
  return whatever
}

It's dummy but it has a problem: calling maybe(null) will throw a runtime error (TypeError: Cannot convert undefined or null to object). Therefore I'd like Typescript to fail here. It does now, all good so far.

The rule no-unnecessary-type-constraint suggests replacing <T extends any> with <T>, because "Constraining the generic type `T` to `any` does nothing and is unnecessary."

So I change it to:

const maybe = <T>(whatever: T): T => {
  console.log(Object.keys(whatever)))
  return whatever
}

Result: TS is silent, the code is not safe against a runtime error.

Versions

package version
@typescript-eslint/eslint-plugin ^4.25.0
@typescript-eslint/parser ^4.25.0
TypeScript ^4.2.4
ESLint ^7.27.0
node 14.16.0
@nikolai-katkov nikolai-katkov added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Jun 9, 2021
@bradzacher
Copy link
Member

I'll be honest when I say that this seems like a bug in TypeScript's generic logic.
I've filed an issue - microsoft/TypeScript#44520
For some reason an unconstrained generic is assignable to {} - which is not correct or safe, as T can be null | undefined.

T extends any (as of TS3.9) explicitly constrains T to unknown.
Which is why it works around this issue.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party bug Something isn't working and removed triage Waiting for maintainers to take a look awaiting response Issues waiting for a reply from the OP or another party labels Jun 9, 2021
@bradzacher
Copy link
Member

Looks like this is regrettably working as intended from the TS side.
So we need to remove the fixer from the rule.

@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
@bradzacher bradzacher changed the title [no-unnecessary-type-constraint] fixing as suggested can lead to a runtime error [no-unnecessary-type-constraint] fixing as suggested can lead to a TS error Feb 17, 2022
@Josh-Cena
Copy link
Member

This should have been fixed by #4901

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

4 participants