fix(eslint-plugin): [await-thenable, return-await] don't flag awaiting unconstrained type parameter as unnecessary #10314

ronami · 2024-11-09T15:21:16Z

PR Checklist

Addresses an existing open issue: fixes Bug: [await-thenable] Don't flag values of an unconstrained type parameter #10311
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

This PR resolves #10311 and stops reporting on some type constraints (see playground link).

As per @kirkwaiblinger's comment, I shared common logic between await-thenable and return-await, so this fixes similar issues with both (playground link for return-await).

On top of not reporting on unconstrained type parameters, this also stops reporting on type parameters with valid constraints like unknown or any (not part of the original issue); please let me know if this is OK.

// type parameter with no constraint
async function test1<T>(value: T) {
  return await value;
}

// type parameter with `any` or `unknown` constraint
async function test2<T extends unknown>(value: T) {
  return await value;
}

async function test3<T extends any>(value: T) {
  return await value;
}

// direct `any` or `unknown` type pass
async function test4(value: unknown) {
  return await value;
}

async function test5(value: any) {
  return await value;
}

typescript-eslint · 2024-11-09T15:21:19Z

Thanks for the PR, @ronami!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

netlify · 2024-11-09T15:21:32Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`aa274e9`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/673250d6780aa90008b9c765
😎 Deploy Preview	https://deploy-preview-10314--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (no change from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

nx-cloud · 2024-11-09T15:22:07Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit aa274e9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 1 target

nx run-many --target=clean

Sent with 💌 from NxCloud.

codecov · 2024-11-09T15:35:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.58%. Comparing base (124b65d) to head (aa274e9).
Report is 23 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10314      +/-   ##
==========================================
+ Coverage   86.56%   86.58%   +0.01%     
==========================================
  Files         431      432       +1     
  Lines       15188    15145      -43     
  Branches     4418     4424       +6     
==========================================
- Hits        13148    13113      -35     
+ Misses       1683     1675       -8     
  Partials      357      357

