Bug: [no-floating-promises] False negative calling .then with second argument undefined

[kirk-waiblinger-trimble]

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.0.2&sourceType=module&code=PTAEGEHsCdoUwMYBcA2BPUAzFBDA5nnACYCwAUAArSQC2AlgM5wB08AVokgBQCUzSACzgA7Lr1ABeAHygA3qAC+PciFAAVAY1BbBcUAxw09gusLzbhoAFIBlZqACSSfUjooU2hqByWArsIEfIhRiUHZOOkhhZnIqWkYWcORefiFRcWk5RQAaUH8iOExTYh4gA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1oDN4OBDfMwDmtYtA4BbSshTooiaBOiRwYAL4h1QA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA

Repro Code

Promise.reject().then(() => { }, undefined)

ESLint Config

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/no-floating-promises": ["error"],
  },
};

tsconfig

No response

Expected Result

I expect that .then(f, undefined) should be flagged as an error the same as .then(f)

Actual Result

No error

Additional Info

No response

[JoshuaKGoldberg]

Nice spot - agreed. If the second argument to .then is a potentially null or undefined, we'd still want the rule to file a complaint.

I think we'd want to special-case this error though. How about this?

Rejection handler is (potentially?) (null|undefined). This Promise may become 'floating' if a nullable value is provided as a rejection handler.

Seeking bike-shedding on that error text. I'm not super satisfied with how that one repeats itself...

[kirk-waiblinger-trimble]

I have no strong opinion on this. Just a thought, though, it might be worth explaining why it would become floating in that case. What about including in the message something like the phrase The Promise constructor treats a nullish rejection handler the same as an omitted one.?

I would imagine though anyone reading it will make the connection with or without the hint, though.

[kirk-waiblinger-trimble]

would you mind if I try my hand at a PR for this (pending consensus on error message)?

[Josh-Cena]

Not just nullish handlers; but anything non-callable.

[JoshuaKGoldberg]

Go ahead @kirk-waiblinger-trimble! 🚀

[kirk-waiblinger-trimble]

Not just nullish handlers; but anything non-callable.

Good point... I hadn't thought to check since the TS declarations only allow null | undefined | () => PromiseLike<T>. Is the convention in the Typescrpt-Eslint codebase to acknowledge all JS possibilities even if they're a TS error?

Maybe "If the rejection handler is not a function, it will be ignored." or "If the rejection handler is not a function, it will be treated the same as if it were omitted." would be more general?

[JoshuaKGoldberg]

acknowledge all JS possibilities even if they're a TS error?

I don't think we need to - TS giving an error is enough for users. No need for us to add an addition scream-in-your-face on top of it. It is nifty to know that JS natively ignores non-callable things though.

[Josh-Cena]

I think "If the rejection handler is not a function, it will be ignored." is much better than "if it is nullish". The latter is technically not wrong (because it's a sufficient condition), but it doesn't give the full information. The former is also not too complex.