Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[no-misused-generics] False positives when contextual types are used #663

Closed
karol-majewski opened this issue Oct 22, 2019 · 2 comments
Closed

Comments

@karol-majewski
Copy link

Hello and thank you for your wonderful tool.

As I was working with RxJS, the no-misused-generics rule highlighted an error saying my type parameter cannot be inferred from arguments.

Example

We have an Observable<string> which is then piped using a custom operator, here called buffer.

declare const string$: RxJS.Observable<string>;

string$.pipe(buffer(1000));

The pipe method accepts functions of type OperatorFunction<T, R>.

Non-compliant implementation

If we try and define buffer using the OperatorFunction type provided by rxjs, it looks like this:

declare const buffer: <T>(ms: number) => RxJS.OperatorFunction<T, T[]>;

Because T bears no relation to ms, Wotan raises an error. However, our buffer function is required to return an OperatorFunction<T, R> which requires two type parameters and we need to provide them somehow. There is only one place we can declare T — the place it's defined here.

Despite the warning, this signature works as expected and T is inferred correctly.

Compliant implementation

We could make Wotan happy by inlining the definition instead of using OperatorFunction<T, R>.

declare const buffer: (ms: number) => <T>(input: RxJS.Observable<T>) => RxJS.Observable<T[]>;

The type parameter is defined next to the related argument, and everything works fine. Wotan is also happy. The problem is, we can't use the provided OperatorFunction<T, R> type anymore, so the implementation is not type-first anymore.


The big question: in such scenarios, should the programmer work around the warning by defining the function signature themselves, or should they be allowed to deliberately misplace the type parameter in order to feed the returned generic type that requires it?

I know it might be a case where disabling Wotan locally is justified (since the code works), but I wanted to bring that to your attention anyway. If handling such cases and making an exception for them is stupid or beyond the scope of this project, then please don't hesitate to close this issue.

A complete snippet
import * as RxJS from 'rxjs';

declare const bad: <T>(ms: number) => RxJS.OperatorFunction<T, T[]>;
declare const good: (ms: number) => <T>(input: RxJS.Observable<T>) => RxJS.Observable<T[]>;

declare const string$: RxJS.Observable<string>;

string$.pipe(bad(1000));
string$.pipe(good(1000));

I'm using "rxjs": "^6.4.0".

@ajafff
Copy link
Member

ajafff commented Jan 2, 2020

Thank your for the detailed write-up. Unfortunately there's no way to detect this pattern.
The rule is only inspecting the declaration of your function signature without any knowledge about the intended (and actual) use. I don't see a possibility to exempt some functions from this rule based on syntax (or types).

I'm going to close this as it's not actionable.
As you already noted there are two possibilities: disable the rule for that particular line or work around the warning by using a more verbose syntax.

@ajafff ajafff closed this as completed Jan 2, 2020
@OliverJAsh
Copy link

OliverJAsh commented Jan 3, 2020

With microsoft/TypeScript#17574, we could do something like:

declare const buffer: (ms: number) =>  <T> RxJS.OperatorFunction<T, T[]>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants