Skip to content

Commit c6bd176

Browse files
brettz9brettz9
committed
fix(no-unnecessary-type-assertion): handle generics
1 parent 4e56312 commit c6bd176

4 files changed

Lines changed: 160 additions & 3 deletions

File tree

‎.README/rules/no-unnecessary-type-assertion.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ fixed) on a `const` declarator, where the literal type is inferred anyway;
1818
elsewhere (a `let`/`var` binding, a `return`, an object-property value, etc.) the
1919
cast suppresses widening, so it is doing real work and is left alone.
2020

21+
Generic `call()`/`new` expressions whose type arguments are inferred (e.g.
22+
`document.querySelectorAll(sel)`, which defaults to `NodeListOf<Element>`) are
23+
never reported: the `@type` supplies the contextual type TypeScript uses to
24+
infer those arguments, so the inferred and asserted types always coincide and a
25+
real narrowing (to `NodeListOf<HTMLElement>`, say) cannot be told apart from a
26+
redundant one.
27+
2128
**Note that this experimental rule requires that the `typescript` package is installed.
2229
You must also install and point to the `typescript-eslint` parser, targeting your
2330
JavaScript + JSDoc files. Note also that this rule runs fairly slowly.**

‎docs/rules/no-unnecessary-type-assertion.md‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ fixed) on a `const` declarator, where the literal type is inferred anyway;
2020
elsewhere (a `let`/`var` binding, a `return`, an object-property value, etc.) the
2121
cast suppresses widening, so it is doing real work and is left alone.
2222

23+
Generic `call()`/`new` expressions whose type arguments are inferred (e.g.
24+
`document.querySelectorAll(sel)`, which defaults to `NodeListOf<Element>`) are
25+
never reported: the `@type` supplies the contextual type TypeScript uses to
26+
infer those arguments, so the inferred and asserted types always coincide and a
27+
real narrowing (to `NodeListOf<HTMLElement>`, say) cannot be told apart from a
28+
redundant one.
29+
2330
**Note that this experimental rule requires that the `typescript` package is installed.
2431
You must also install and point to the `typescript-eslint` parser, targeting your
2532
JavaScript + JSDoc files. Note also that this rule runs fairly slowly.**
@@ -186,6 +193,9 @@ const b = a;
186193
foo(/** @type {number[]} */ ([1, 2]));
187194
// Message: The @type tag declaring "number[]" is redundant as TypeScript infers it automatically.
188195

196+
const d = /** @type {Date} */ (new Date());
197+
// Message: The @type tag declaring "Date" is redundant as TypeScript infers it automatically.
198+
189199
let a;
190200
a = /** @type {5} */ (5);
191201
// Message: The @type tag declaring "5" is redundant as TypeScript infers it automatically.
@@ -321,5 +331,26 @@ const mapPaths = {prop: "text"};
321331

322332
/** @type {string} */
323333
const a = 5, b = 'x';
334+
335+
/**
336+
* @param {string} sel
337+
* @returns {HTMLElement[]}
338+
*/
339+
const $$ = (sel) => [...(/** @type {NodeListOf<HTMLElement>} */ (
340+
document.querySelectorAll(sel)
341+
))];
342+
343+
/**
344+
* @param {string} sel
345+
*/
346+
const q = (sel) => {
347+
/** @type {NodeListOf<HTMLElement>} */
348+
const els = document.querySelectorAll(sel);
349+
return els;
350+
};
351+
352+
const p = /** @type {Promise<number>} */ (Promise.resolve(5));
353+
354+
const m = /** @type {Map<string, number>} */ (new Map());
324355
````
325356