Flag	Coverage Δ
unittest	`86.58% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/eslint-plugin/src/rules/await-thenable.ts	`100.00% <100.00%> (ø)`
packages/eslint-plugin/src/rules/return-await.ts	`97.41% <100.00%> (-0.03%)`	⬇️
...ackages/eslint-plugin/src/util/needsToBeAwaited.ts	`100.00% <100.00%> (ø)`

... and 27 files with indirect coverage changes

JoshuaKGoldberg

kirkwaiblinger

Just minor changes requested. Looks great!

packages/eslint-plugin/src/rules/await-thenable.ts

packages/eslint-plugin/src/util/needsToBeAwaited.ts

Co-authored-by: Kirk Waiblinger <kirk.waiblinger@gmail.com>

@developer-bandi

##### [v8.15.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8150-2024-11-18) ##### 🚀 Features - **eslint-plugin:** \[prefer-nullish-coalescing] fix detection of `ignoreConditionalTests` involving boolean `!` operator ([#10299](typescript-eslint/typescript-eslint#10299)) - **eslint-plugin:** new rule `no-unsafe-type-assertion` ([#10051](typescript-eslint/typescript-eslint#10051)) - **eslint-plugin:** added related-getter-setter-pairs rule ([#10192](typescript-eslint/typescript-eslint#10192)) ##### 🩹 Fixes - **utils:** add defaultOptions to meta in rule ([#10339](typescript-eslint/typescript-eslint#10339)) - **eslint-plugin:** report deprecations used in default export ([#10330](typescript-eslint/typescript-eslint#10330)) - **eslint-plugin:** \[explicit-module-boundary-types] and \[explicit-function-return-type] don't report on `as const satisfies` ([#10315](typescript-eslint/typescript-eslint#10315)) - **eslint-plugin:** \[await-thenable, return-await] don't flag awaiting unconstrained type parameter as unnecessary ([#10314](typescript-eslint/typescript-eslint#10314)) - **eslint-plugin:** \[consistent-indexed-object-style] handle circular mapped types ([#10301](typescript-eslint/typescript-eslint#10301)) ##### ❤️ Thank You - Josh Goldberg ✨ - Kim Sang Du [@developer-bandi](https://github.com/developer-bandi) - Luis Sebastian Urrutia Fuentes [@LuisUrrutia](https://github.com/LuisUrrutia) - Phillip Huang - Ronen Amiel - Szydlak [@wszydlak](https://github.com/wszydlak) You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.

@developer-bandi

| datasource | package | from | to | | ---------- | -------------------------------- | ------ | ------ | | npm | @typescript-eslint/eslint-plugin | 8.14.0 | 8.15.0 | | npm | @typescript-eslint/parser | 8.14.0 | 8.15.0 | ## [v8.15.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8150-2024-11-18) ##### 🚀 Features - **eslint-plugin:** \[prefer-nullish-coalescing] fix detection of `ignoreConditionalTests` involving boolean `!` operator ([#10299](typescript-eslint/typescript-eslint#10299)) - **eslint-plugin:** new rule `no-unsafe-type-assertion` ([#10051](typescript-eslint/typescript-eslint#10051)) - **eslint-plugin:** added related-getter-setter-pairs rule ([#10192](typescript-eslint/typescript-eslint#10192)) ##### 🩹 Fixes - **utils:** add defaultOptions to meta in rule ([#10339](typescript-eslint/typescript-eslint#10339)) - **eslint-plugin:** report deprecations used in default export ([#10330](typescript-eslint/typescript-eslint#10330)) - **eslint-plugin:** \[explicit-module-boundary-types] and \[explicit-function-return-type] don't report on `as const satisfies` ([#10315](typescript-eslint/typescript-eslint#10315)) - **eslint-plugin:** \[await-thenable, return-await] don't flag awaiting unconstrained type parameter as unnecessary ([#10314](typescript-eslint/typescript-eslint#10314)) - **eslint-plugin:** \[consistent-indexed-object-style] handle circular mapped types ([#10301](typescript-eslint/typescript-eslint#10301)) ##### ❤️ Thank You - Josh Goldberg ✨ - Kim Sang Du [@developer-bandi](https://github.com/developer-bandi) - Luis Sebastian Urrutia Fuentes [@LuisUrrutia](https://github.com/LuisUrrutia) - Phillip Huang - Ronen Amiel - Szydlak [@wszydlak](https://github.com/wszydlak) You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.

ronami added 6 commits November 9, 2024 17:00

initial implementation

2fa678f

share code with return-await

8dbfec1

tests

169f100

small changes

92fb850

remove unnecessary spaces

18f3e7b

adjust tests

fa7e350

adjust tests

6481ef0

ronami marked this pull request as ready for review November 9, 2024 15:54

JoshuaKGoldberg previously approved these changes Nov 10, 2024

View reviewed changes

JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Nov 10, 2024

kirkwaiblinger changed the title ~~fix(eslint-plugin): [await-thenable] don't flag values of an unconstrained type parameter~~ fix(eslint-plugin): [await-thenable, return-await] don't flag awaiting unconstrained type parameter as unnecessary Nov 11, 2024

kirkwaiblinger mentioned this pull request Nov 11, 2024

Add getUnknownType to checker api microsoft/TypeScript#60475

Closed

6 tasks

kirkwaiblinger requested changes Nov 11, 2024

View reviewed changes

packages/eslint-plugin/src/rules/await-thenable.ts Show resolved Hide resolved

packages/eslint-plugin/src/util/needsToBeAwaited.ts Outdated Show resolved Hide resolved

Update packages/eslint-plugin/src/util/needsToBeAwaited.ts

3b7cf26

Co-authored-by: Kirk Waiblinger <kirk.waiblinger@gmail.com>

ronami dismissed JoshuaKGoldberg’s stale review via 3b7cf26 November 11, 2024 18:42

ronami added 2 commits November 11, 2024 20:43

remove unnecessary code

7b3b64f

test a case with a generic whose constraint is another generic

aa274e9

ronami requested a review from kirkwaiblinger November 11, 2024 19:11

github-actions bot removed the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Nov 11, 2024

bradzacher added the bug Something isn't working label Nov 16, 2024

bradzacher approved these changes Nov 16, 2024

View reviewed changes

bradzacher merged commit 013c5cc into typescript-eslint:main Nov 16, 2024
64 of 65 checks passed

ronami deleted the await-thenable-no-type-constraint branch November 16, 2024 04:16

github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(eslint-plugin): [await-thenable, return-await] don't flag awaiting unconstrained type parameter as unnecessary #10314

fix(eslint-plugin): [await-thenable, return-await] don't flag awaiting unconstrained type parameter as unnecessary #10314

ronami commented Nov 9, 2024 •

edited

Loading

typescript-eslint bot commented Nov 9, 2024

netlify bot commented Nov 9, 2024 •

edited

Loading

nx-cloud bot commented Nov 9, 2024 •

edited

Loading

codecov bot commented Nov 9, 2024 •

edited

Loading

JoshuaKGoldberg left a comment

kirkwaiblinger left a comment

fix(eslint-plugin): [await-thenable, return-await] don't flag awaiting unconstrained type parameter as unnecessary #10314

fix(eslint-plugin): [await-thenable, return-await] don't flag awaiting unconstrained type parameter as unnecessary #10314

Conversation

ronami commented Nov 9, 2024 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Nov 9, 2024

netlify bot commented Nov 9, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Nov 9, 2024 • edited Loading

☁️ Nx Cloud Report

codecov bot commented Nov 9, 2024 • edited Loading

Codecov Report

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

kirkwaiblinger left a comment

Choose a reason for hiding this comment

ronami commented Nov 9, 2024 •

edited

Loading

netlify bot commented Nov 9, 2024 •

edited

Loading

nx-cloud bot commented Nov 9, 2024 •

edited

Loading

codecov bot commented Nov 9, 2024 •

edited

Loading