False positive for no-misused-generics

[danvk]

Here's a generic function:

/**
 * Call two functions with the same args, e.g.
 *
 * onClick={both(action('click'), setState)}
 */
export function both<
  Args extends unknown[],
  CB1 extends (...args: Args) => void,
  CB2 extends (...args: Args) => void
>(fn1: CB1, fn2: CB2): (...args: Args) => void {
  // See https://stackoverflow.com/a/62093430/388951
  return function (...args: Args) {
    fn1(...args);
    fn2(...args);
  };
}

I think this is fine in terms of every type parameter appearing multiple times. Args only appears in other extends clauses, but it can still be inferred. Here's an example of it being used, with all the parameters inferred:

const noop = () => {};
const fnWithCallback = (cb: (a: string, b: number) => void) => _.noop;

fnWithCallback(
  both((a, b) => {
    a;  // type is string
    b;  // type is number
    return a * b;  // error
  }, _.noop),
);

Mousing over both in that example, I see:

const both: <[string, number], (a: string, b: number) => number, (...args: any[]) => void>(fn1: (a: string, b: number) => number, fn2: (...args: any[]) => void) => (a: string, b: number) => void

In other words, Args is very much inferred.

[cartant]

I'm not convinced that this is a bug or a false positive.

Rather, I think this is one of the exceptions to the rule. As with operators and pipe in RxJS - which will also fail this rule - here, the inference is driven via the return type - as what's returned from both is passed to a function - fnWithCallback - and it's that function's signature that drives the inference.

If both is called and the result is assigned to a variable, there is no inference and the usage is not type-safe:

const b = both((name: string) => {}, (age: number) => {}); // (...args: unknown[]) => void

IMO, this is an example of a situation in which the rule is inappropriate and I don't see how the rule could cope with these situations - given that the declaration and the call sites are separated, so to speak.

Thoughts?

[cartant]

One option could be a specific failure message that states that inference is only possible via the return value.

[danvk]

That's a good point @cartant. The error would have to be at the call site, not the definition. Or perhaps this is just a case where disabling the linter for that line makes sense.

[OliverJAsh]

As with operators and pipe in RxJS

Yes, I'm running into this a lot with operators. Not just for RxJS but all the types in fp-ts.

Reduced test case:

const fn = <E>() => (e: E): E => e;

Maybe this lint rule should only be concerned with the definition, not the call site (we could have a separate lint rule for that). In that case, this lint rule would determine this is a safe definition, because the generic is used in two positions: a parameter and the return type.

[cartant]

@OliverJAsh ATM, I have little appetite for tweaking this rule, as I didn't write it; I just ported it. I've removed it from the recommended configuration, as I think there are too many exceptions and I don't want to support/explain them.

In the example, E is only 'used twice' because the return type is a function type in which it's used twice. However, if that function type wasn't inlined, but was something like RxJS's MonoTypeOperatorFunction<T>, that type itself would have to be found and checked. There is a similar problem with other contextual types too - like in this issue. (And there's a link, IIRC, to the Wotan issue - in the comment above the linked-to comment - which is labelled as "won't fix".)

[danvk]

3+ years later, I see that my both function can easily be rewritten to not break this rule:

export function both<Args extends unknown[]>(
  fn1: (...args: Args) => void,
  fn2: (...args: Args) => void,
): (...args: Args) => void {
  return function (...args: Args) {
    fn1(...args);
    fn2(...args);
  };
}

Maybe TypeScript has gotten smarter since 2020? Or maybe this was just working as intended all along!