Add fix to rxjs-no-explicit-generics rule

[Malvolio]

If the code says for example new BehaviorSubject<number>(1), the lint message should suggest correcting it to new BehaviorSubject(1)

[cartant]

Thanks. I'll give this some thought. I'm not sure that the fixer can always be applied. There are some subtleties. For example, changing this:

BehaviorSubject<string[]>([])

to this:

BehaviorSubject([])

would change the type of the subject. It would need to be corrected to:

BehaviorSubject([] as string[])

[Malvolio]

Nicholas, yes, that is almost certainly the case. I considered putting in exactly the fix you requested but either I would have to allow new BehaviorSubject<number>(1) to be fixed as new BehaviorSubject(1 as number) (ick) or make the fixer sophisticated enough to check that the types given in the explicit generic is exactly the same as the type given by the argument.

Is that even possible?

If it is, would it still be possible in the case of a.pipe(scan<number, string>((acc, value) => ${acc} + String(value), "")?

[Malvolio]

(Incidentally, I put this change in at the request of Ben Lesh, who has some specific need for it.)

[cartant]

I think it would be best to make the scope of the fixer as narrow as possible - i.e. white list it by looking for [] or {} arguments - so that it's only applied when it's known to be safe. I agree that BehaviorSubject(1 as number) would be horrible.

This is why, in general, I'm reluctant to implement fixers: it's too hard to handle all of the cases.

[cartant]

My other concern is that when fixers are enabled, TSLint seems to enable them for all rules that have them. Is that the case, as you see it, or have I missed something? If that is how TSLint works, the fixers have to be absolutely solid.

[cartant]

And, IIRC, the fix for the scan use case would need to be:

a.pipe(scan((acc: string, value: number) => ${acc} + String(value), "")

So this really is tricky.

[cartant]

BTW, I would be happy to have the rule fix situations that are known to be safe and simply flag others as failures. That's what the no-unused-declaration rule in tslint-etc does: it removes unused imports, but never removes unused code - as it would the soul-crushingly bad if it removed some code you'd just written written but hadn't yet used.

[xiaoxiangmoe]

Here is a unsafe example after fix it into as cask

const a = new BehaviorSubject<{a:number,b:string}>({a:2333}) // error,   Property 'b' is missing in type '{ a: number; }' but required in type '{ a: number; b: string; }'

fix it into as cask

const a = new BehaviorSubject({a:2333} as {a:number,b:string}) // no error

[cartant]

@xiaoxiangmoe FYI, in 4.82.1, the enforcing of no-explicit-type-arguments was relaxed when literal arguments are specified.

[xiaoxiangmoe]

@cartant I don't think it is a good practice to relaxed when literal arguments are specified.

for example:

const a = new BehaviorSubject<{a:number,b:string}>({a:2333, b: '2333'})

this type arguments are useless.

also

const a = new BehaviorSubject<123|234>(123)

this should be allowed.

---

I think we should check whether the derivation result without the generic parameter is consistent with the added derivation result. If it is consistent, it is redundant.

[cartant]

@xiaoxiangmoe

I think we should check whether the derivation result without the generic parameter is consistent with the added derivation result. If it is consistent, it is redundant.

I don't believe that this is possible - well, feasible - as the APIs required are private. See this issue:

microsoft/TypeScript#9943

The author of tsd gets around this by including a modified fork of TypeScript within the tool. Here, that's not an option, as the TypeScript dependency lies with TSLint (or, more specifically, with the project being linted).

Relaxing the rule for literals is a compromise largely because of the behaviour outlined in the comments above - that is, as functions as a type assertion that does not enforce type requirements.

The reality of the situation is that usage will typically look like this - and not like the snippets in the fixtures or in your comment:

const b = new BehaviorSubject<string[]>([]);

or this:

const b = new BehaviorSubject<Person>({ name: "alice" });

And, yes, I suppose it should also include string, number and Boolean literals.