-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Bug: [no-misused-promises] An array of promises passed with spread (...args) syntax is not checked #5744
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
Comments
I think this may have been fixed. When I click on the playground link, I do see an error as described in the Expected Result. |
Looking a little deeper, it looks like the test case in the issue is checked because in a scenario like declare const f: (...args: Array<() => void>);
declare argsList: Array<() => Promise<void>>;
f(...argsList) the rule will ask "what is the type of typescript-eslint/packages/eslint-plugin/src/rules/no-misused-promises.ts Lines 672 to 683 in 0ca3a11
So, the test cases function consume(..._callbacks: Array<() => void>): void { }
// array
let cbs = [() => Promise.resolve(true), () => Promise.resolve(true)];
consume(...cbs) and function consume(..._callbacks: Array<() => void>): void { }
// tuple
const cbs = [() => Promise.resolve(true), () => Promise.resolve(true)] as const;
consume(...cbs) pass just fine. This does reveal though, the rule isn't too careful about checking unions of call signatures, so something like the following will fail to report, though it would be possible to detect. function consume(...maybeCallbacks: Array<(() => void) | number>): void { }
const cbs = [() => Promise.resolve(true), 3] as const;
consume(...cbs) That's not isolated to spread arguments; we can just as easily trick the rule with function consume(...maybeCallbacks: Array<(() => void) | number>): void { }
declare const cb: (() => Promise<boolean>) | 3
consume(cb) However, the use of spread args might be a more compelling case to address it than on its own. Furthermore, while the example in the issue does pass, there is no test case at present to ensure that it passes. So, I'd see as actionable
Would maintainers agree with these actions? |
Yup, thanks for the deep dive @kirkwaiblinger! All sounds good to me. 🚀 |
Before You File a Bug Report Please Confirm You Have Done The Following...
Playground Link
https://typescript-eslint.io/play/#ts=4.8.4&sourceType=module&code=GYVwdgxgLglg9mABBBBnEBbApgCgHQED6EAhgDZkBGJEA1qgFyICCATqyQJ4A8OAlIgC8APkQA3ODAAmwvkwnTEAb0QBfAFDqyWKMkqohiANr8hogAqs4GGKix5WWVHDJjcUViCx8ANIlMiiJbWtvaOzq7unt4AugDc6ihg6Nj4BBD6fEA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1oFtLlZkiACa1i0Dr0GoMkRNHHRI4MAF8QKoA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA
Repro Code
ESLint Config
tsconfig
No response
Expected Result
I expected that the 4th line should report "Promise returned in function argument where a void return was expected.", just as if I manually called the function with each value from the array (
consume(cbs[0], consume(cbs[1])
)Actual Result
There was no error reported on the fourth line.
Additional Info
Handling of 'rest' parameters is implemented in #5731 . However, support for an explicit 'spread' argument still needs to be added.
Versions
@typescript-eslint/eslint-plugin
5.39.0
@typescript-eslint/parser
5.39.0
TypeScript
4.8.4
ESLint
8.15.0
node
web
The text was updated successfully, but these errors were encountered: