no-useless-assertion: check JSDoc type assertions

baselines/packages/mimir/test/no-useless-assertion/strict/jsdoc.js.lint

@@ -0,0 +1,25 @@
+export {};
+
+function takeString(/** @type {string} */param) {}
+function takeStringOrUndefined(/** @type {string} */param = '') {}
+
+let s = '';
+let su = Boolean() ? s : undefined;
+
+s = (s);
+s = /** foo */(s);
+s = /** @type {string} */s;
+s = /** @type {string} */(s);
+        ~~~~~~~~~~~~~~        [error no-useless-assertion: This assertion is unnecesary as it doesn't change the type of the expression.]
+s = /** @type {string} */ /** @type {number} */(s);
+        ~~~~~~~~~~~~~~                              [error no-useless-assertion: This assertion is unnecesary as it doesn't change the type of the expression.]
+s = /** foobar */ /** @type {string} */ // foo
+                      ~~~~~~~~~~~~~~           [error no-useless-assertion: This assertion is unnecesary as it doesn't change the type of the expression.]
+    (s);
+s = /** @type {string} */(su);
+
+takeString(/** @type {string} */(su))
+takeStringOrUndefined(/** @type {string} */(su));
+                          ~~~~~~~~~~~~~~          [error no-useless-assertion: This assertion is unnecessary as the receiver accepts the original type of the expression.]
+
+let literal = /** @type {1} */(1);

packages/mimir/docs/no-useless-assertion.md

@@ -41,6 +41,15 @@ class C {
 let tuple = [1 as const] as const; // inner 'const' assertion is redundant
 ```
 
+This rule also checks JSDoc type assertions in JavaScript files:
+
+```js
+// @ts-check
+
+let str = '';
+console.log(/** @type {string} */(str)); // 'str' is already of type 'string'
+```
+
 :thumbsup: Examples of correct code
 
 ```ts

packages/mimir/src/rules/no-useless-assertion.ts

@@ -1,4 +1,4 @@
-import { TypedRule, Replacement, typescriptOnly, excludeDeclarationFiles } from '@fimbul/ymir';
+import { TypedRule, Replacement, excludeDeclarationFiles } from '@fimbul/ymir';
 import * as ts from 'typescript';
 import { expressionNeedsParensWhenReplacingNode, typesAreEqual } from '../utils';
 import {
@@ -23,11 +23,16 @@ const FAIL_MESSAGE = "This assertion is unnecesary as it doesn't change the type
 const FAIL_DEFINITE_ASSIGNMENT = 'This assertion is unnecessary as it has no effect on this declaration.';
 
 @excludeDeclarationFiles
-@typescriptOnly
 export class Rule extends TypedRule {
     private strictNullChecks = isStrictCompilerOptionEnabled(this.program.getCompilerOptions(), 'strictNullChecks');
 
     public apply(): void {
+        if (/\.tsx?$/.test(this.sourceFile.fileName))
+            return this.checkTsFile();
+        return this.checkJsFile();
+    }
+
+    private checkTsFile() {
         for (const node of this.context.getFlatAst()) {
             switch (node.kind) {
                 case ts.SyntaxKind.NonNullExpression:
@@ -46,6 +51,22 @@ export class Rule extends TypedRule {
         }
     }
 
+    private checkJsFile() {
+        for (const node of this.context.getFlatAst()) {
+            if (node.kind === ts.SyntaxKind.ParenthesizedExpression) {
+                const typeTags = <readonly ts.JSDocTypeTag[]>ts.getAllJSDocTagsOfKind(node, ts.SyntaxKind.JSDocTypeTag);
+                if (typeTags.length === 0)
+                    continue;
+                const tag = typeTags[0];
+                const message =
+                    this.checkAssertedType((<ts.ParenthesizedExpression>node).expression, tag.typeExpression!.type, <ts.Expression>node);
+                if (message === undefined)
+                    continue;
+                this.addFinding(tag.pos, tag.end, message); // TODO autofix is a bit more complicated here
+            }
+        }
+    }
+
     private checkDefiniteAssignmentAssertion(node: ts.VariableDeclaration) {
         // compiler already emits an error for definite assignment assertions on ambient or initialized variables
         if (node.exclamationToken !== undefined &&
@@ -98,16 +119,41 @@ export class Rule extends TypedRule {
     }
 
     private checkTypeAssertion(node: ts.AssertionExpression) {
+        let message;
         if (isConstAssertion(node)) {
-            if (isInConstContext(node))
-                this.reportUselessTypeAssertion(node, 'This assertion is unnecessary as it is already in a const context.');
-            return;
+            if (!isInConstContext(node))
+                return;
+            message = 'This assertion is unnecessary as it is already in a const context.';
+        } else {
+            message = this.checkAssertedType(node.expression, node.type, node);
+            if (message === undefined)
+                return;
         }
-        let targetType = this.checker.getTypeFromTypeNode(node.type);
+        if (node.kind === ts.SyntaxKind.AsExpression) {
+            this.addFinding(
+                node.type.pos - 'as'.length,
+                node.end,
+                message,
+                Replacement.delete(node.expression.end, node.end),
+            );
+        } else {
+            const start = node.getStart(this.sourceFile);
+            const fix = [Replacement.delete(start, node.expression.getStart(this.sourceFile))];
+            if (expressionNeedsParensWhenReplacingNode(node.expression, node))
+                fix.push(
+                    Replacement.append(start, '('),
+                    Replacement.append(node.end, ')'),
+                );
+            this.addFinding(start, node.expression.pos, message, fix);
+        }
+    }
+
+    private checkAssertedType(expression: ts.Expression, type: ts.TypeNode, node: ts.Expression): string | undefined {
+        let targetType = this.checker.getTypeFromTypeNode(type);
         if ((targetType.flags & ts.TypeFlags.Literal) !== 0 && !isInConstContext(node) || // allow "foo" as "foo" to avoid widening
             isObjectType(targetType) && (targetType.objectFlags & ts.ObjectFlags.Tuple || couldBeTupleType(targetType)))
             return;
-        let sourceType = this.checker.getTypeAtLocation(node.expression);
+        let sourceType = this.checker.getTypeAtLocation(expression);
         if ((targetType.flags & (ts.TypeFlags.TypeVariable | ts.TypeFlags.Instantiable)) === 0) {
             targetType = this.checker.getBaseConstraintOfType(targetType) || targetType;
             sourceType = this.checker.getBaseConstraintOfType(sourceType) || sourceType;
@@ -128,27 +174,7 @@ export class Rule extends TypedRule {
                 return;
             message = 'This assertion is unnecessary as the receiver accepts the original type of the expression.';
         }
-        this.reportUselessTypeAssertion(node, message);
-    }
-
-    private reportUselessTypeAssertion(node: ts.AssertionExpression, message: string) {
-        if (node.kind === ts.SyntaxKind.AsExpression) {
-            this.addFinding(
-                node.type.pos - 'as'.length,
-                node.end,
-                message,
-                Replacement.delete(node.expression.end, node.end),
-            );
-        } else {
-            const start = node.getStart(this.sourceFile);
-            const fix = [Replacement.delete(start, node.expression.getStart(this.sourceFile))];
-            if (expressionNeedsParensWhenReplacingNode(node.expression, node))
-                fix.push(
-                    Replacement.append(start, '('),
-                    Replacement.append(node.end, ')'),
-                );
-            this.addFinding(start, node.expression.pos, message, fix);
-        }
+        return message;
     }
 
     /** Returns the contextual type if it is a position that does not contribute to control flow analysis. */

packages/mimir/test/no-useless-assertion/conditional-types.test.json

@@ -1,5 +1,4 @@
 {
   "project": "tsconfig.json",
-  "files": "conditional-types.ts",
-  "typescriptVersion": ">= 2.8.0"
+  "files": "conditional-types.ts"
 }

packages/mimir/test/no-useless-assertion/definite-assignment-loose.test.json

@@ -1,5 +1,4 @@
 {
   "project": "tsconfig-loose.json",
-  "files": "definite-assignment.ts",
-  "typescriptVersion": ">= 2.7.0"
+  "files": "definite-assignment.ts"
 }

packages/mimir/test/no-useless-assertion/definite-assignment-strict.test.json

@@ -1,5 +1,4 @@
 {
   "project": "tsconfig.json",
-  "files": "definite-assignment.ts",
-  "typescriptVersion": ">= 2.7.0"
+  "files": "definite-assignment.ts"
 }

packages/mimir/test/no-useless-assertion/jsdoc.js

@@ -0,0 +1,21 @@
+export {};
+
+function takeString(/** @type {string} */param) {}
+function takeStringOrUndefined(/** @type {string} */param = '') {}
+
+let s = '';
+let su = Boolean() ? s : undefined;
+
+s = (s);
+s = /** foo */(s);
+s = /** @type {string} */s;
+s = /** @type {string} */(s);
+s = /** @type {string} */ /** @type {number} */(s);
+s = /** foobar */ /** @type {string} */ // foo
+    (s);
+s = /** @type {string} */(su);
+
+takeString(/** @type {string} */(su))
+takeStringOrUndefined(/** @type {string} */(su));
+
+let literal = /** @type {1} */(1);

packages/mimir/test/no-useless-assertion/strict.test.json

@@ -1,5 +1,4 @@
 {
   "project": "tsconfig.json",
-  "files": ["global.ts", "non-null.ts", "type-assertion.ts"],
-  "typescriptVersion": "^2.8.0 || >= 3.1.0"
+  "files": ["global.ts", "non-null.ts", "type-assertion.ts", "jsdoc.js"]
 }

packages/mimir/test/no-useless-assertion/tsconfig.json

@@ -1,5 +1,7 @@
 {
   "compilerOptions": {
-    "strict": true
+    "strict": true,
+    "allowJs": true,
+    "checkJs": true
   }
 }