[no-unnecessary-type-assertion] only checks non-null-assertion #4485
Comments
|
The rule specifically ignores For example: let x = 1; // typeof x = number
let y = 1 as 1; // typeof y = 1One could argue that you should use const x = 1; // typeof x = 1
let y = x; // typeof y = number
let z1 = x as const; // ERROR - 'const' assertions can only be applied to references to enum members, or string, number, boolean, array, or object literals
let z2 = x as 1; // okay - typeof z2 = 1So there are cases where a literal assertion is completely necessary, even though the thing it's asserting is already that type. We could probably reduce the scope of the check i.e. ignore some cases rather than ignoring *all• cases. However the difficult thing with that is figuring out the cases that are always provably fine to ignore. Worth noting that the "literal-ness" of a type is resolved based on the type itself, regardless of if it's an alias - which is why |
|
I get that ensuring no false positives and no false negatives is quite difficult. And if the most complex cases are not found, I think that's better than false positives. However, the examples show straightforward cases, and they are lifted directly from the docs.
That would target this 👇 case, which is actually not useless (although I would suggest an annotation instead of an assertion): const foo = 3 as number;In the samples, we are generally asserting that
Actually, I added the “correct” samples, plus a line using the function at the end: //…
const foon = <number>3;
const foom = 3 as number;
const fool = 'foo' as const;
function fook(x: number | undefined): number {
return x!;
}
const foos = fook(3) as numberNow, I get this output: 11:13 error This assertion is unnecessary since it does not change the type of the expression @typescript-eslint/no-unnecessary-type-assertion
14:9 error This assertion is unnecessary since it does not change the type of the expression @typescript-eslint/no-unnecessary-type-assertion
27:14 error This assertion is unnecessary since it does not change the type of the expression @typescript-eslint/no-unnecessary-type-assertion
✖ 3 problems (3 errors, 0 warnings)So, I can see that it works outside simple literal assignments. All in all, I think at least a docs update is in order, and checking for exact matches with literals would be a nice addition. doAsync()
.catch((error) => {
console.error(error)
})
type Foo = 3
// bad
const foox: Foo = 3 as 3
// bad
const fooy = 3 as Foo
// bad ‑ probably a different rule since this is an annotation, not an assertion
const fooz: Foo = 3
// bad
const fooa = <3>3
// bad
const foob = <Foo>3
// good
const fooc = 3
// bad
const bar = fooc!
function foo (x: number): number {
// bad
return x!
}
// good
const foon = <number>3
// good
const foom = 3 as number
// bad
const fool = 'foo' as const
function fook(x: number | undefined): number {
// good
return x!
}
// bad
const foos = fook(3) as number
// good
let vy = 1 as 1
// bad
const cy = 1 as 1
const cx = 1
// bad
let z0 = cx as number
// bad, nice to check but not necessary
let z1 = cx as const
// good
let z2 = cx as 1 |
Note that this case is ignored intentionally because the type There are two cases the rule ignores:
It doesn't matter what the value is in the assertion - if the asserted type resolves to a type that matches one of the above, it is ignored. AFAIK the tuple type case is never safe to remove, even in a I'm happy for someone to teach the rule about cases where it is 100% provably safe to not apply the above cases. |
|
Hi! I've run into this issue too. I created a bare playground with no configuration at all, added the
If this is the case, @bradzacher , maybe the documentation is what needs fixing. I mean, the rule would then be right, it's just that the docs are wrong. |
|
IMO, ignoring literals is arbitrary and potentially unsafe. Can I do a PR for a edit - typoed my intentions for the flag |
|
How would that be beneficial once #4485 (comment) is implemented?
|
|
I don't know what the algorithm would be to determine 100% provably safe cases, so I can't help improve the rule in that way. Until someone comes along who is smart enough to implement this, I think the rule could be improved by a |
Uh oh!
There was an error while loading. Please reload this page.
Repro
{ "root": true, "parser": "@typescript-eslint/parser", "parserOptions": { "ecmaVersion": 2021, "project": "./tsconfig.json", "sourceType": "module" }, "plugins": [ "@typescript-eslint" ], "rules": { "@typescript-eslint/no-unnecessary-type-assertion": "error" } }{ "compilerOptions": { "strict": true }, "include": [ "./code.ts" ], }Expected Result
OR (see below)
Actual Result
Additional Info
The
typesToIgnoreoption description implies also checking annotations. Since annotations and assertions are different things, I would expect them to be handled by separate rules. Depending on whether or not they are an intended target for this rule, the first or second expectation should be fulfilled.Versions
@typescript-eslint/eslint-plugin5.10.0@typescript-eslint/parser5.10.0TypeScript4.5.5ESLint8.7.0node14.17.5,16.13.2The text was updated successfully, but these errors were encountered: