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-misused-promises] checksVoidReturn=true doesn't catch the error when variadic arguments are used (like ...handlers: Array<() => void>) #4015

Closed
3 tasks done
dko-slapdash opened this issue Oct 16, 2021 · 1 comment · Fixed by #5731
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

@dko-slapdash
Copy link

dko-slapdash commented Oct 16, 2021

This issue affects e.g. express used with new route handlers which are required to be non-async in their TS typings. I think in case of variadic arguments, the plugin verifies just the 1st argument and, if it's okay, doesn't even check the rest of the arguments.

  • 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-misused-promises": "error",
  }
}
function get2(_h1: () => void, _h2: () => void) {}

function get(..._handlers: Array<() => void>) {}

get2(
  () => {},
  async () => {} // <-- fails, which is what we want
);

get(
  () => {},
  async () => {}  // <-- this must fail too, but it does not
);

Expected Result

Both get() and get2() calls must raise an eslint error.

Actual Result

Only get2() is treated as incorrect, and get() is fine - although it should fail too:

image

Additional Info

Versions

package version
@typescript-eslint/eslint-plugin 5.0.0
@typescript-eslint/parser 5.0.0
TypeScript 4.4.4
ESLint 8.0.1
node 16.11.1
@dko-slapdash dko-slapdash added package: eslint-plugin-tslint Issues related to @typescript-eslint/eslint-plugin-tslint triage Waiting for maintainers to take a look labels Oct 16, 2021
@dko-slapdash
Copy link
Author

dko-slapdash commented Oct 16, 2021

Here is BTW what I have to do to work-around this eslint issue with variadic parameters:

image

(Originally in express-serve-static-core, they're defined as methods with variadic ...handlers: Array<RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>>).

This allows to write something like

import asyncHandler from "express-async-handler";
...
app.get(
  AppRoutes.connectApp.pattern,
  asyncHandler(requireAuth),
  asyncHandler(connectAppRoute)
);

and have peace in mind that, if we accidentally forget asyncHandler wrapper for connectAppRoute, no-misused-promises will warn us.

@dko-slapdash dko-slapdash changed the title [no-misused-promises] checksVoidReturn=true doesn't catch when variadic arguments are used (like ...handlers: Array<() => void>) [no-misused-promises] checksVoidReturn=true doesn't catch the error when variadic arguments are used (like ...handlers: Array<() => void>) Oct 16, 2021
@bradzacher bradzacher added bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin and removed package: eslint-plugin-tslint Issues related to @typescript-eslint/eslint-plugin-tslint triage Waiting for maintainers to take a look labels Oct 18, 2021
@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
Aaron1011 added a commit to Aaron1011/typescript-eslint that referenced this issue Oct 3, 2022
Fixes typescript-eslint#4015

This extends 'no-misued-promises' with support for checking
variadic arguments passed to a 'rest' parameter. If a function
is declared with an argument like '...handlers: Array<() => void>',
we now check if the type argument to `Array` is a void-returning function,
and if so, check if any of the variadic arguments return a Promise.
Aaron1011 added a commit to Aaron1011/typescript-eslint that referenced this issue Oct 3, 2022
Fixes typescript-eslint#4015

This extends 'no-misued-promises' with support for checking
variadic arguments passed to a 'rest' parameter. If a function
is declared with an argument like '...handlers: Array<() => void>',
we now check if the type argument to `Array` is a void-returning function,
and if so, check if any of the variadic arguments return a Promise.
JoshuaKGoldberg added a commit that referenced this issue Oct 7, 2022
…5731)

* feat(eslint-plugin): Check 'rest' parameters in no-misused-promises

Fixes #4015

This extends 'no-misued-promises' with support for checking
variadic arguments passed to a 'rest' parameter. If a function
is declared with an argument like '...handlers: Array<() => void>',
we now check if the type argument to `Array` is a void-returning function,
and if so, check if any of the variadic arguments return a Promise.

* Address review comments

* Fix spelling

* Address additional review comments

* nit: split up tests a bit, and add comments about ()

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Oct 13, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.39.0` -> `5.40.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.39.0/5.40.0) |
| [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.39.0` -> `5.40.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.39.0/5.40.0) |

---

### Release Notes

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/eslint-plugin)</summary>

### [`v5.40.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;5400-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5390v5400-2022-10-10)

[Compare Source](typescript-eslint/typescript-eslint@v5.39.0...v5.40.0)

##### Bug Fixes

-   **eslint-plugin:** \[consistent-indexed-object-style] handle interface generic ([#&#8203;5746](typescript-eslint/typescript-eslint#5746)) ([7a8a0a3](typescript-eslint/typescript-eslint@7a8a0a3))
-   **eslint-plugin:** \[no-unnecessary-condition] handle void ([#&#8203;5766](typescript-eslint/typescript-eslint#5766)) ([ac8f06b](typescript-eslint/typescript-eslint@ac8f06b))

##### Features

-   **eslint-plugin:** Check 'rest' parameters in no-misused-promises ([#&#8203;5731](typescript-eslint/typescript-eslint#5731)) ([6477f38](typescript-eslint/typescript-eslint@6477f38)), closes [#&#8203;4015](typescript-eslint/typescript-eslint#4015)
-   **utils:** add dependency constraint filtering for `RuleTester` ([#&#8203;5750](typescript-eslint/typescript-eslint#5750)) ([121f4c0](typescript-eslint/typescript-eslint@121f4c0))

</details>

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/parser)</summary>

### [`v5.40.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;5400-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5390v5400-2022-10-10)

[Compare Source](typescript-eslint/typescript-eslint@v5.39.0...v5.40.0)

**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yMjYuMCIsInVwZGF0ZWRJblZlciI6IjMyLjIyNi4wIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1580
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 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

Successfully merging a pull request may close this issue.

3 participants