Rule proposal: no `as any` when type is already expected type
Before You File a Proposal Please Confirm You Have Done The Following...
- [X] I have searched for related issues and found none that match my proposal.
- [X] I have searched the current rule list and found no rules that match my proposal.
- [X] I have read the FAQ and my problem is not listed.
My proposal is suitable for this project
- [X] My proposal specifically checks TypeScript syntax, or it proposes a check that requires type information to be accurate.
- [X] My proposal is not a "formatting rule"; meaning it does not just enforce how code is formatted (whitespace, brace placement, etc).
- [X] I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).
Description
This proposal is for a rule that detects and can auto-fix casts to any, when the type is already the type that's expected in that situation.
It's semi-common in test code, to use as any to make a stub or similar pass type checks when passing as a function parameter, however this is hard to get away from once already done because it's hard to identify when it's made redundant.
My proposal is basically for the following situation to warn that the as any is a redundant cast, as the type already satisfies the required conditions.
function doThing(a: number) {
}
doThing(5 as any); // <-- Rule fails on this line, as `5` already satisfies the type of number
The main goal of this rule is to help cleanup erasure of type that's been added (mostly in test code) by making it more evident where as any calls can safely be removed, overall leading to improved type safety of a codebase.
A possible extension to this rule would be identifying the pattern of 5 as unknown as number or 5 as any as number, another common pattern used within tests to forcibly make stubs conform to a given type. However, that might also make sense as an extension to no-unnecessary-type-assertion as it also fits into "when a type assertion does not change the type of an expression" unlike the main case of this proposed rule.
Fail Cases
interface A {
required: string;
alsoRequired: number;
}
function doThing(a: A) {}
// Fails as this already fulfils the type requirement, the `as any` is not preventing a type error here
doThing({ required: 'yes', alsoRequired: 'alsoYes' } as any);
Pass Cases
interface A {
required: string;
alsoRequired: number;
}
function doThing(a: A) {}
// The rule accepts this code, as it does not fulfil the requirements and the `any` is masking a type error
doThing({ required: 'yes' } as any);
Additional Info
I can see an argument where making this a part of no-unnecessary-type-assertion would make sense, and am happy to switch this proposal to that if requested. Given the underlying "cause" is different though it felt like it made more sense to propose this as a separate rule.
Is as any a common practice across your codebase?
That seems like a pretty major whole that should be addressed instead of just removing the ones that are purely "unnecessary".
It's common amongst tests, usually for test data of complex types. I'm not super happy about it, but in my experience this is a fairly common pattern across tests in large codebases. I've been working on an initiative to improve test quality, and I feel a lint rule like this would assist significantly in situations like this where it's overused but there's a drive to improve test quality. Basically a "migration helper" type rule
I've also seen as unknown used in these cases. Or as Partial<TheActualType>. Taking a step back, I think there's a root need for no-unnecessary-type-assertion to report on any type assertion where the source type is already assignable to the target type. I.e. using the type assignability APIs from #7936. Thoughts?
We don't want to report upcasts because const x = true as boolean and stuff are common enough. A downcast is not an "unnecessary assertion" and reporting them should be a separate issue. The only thing we can report is if the original and final types are exactly equivalent (i.e. cross-assignable). However, anything is cross-assignable with any, so not sure how this can be helped 😅 Although the x as unknown as T pattern can be detected, if x is already of type T.
The idea for that sort of check would be you have v as any as T - so you would compare typeof v and T in this case and the report would say "remove the unnecessary as any".
You'd also do the check on code like (as suggested in this issue) fn(v as any) or const x: T = v as any (eg any assignment really) which would be evaluated likethe as T case.
Okay, so we can report two things: x as ... as T where x is already exactly T (excluding T = any), and x as T where x has a contextual type and the type of x is already assignable (but not necessarily cross-assignable) to that contextual type?
That makes sense to me. cc @bradzacher @me4502 - any thoughts?
Comparing exactness does feel like it'd catch a majority of cases at least so that makes sense to me. Only case I'm not sure it'd match is the assignability case when an object is created within a method parameter that is assignable to the type but is unnecessarily cast anyway, but it's not super major if it doesn't cover all cases
Hey @me4502, I'm implementing your desired changes in #11789. I noticed that the example cases from your first comment are incorrect. alsoRequired should be of type string and not number, right?
Hey @me4502, I'm implementing your desired changes in #11789. I noticed that the example cases from your first comment are incorrect.
alsoRequiredshould be of typestringand notnumber, right?
Thank you! :) Yes, not sure how I missed that 😅. The example should have had a number there so it was a valid type without error