‎src/rules/noUnnecessaryTypeAssertion.js‎

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,57 @@ export default iterateJsdoc(({
154154
'The @type tag declaring "{{ type }}" is redundant as TypeScript infers it automatically for literals.' :
155155
'The @type tag declaring "{{ type }}" is redundant as TypeScript infers it automatically.';
156156

157+
/**
158+
* Whether `inferredType` is a generic reference carrying `any` type arguments
159+
* that `assertedType` replaces with concrete ones (e.g. an untyped
160+
* `document.querySelectorAll(sel)` giving `NodeListOf<any>`, asserted as
161+
* `NodeListOf<HTMLElement>`, or `new Map()` giving `Map<any, any>`). Such an
162+
* assertion supplies real type information, so it is not redundant even though
163+
* `any` leaves the two types mutually assignable.
164+
* @param {any} inferredType `ts.Type`
165+
* @param {any} assertedType `ts.Type`
166+
* @returns {boolean}
167+
*/
168+
const tightensAnyTypeArgument = (inferredType, assertedType) => {
169+
// Only ever called for an object type, which always carries `objectFlags`.
170+
if ((inferredType.objectFlags & ts.ObjectFlags.Reference) === 0) {
171+
return false;
172+
}
173+
174+
const assertedTypeArguments = checker.getTypeArguments(assertedType);
175+
176+
return checker.getTypeArguments(inferredType).some((inferredTypeArgument, index) => {
177+
return (inferredTypeArgument.flags & ts.TypeFlags.Any) !== 0 &&
178+
assertedTypeArguments[index] !== undefined &&
179+
(assertedTypeArguments[index].flags & ts.TypeFlags.Any) === 0;
180+
});
181+
};
182+
183+
/**
184+
* A generic call/`new` expression takes its type arguments partly from the
185+
* surrounding contextual type, which under a `@type` (a cast, or a
186+
* declaration) is the asserted type itself. `getTypeAtLocation` then just
187+
* echoes the asserted type back, so a genuine tightening looks redundant
188+
* (`document.querySelectorAll(sel)` is really `NodeListOf<Element>`, not the
189+
* asserted `NodeListOf<HTMLElement>`). The uncontaminated type cannot be
190+
* recovered here, so such expressions are left alone.
191+
* @param {any} tsExpression `ts.Node`
192+
* @returns {boolean}
193+
*/
194+
const isGenericCall = (tsExpression) => {
195+
if (!ts.isCallExpression(tsExpression) && !ts.isNewExpression(tsExpression)) {
196+
return false;
197+
}
198+
199+
const signature = /** @type {any} */ (
200+
checker.getResolvedSignature(tsExpression)
201+
);
202+
return Boolean(
203+
signature &&
204+
(signature.typeParameters ?? signature.target?.typeParameters)?.length,
205+
);
206+
};
207+
157208
/**
158209
* Whether the JSDoc-asserted type adds nothing over the type TypeScript
159210
* already infers for the expression it is attached to.
@@ -209,6 +260,10 @@ export default iterateJsdoc(({
209260
return false;
210261
}
211262

263+
if (tightensAnyTypeArgument(rawInferredType, rawAssertedType)) {
264+
return false;
265+
}
266+
212267
// Objects: strip the literal-initialization flags, then require structural
213268
// equivalence in both directions, so `{prop: string}` vs `{prop: string}` is
214269
// redundant while `{prop?: string}` vs `{prop: string}` fails backward.
@@ -256,9 +311,12 @@ export default iterateJsdoc(({
256311
return;
257312
}
258313

259-
const declInferredType = checker.getTypeAtLocation(
260-
services.esTreeNodeToTSNodeMap.get(decl.init),
261-
);
314+
const declInitTsNode = services.esTreeNodeToTSNodeMap.get(decl.init);
315+
if (isGenericCall(declInitTsNode)) {
316+
return;
317+
}
318+
319+
const declInferredType = checker.getTypeAtLocation(declInitTsNode);
262320
const declAssertedType = checker.getTypeFromTypeNode(jsdocTypeNode);
263321

264322
if (isRedundantAssertion(declInferredType, declAssertedType)) {
@@ -285,6 +343,10 @@ export default iterateJsdoc(({
285343
return;
286344
}
287345

346+
if (isGenericCall(exprTsNode)) {
347+
return;
348+
}
349+
288350
const parent = /** @type {any} */ (node.parent);
289351
const declaration = parent.type === 'VariableDeclarator' ? parent.parent : null;
290352

‎test/rules/assertions/noUnnecessaryTypeAssertion.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,22 @@ export default /** @type {import('../index.js').TestCases} */ ({
300300
foo([1, 2]);
301301
`,
302302
},
303+
{
304+
code: `
305+
const d = /** @type {Date} */ (new Date());
306+
`,
307+
errors: [
308+
{
309+
line: 2,
310+
message: 'The @type tag declaring "Date" is redundant as TypeScript infers it automatically.',
311+
},
312+
],
313+
filename: 'dummy.js',
314+
languageOptions,
315+
output: `
316+
const d = new Date();
317+
`,
318+
},
303319
{
304320
code: `
305321
let a;
@@ -683,6 +699,47 @@ export default /** @type {import('../index.js').TestCases} */ ({
683699
filename: 'dummy.js',
684700
languageOptions,
685701
},
702+
{
703+
code: `
704+
/**
705+
* @param {string} sel
706+
* @returns {HTMLElement[]}
707+
*/
708+
const $$ = (sel) => [...(/** @type {NodeListOf<HTMLElement>} */ (
709+
document.querySelectorAll(sel)
710+
))];
711+
`,
712+
filename: 'dummy.js',
713+
languageOptions,
714+
},
715+
{
716+
code: `
717+
/**
718+
* @param {string} sel
719+
*/
720+
const q = (sel) => {
721+
/** @type {NodeListOf<HTMLElement>} */
722+
const els = document.querySelectorAll(sel);
723+
return els;
724+
};
725+
`,
726+
filename: 'dummy.js',
727+
languageOptions,
728+
},
729+
{
730+
code: `
731+
const p = /** @type {Promise<number>} */ (Promise.resolve(5));
732+
`,
733+
filename: 'dummy.js',
734+
languageOptions,
735+
},
736+
{
737+
code: `
738+
const m = /** @type {Map<string, number>} */ (new Map());
739+
`,
740+
filename: 'dummy.js',
741+
languageOptions,
742+
},
686743
],
687744
});
688745

0 commit comments

Comments
 (0)