Enhancement: [use-unknown-in-catch-callback-variable] Option to only check inline functions

[cobaltt7]

Before You File a Proposal Please Confirm You Have Done The Following...

• I have searched for related issues and found none that match my proposal.
• I have searched the current rule list and found no rules that match my proposal.
• I have read the FAQ and my problem is not listed.

My proposal is suitable for this project

• I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/use-unknown-in-catch-callback-variable/

Description

I propose an option to only check on inline functions, i.e. .catch((foo) => bar) and not .catch(baz). If baz accepts any, it's either from an external library or will be flagged by --noImplicitAny/no-explicit-any.

Use case: I have a few instances of .catch(console.error) in my codebase to log errors without throwing. console.error is typed as (...data: any[]): void, and as such this rule triggers an error.

Fail

Promise.resolve().catch((error) => error) // Preserve old behavior

Pass

Promise.resolve().catch((error: unknown) => error) // Preserve old behavior
Promise.resolve().catch(console.log) // Would previously error
Promise.resolve().catch(foo) // Would previously error

function foo(bar: any) { }

Additional Info

No response

[kirkwaiblinger]

Note: a related idea came up in the rule proposal, see #7526 (comment)

I personally don't have a strong opinion. Several points in favor and several points against spring to mind. We'll see what others say.

[bradzacher]

yeah I'm personally -1 on this given that the usecase of specifically passing an "external" function that accepts any is rare.
This is also trivially worked-around from the user side (eg switching to an explicit arrow .catch((arg: unknown) => console.error(arg))).

Now-a-days I think most 3rd-party things are going to accept unknown instead of any, and within your own codebase you can always switch the fn to unknown.

it's either from an external library or will be flagged by --noImplicitAny/no-explicit-any

Assuming that you have that compiler option turned on or that you have that rule turned on.
It's not uncommon to not use the latter -- a lot of people don't love how restrictive it is.

[Josh-Cena]

FWIW the only pain I had with adopting this rule is when I'm passing error-tracking utilities.

redisClient.connect().catch(winston.error);
redisClient.connect().catch(console.error);
redisClient.connect().catch(Sentry.captureException);

NONE OF THESE work.

So, I'm a big fan. If you are not implementing this function there's not much you can do anyway.

[kirkwaiblinger]

Having thought more thoroughly about this, I am +1 to the request as well. Passed in handlers will never have implicitly any parameters, which is the real problem with supplying a function expression as a handler; the parameter doesn't require an annotation because it's deduced as any

[JoshuaKGoldberg]

I don't feel strongly enough about this to want to push back against multiple 👍s. I'll defer to the rest of the team's majority. 🙂

[Josh-Cena]

Kirk and I both hold the thought that this is fine:

function handler(x: any) {
  // ...
}

Promise.reject().catch(handler);

It's still any but it's explicit. If one turns off no-explicit-any, I don't see why we should report this one but not any other blatantly unsafe things going on.

[bradzacher]

3 months later and this request has received 2 community reactions.
With the team split and low community support, I think we can take this as signal to reject this request.

There are documented workarounds:

• wrapping the calls with explicit .catch((arg: unknown) => cb(arg))
• patching the types for the 3rd party lib locally to use unknown

And this is not a super common usecase given the low volume of community engagement.
As per our workflow guidelines - we are taking this as signal that this check is not important to the broader community and are thus rejecting the proposal. Thanks for filing!

[JoshuaKGoldberg]

Coming over from #10435: I've experienced the pain of this in production and now believe it is worth it to implement.

wrapping the calls with explicit .catch((arg: unknown) => cb(arg))

This is annoying, unnecessary, and makes the rule overly pedantic

patching the types for the 3rd party lib locally to use unknown

Also inconvenient. And not always doable, e.g. with the common console.error.bind(console).

I'm now in favor.