Skip to content

Bug: [no-throw-literal] False positive report on Readonly<Error> #8233

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.

Sign up for GitHub

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

Jump to bottom
Closed
4 tasks done
auvred opened this issue Jan 9, 2024 · 4 comments
Closed
4 tasks done

Bug: [no-throw-literal] False positive report on Readonly<Error> #8233

auvred opened this issue Jan 9, 2024 · 4 comments
Labels
bug Something isn't working 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 wontfix This will not be worked on working as intended Issues that are closed as they are working as intended

Comments

@auvred
Copy link
Member

auvred commented Jan 9, 2024 •
edited
Loading

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.3.3&fileType=.tsx&code=CYUwxgNghgTiAEYD2A7AzgF3gMyUgXPAEohTCoQCeAPAKIwxIwB8AUBgBaMDuOeQA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1vwAtoOA7rUb5E0AIbx0UMf2iRwYAL4glQA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

declare const foo: Readonly<Error>
throw foo

ESLint Config

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/no-throw-literal": "error",
  },
};

tsconfig

{
  "compilerOptions": {
    // ...
  }
}

Expected Result

Should not report on Readonly<Error>

Actual Result

False positive report on Readonly<Error>

Expected an error object to be thrown.

Additional Info

Followup for #8011 (comment)
@auvred auvred 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 Jan 9, 2024
@bradzacher bradzacher added evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important and removed triage Waiting for team members to take a look labels Jan 9, 2024
@bradzacher
Copy link
Member

In theory this is a bug but idk if it is actually something people will ever hit in practice.

Its just not a common thing to pass around immutable error instances - it's even rare to pass error instances around much!

@auvred
Copy link
Member Author

auvred commented Jan 10, 2024 •
edited
Loading

Yes, I agree, but we already implemented this for prefer-promise-reject-errors rule (playground). It would be nice to synchronize these rules.

Also, to support Readonly<Error> we just need to call isReadonlyErrorLike in this line

typescript-eslint/packages/eslint-plugin/src/rules/no-throw-literal.ts

Line 83 in c3767ed

if (isErrorLike(services.program, type)) {

So this is a pretty trivial fix!

@JoshuaKGoldberg
Copy link
Member

A Sourcegraph query for Readonly<Error> outside of this repo shows about 100 examples on github.com. Which isn't a huge amount, but I suppose shows that some people do have this...

I'm slightly leaning towards accepting this given how it technically is correct and is a pretty small change (#8233 (comment)). I can see projects that enforce deep immutability benefiting from it.

@bradzacher
Copy link
Member

7 months later and this request has received 0 community reactions.
As per our workflow guidelines - we are taking this as signal that this check is not important to the broader community and are thus rejecting the proposal. Thanks for filing!

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2024
@bradzacher bradzacher added working as intended Issues that are closed as they are working as intended wontfix This will not be worked on and removed evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important labels Aug 25, 2024
@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 Sep 2, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working 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 wontfix This will not be worked on working as intended Issues that are closed as they are working as intended
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@JoshuaKGoldberg @bradzacher @auvred