-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Bug: [no-unsafe-type-assertion] Fails to report unsafe assertions involving constrained type parameters #10453
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
Comments
extends
Interesting! I think it is a valid case the rule should warn on, as this example literally narrows down a type from its type constraint to a potentially more specific one. I initially thought this has to do with type-widening, but it looks like it's related to how type constraints are handled. The rule compares the types (or the type constraints if the compared types have any). Since one type has a type constraint, which is the same as the other type, the validation stops early on. I think a potential solution would be to check the specific types, rather than the type constraints if both type constraints are the same. |
Ok, interesting! So to clarify my understanding of what you said: this behavior is because the rule obtains the Which means the rule will need to have logic for the special case of generic constraints—because it’s not generally true that A is assignable to B if A’s generic constraint is assignable to B’s (since B “could be instantiated with a different subtype of constraint”) |
Huh! We're so used to checking generics' constraints, but, yeah, that might actually be a mistake for this rule. |
Might be nice if we can special-case the error message here since the TS error is one that always surprises and confuses me at first when I run into it. |
What do you think a better error message would be? |
As a first pass, I think we could just model it after the TS error message?
Doesn't necessarily have to be that, verbatim 🤷... if we get working code that detects this case as the reason for erroring, we could probably bikeshed about wording on the PR. The only import technical thing here is ensuring that we don't give this error message on a case like export function myfunc<CustomObjectT extends string>(input: number): CustomObjectT {
const newCustomObject = input as CustomObjectT;
return newCustomObject;
} That snippet is unsafe, but because |
I took a stab at this in #10461—but I really don’t know what I’m doing, so I’d welcome feedback! |
I added this example as a test case to ensure we don’t give the “assignable to the constraint” message here! |
Before You File a Bug Report Please Confirm You Have Done The Following...
Playground Link
Playground link
Repro Code
ESLint Config
tsconfig
Expected Result
The line with the type assertion involving the generic parameter (
newObject as CustomObjectT
) should report@typescript-eslint/no-unsafe-type-assertion
by the same logic wherein Typescript considers such a conversion unsafe:The first example in the playground shows that this case is not an allowable assignment according to TS, which, to my understanding, should be the heuristic used by
no-unsafe-type-assertion
Actual Result
No error is reported from
@typescript-eslint/no-unsafe-type-assertion
Additional Info
cc @ronami
thanks for your hard work on this rule!
The text was updated successfully, but these errors were encountered: