feat(eslint-plugin): [no-floating-promises] add checkThenables option

[JoshuaKGoldberg]

PR Checklist

• Addresses an existing open issue: fixes Enhancement: [no-floating-promises] Don't check coincidentally Promise-like (Thenable) type values by default #8433
• That issue was marked as accepting prs
• Steps in Contributing were taken

Overview

Adds a new checkThenables boolean option to the rule that enables checking all shapes for matching the .then() interface. It essentially gates the existing "does this have a .then() with two parameters?" logic behind the option.

checkThenables is true by default in this PR so as to not cause a breaking change. If this PR goes in roughly as-is, I'd like to file a followup issue for v8 to turn it to false by default.

#8433 also mentioned adding in an option for adding additional types to what's flagged by default. I haven't heard anybody mention this as a strong need, so for now I'm holding off on adding that.

💖

[typescript-eslint]

Thanks for the PR, @JoshuaKGoldberg!

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]

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	dac381c
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/668996765b425e00089b41b5
😎 Deploy Preview	https://deploy-preview-9263--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 96 (🟢 up 2 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 90 (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]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit dac381c. 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

---

🟥 Failed Commands
nx test eslint-plugin --coverage=false

✅ Successfully ran 29 targets

• nx test typescript-eslint --coverage=false
• nx test typescript-estree --coverage=false
• nx test visitor-keys --coverage=false
• nx run types:build
• nx test type-utils --coverage=false
• nx test scope-manager --coverage=false
• nx run repo-tools:postinstall-script
• nx test utils --coverage=false
• nx test rule-schema-to-typescript-types --coverage=false
• nx test parser --coverage=false
• nx test repo-tools --coverage=false
• nx run-many --target=lint --parallel --exclude eslint-plugin
• nx test ast-spec --coverage=false
• nx test eslint-plugin-internal --coverage=false
• nx run-many --target=build --parallel --exclude=website --exclude=website-eslint
• nx test visitor-keys
• nx test typescript-estree
• nx test utils
• nx test repo-tools
• nx test rule-schema-to-typescript-types
• nx test typescript-eslint
• nx test parser
• nx test type-utils
• nx test scope-manager
• nx test eslint-plugin-internal
• nx test ast-spec
• nx run-many --target=typecheck --parallel
• nx run integration-tests:test
• nx run repo-tools:generate-configs

---

Sent with 💌 from NxCloud.

[Josh-Cena]

I don't get the motivation for this, TBH. How can something "happen to be thenable" without noticing? Does then have any other meaning other than interoperating with promises? JavaScript itself resolves thenables, so if your thenable is not supposed to quack like a promise you will shoot yourself in the foot everywhere, like returning it from an async function, etc.

[kirkwaiblinger]

[JoshuaKGoldberg]

How can something "happen to be thenable" without noticing?

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/e40cab53d9971a3e0b34e8fb18ece74240d9425d/types/bluebird/index.d.ts#L59: older projects that have polyfills which generally intentionally match something like Promises/A+.

[Josh-Cena]

Why would one expect a polyfill to not be checked like a real promise?

[JoshuaKGoldberg]

One example I've believe I've been told is that if a framework implements its own classes that happen to have a similar API to Promises, it's inconvenient to have the rule fire on them by default.

cc @mcollina as I think this was from you and Fastify, but I might be mistaken / mistinterpreting.

[Josh-Cena]

The issue mentions node:test, but doesn't test() return a real promise?

[JoshuaKGoldberg]

@Josh-Cena the Node issue does, but our #8433 -> this PR isn't around that I don't think.

[Josh-Cena]

That issue doesn't mention a valid use case where you want a thenable to not be checked like a promise though. It just mentions polyfills but those are why we check thenables.

[JoshuaKGoldberg]

Great points. Putting into draft per #8433 (comment). Thanks!

[JoshuaKGoldberg]

#8433 was confirmed as good to go, so un-drafting. 👍

[kirkwaiblinger]

Code makes sense to me! only blocker is some test coverage

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.24%. Comparing base (dfae228) to head (6078b07).
Report is 111 commits behind head on main.

❗ Current head 6078b07 differs from pull request most recent head dac381c

Please upload reports for the commit dac381c to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9263      +/-   ##
==========================================
+ Coverage   87.42%   88.24%   +0.81%     
==========================================
  Files         261      416     +155     
  Lines       12724    14380    +1656     
  Branches     3975     4195     +220     
==========================================
+ Hits        11124    12689    +1565     
- Misses       1319     1372      +53     
- Partials      281      319      +38     

Flag	Coverage Δ
unittest	88.24% <100.00%> (+0.81%)	⬆️

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

Files	Coverage Δ
...es/eslint-plugin/src/rules/no-floating-promises.ts	100.00% <100.00%> (ø)

... and 187 files with indirect coverage changes

[kirkwaiblinger]

[JoshuaKGoldberg]

Confirmed with @Josh-Cena that we're good to merge with those docs suggestions. 🚀!