Rewrite no-unsafe-any (palantir#2496)

src/rules/noUnsafeAnyRule.ts

@@ -43,72 +43,185 @@ export class Rule extends Lint.Rules.TypedRule {
     }
 }
 
-// This is marked @internal, but we need it!
-const isExpression: (node: ts.Node) => node is ts.Expression = (ts as any).isExpression;
-
 function walk(ctx: Lint.WalkContext<void>, checker: ts.TypeChecker): void {
-    return ts.forEachChild(ctx.sourceFile, recur);
-    function recur(node: ts.Node): void {
-        if (isExpression(node) && isAny(checker.getTypeAtLocation(node)) && !isAllowedLocation(node, checker)) {
-            ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
-        } else {
-            return ts.forEachChild(node, recur);
-        }
+    if (ctx.sourceFile.isDeclarationFile) {
+        // Not possible in a declaration file.
+        return;
     }
-}
+    return ts.forEachChild(ctx.sourceFile, cb);
+
+    /** @param anyOk If true, this node will be allowed to be of type *any*. (But its children might not.) */
+    function cb(node: ts.Node, anyOk?: boolean): void {
+        switch (node.kind) {
+            case ts.SyntaxKind.ParenthesizedExpression:
+                // Don't warn on a parenthesized expression, warn on its contents.
+                return cb((node as ts.ParenthesizedExpression).expression, anyOk);
+
+            case ts.SyntaxKind.Parameter: {
+                const { type, initializer } = node as ts.ParameterDeclaration;
+                if (initializer !== undefined) {
+                    return cb(initializer, /*anyOk*/ type !== undefined && type.kind === ts.SyntaxKind.AnyKeyword);
+                }
+                return;
+            }
+
+            // Ignore types
+            case ts.SyntaxKind.InterfaceDeclaration:
+            case ts.SyntaxKind.TypeAliasDeclaration:
+            case ts.SyntaxKind.QualifiedName:
+            case ts.SyntaxKind.TypePredicate:
+            case ts.SyntaxKind.TypeOfExpression:
+            // Ignore imports
+            case ts.SyntaxKind.ImportEqualsDeclaration:
+            case ts.SyntaxKind.ImportDeclaration:
+            case ts.SyntaxKind.ExportDeclaration:
+            // For some reason, these are of type "any".
+            case ts.SyntaxKind.StringLiteral:
+                return;
 
-function isAllowedLocation(node: ts.Expression, { getContextualType, getTypeAtLocation }: ts.TypeChecker): boolean {
-    const parent = node.parent!;
-    switch (parent.kind) {
-        case ts.SyntaxKind.ExpressionStatement: // Allow unused expression
-        case ts.SyntaxKind.Parameter: // Allow to declare a parameter of type 'any'
-        case ts.SyntaxKind.TypeOfExpression: // Allow test
-        case ts.SyntaxKind.TemplateSpan: // Allow stringification (works on all values)
-        // Allow casts
-        case ts.SyntaxKind.TypeAssertionExpression:
-        case ts.SyntaxKind.AsExpression:
-            return true;
-
-        // OK to pass 'any' to a function that takes 'any' as its argument
-        case ts.SyntaxKind.CallExpression:
-        case ts.SyntaxKind.NewExpression:
-            return isAny(getContextualType(node));
-
-        case ts.SyntaxKind.BinaryExpression:
-            const { left, right, operatorToken } = parent as ts.BinaryExpression;
-            // Allow equality since all values support equality.
-            if (Lint.getEqualsKind(operatorToken) !== undefined) {
-                return true;
+            // Recurse through these, but ignore the immediate child because it is allowed to be 'any'.
+            case ts.SyntaxKind.ExpressionStatement:
+            case ts.SyntaxKind.TypeAssertionExpression:
+            case ts.SyntaxKind.AsExpression:
+            case ts.SyntaxKind.TemplateSpan: // Allow stringification (works on all values). Note: tagged templates handled differently.
+            case ts.SyntaxKind.ThrowStatement: {
+                const { expression } =
+                    node as ts.ExpressionStatement | ts.TypeAssertion | ts.AsExpression | ts.TemplateSpan | ts.ThrowStatement;
+                return cb(expression, /*anyOk*/ true);
             }
-            switch (operatorToken.kind) {
-                case ts.SyntaxKind.InstanceOfKeyword: // Allow test
-                    return true;
-                case ts.SyntaxKind.PlusToken: // Allow stringification
-                    return node === left ? isStringLike(right) : isStringLike(left);
-                case ts.SyntaxKind.PlusEqualsToken: // Allow stringification in `str += x;`, but not `x += str;`.
-                    return node === right && isStringLike(left);
-                default:
-                    return false;
+
+            case ts.SyntaxKind.PropertyDeclaration: {
+                const { name, initializer } = node as ts.PropertyDeclaration;
+                if (initializer !== undefined) {
+                    return cb(initializer, /*anyOk*/ isNodeAny(name, checker));
+                }
+                return;
             }
 
-        // Allow `const x = foo;`, but not `const x: Foo = foo;`.
-        case ts.SyntaxKind.VariableDeclaration:
-            return Lint.hasModifier(parent.parent!.parent!.modifiers, ts.SyntaxKind.DeclareKeyword) ||
-                (parent as ts.VariableDeclaration).type === undefined;
+            case ts.SyntaxKind.TaggedTemplateExpression: {
+                const { tag, template } = node as ts.TaggedTemplateExpression;
+                cb(tag);
+                if (template.kind === ts.SyntaxKind.TemplateExpression) {
+                    for (const { expression } of template.templateSpans) {
+                        checkContextual(expression);
+                    }
+                }
+                // Also check the template expression itself
+                check();
+                return;
+            }
 
-        case ts.SyntaxKind.PropertyAccessExpression:
-            // Don't warn for right hand side; this is redundant if we warn for the left-hand side.
-            return (parent as ts.PropertyAccessExpression).name === node;
+            case ts.SyntaxKind.CallExpression:
+            case ts.SyntaxKind.NewExpression: {
+                const { expression, arguments: args } = node as ts.CallExpression | ts.NewExpression;
+                cb(expression);
+                for (const arg of args) {
+                    checkContextual(arg);
+                }
+                // Also check the call expression itself
+                check();
+                return;
+            }
 
-        default:
-            return false;
+            case ts.SyntaxKind.PropertyAccessExpression:
+                // Don't warn for right hand side; this is redundant if we warn for the access itself.
+                cb((node as ts.PropertyAccessExpression).expression);
+                check();
+                return;
+
+            case ts.SyntaxKind.VariableDeclaration:
+                return checkVariableDeclaration(node as ts.VariableDeclaration);
+
+            case ts.SyntaxKind.BinaryExpression:
+                return checkBinaryExpression(node);
+
+            case ts.SyntaxKind.ReturnStatement: {
+                const { expression } = node as ts.ReturnStatement;
+                if (expression) {
+                    return checkContextual(expression);
+                }
+                return;
+            }
+
+            default:
+                if (!(ts.isExpression(node) && check())) {
+                    return ts.forEachChild(node, cb);
+                }
+                return;
+        }
+
+        function check(): boolean {
+            const isUnsafe = !anyOk && isNodeAny(node, checker);
+            if (isUnsafe) {
+                ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
+            }
+            return isUnsafe;
+        }
     }
 
-    function isStringLike(expr: ts.Expression): boolean {
-        return Lint.isTypeFlagSet(getTypeAtLocation(expr), ts.TypeFlags.StringLike);
+    /** OK for this value to be 'any' if that's its contextual type. */
+    function checkContextual(arg: ts.Expression): void {
+        return cb(arg, /*anyOk*/ isAny(checker.getContextualType(arg)));
     }
+
+    // Allow `const x = foo;` and `const x: any = foo`, but not `const x: Foo = foo;`.
+    function checkVariableDeclaration({ type, initializer }: ts.VariableDeclaration): void {
+        // Always allow the LHS to be `any`. Just don't allow RHS to be `any` when LHS isn't.
+        // TODO: handle destructuring
+        if (initializer !== undefined) {
+            return cb(initializer, /*anyOk*/ type === undefined || type.kind === ts.SyntaxKind.AnyKeyword);
+        }
+        return;
+    }
+
+    function checkBinaryExpression(node: ts.Node): void {
+        const { left, right, operatorToken } = node as ts.BinaryExpression;
+        // Allow equality since all values support equality.
+        if (Lint.getEqualsKind(operatorToken) !== undefined) {
+            return;
+        }
+
+        switch (operatorToken.kind) {
+            case ts.SyntaxKind.InstanceOfKeyword: // Allow test
+                return cb(right);
+
+            case ts.SyntaxKind.CommaToken: // Allow `any, any`
+                cb(left, /*anyOk*/ true);
+                return cb(right, /*anyOk*/ true);
+
+            case ts.SyntaxKind.EqualsToken:
+                // Allow assignment if the lhs is also *any*.
+                // TODO: handle destructuring
+                cb(right, /*anyOk*/ isNodeAny(left, checker));
+                return;
+
+            case ts.SyntaxKind.PlusToken: // Allow implicit stringification
+            case ts.SyntaxKind.PlusEqualsToken:
+                const anyOk = isStringLike(left, checker)
+                    || (isStringLike(right, checker) && operatorToken.kind === ts.SyntaxKind.PlusToken);
+                cb(left, anyOk);
+                return cb(right, anyOk);
+
+            default:
+                cb(left);
+                return cb(right);
+        }
+    }
+}
+
+function isNodeAny(node: ts.Node, checker: ts.TypeChecker): boolean {
+    return isAny(checker.getTypeAtLocation(node));
+}
+
+function isStringLike(expr: ts.Expression, checker: ts.TypeChecker): boolean {
+    return Lint.isTypeFlagSet(checker.getTypeAtLocation(expr), ts.TypeFlags.StringLike);
 }
 
 function isAny(type: ts.Type | undefined): boolean {
     return type !== undefined && Lint.isTypeFlagSet(type, ts.TypeFlags.Any);
 }
+
+declare module "typescript" {
+    // This is marked @internal, but we need it!
+    function isExpression(node: ts.Node): node is ts.Expression;
+}

test/rules/no-unsafe-any/commonjsModule.ts

@@ -0,0 +1,3 @@
+const x: any = 0;
+namespace x {}
+export = x;

test/rules/no-unsafe-any/es6Module.ts

@@ -0,0 +1,4 @@
+const defaultExport: any = 0;
+export default defaultExport;
+export const namedExport: any = 0;
+export type T = number;

test/rules/no-unsafe-any/test.ts.lint

@@ -1,6 +1,82 @@
+import importEquals = require("./commonjsModule");
+import importAlias = importEquals;
+namespace N { const x: any = 0; }
+import importQualifiedName = N.x;
+import * as namespaceImport from "./es6Module";
+import defaultExport, { namedExport } from "./es6Module";
+
+const num: namespaceImport.T = 0;
+
+{
+    const { x: { y, z } } = { x: { y: 1, z: 2 } };
+    function f(a: any, b: { x: any }) {
+        const { g } = a; // TODO: warn here!
+        const { x } = b;
+    }
+}
+
+// Ignore string literal in type
+function returnsA(): "a";
+
+
+importAlias.property;
+~~~~~~~~~~~ [0]
+
+namespaceImport.namedExport;
+
+importAlias, importAlias;
+
+
 declare const x: any;
+declare const hasProp: { x: any };
+
+hasProp.x(1);
+~~~~~~~~~ [0]
+
+declare function takesAny(a: any, ...bs: any[]): void;
+declare function takesNumber(a: number, ...bs: number[]): void;
+
+takesAny(x, x);
+takesNumber(x, x);
+            ~ [0]
+               ~ [0]
+
+declare function templateTakesAny(arr: TemplateStringsArray, a: any, ...bs: any[]): any;
+declare function templateTakesNumber(arr: TemplateStringsArray, a: number, ...bs: number[]): any;
+
+templateTakesAny`${x}${x}`;
+templateTakesNumber`${x}${x}`;
+                      ~ [0]
+                          ~ [0]
+templateTakesAny`${x}`.prop;
+~~~~~~~~~~~~~~~~~~~~~~ [0]
+
+declare function decoratorTakesAny(value: any): Function;
+declare function decoratorTakesNumber(value: number): Function;
+declare const decoratorIsAny: any;
+
+class C {
+    @decoratorTakesAny(x) f() {}
+    @decoratorTakesNumber(x) g() {}
+                          ~ [0]
+    @decoratorIsAny h() {}
+     ~~~~~~~~~~~~~~ [0]
+}
+
+x instanceof Date;
+Date instanceof x;
+                ~ [0]
+
+const retBool: () => boolean = x;
+                               ~ [0]
+
+function params(a: any = x, b: boolean = x) {}
+                                         ~ [0]
+
+function f(x: any, retAny: () => any): any {
+    x;
+    (x);
 
-function f(x: any) {
     x.foo;
     ~ [0]
     x(0);
@@ -10,20 +86,44 @@ function f(x: any) {
     x + 3;
     ~ [0]
 
-    // OK to pass it to a function that takes `any`
-    g(x);
-    // Not OK to pass to any other function.
-    [].map(x);
-           ~ [0]
+    retAny();
+    retAny()[0];
+    ~~~~~~~~ [0]
 
     // Same for constructors
     new X(x);
     new Y(x);
           ~ [0]
+
+    // Assignment: assign to 'any' OK, assign to other not OK.
+    const v0: any = x;
+    const v1: boolean = x;
+                        ~ [0]
+    let v2: any, v3: boolean;
+    v2 = x;
+    v3 = x;
+         ~ [0]
+
+    // Return OK if return type is 'any'
+    return x;
+}
+
+function f2(x: any): boolean {
+    return x;
+           ~ [0]
 }
 
-class X { constructor(x: any) {} }
-class Y { constructor(x: number) {} }
+class X {
+    constructor(x: any) {}
+
+    prop0: any = x;
+    prop1: number = x;
+                    ~ [0]
+
+}
+class Y {
+    constructor(x: number) {}
+}
 
 function g(x: any): string {
     if (x === undefined) {
@@ -37,6 +137,8 @@ function g(x: any): string {
         return `Array, length ${x.length}`;
     }
     if (Math.random() > 0.5) {
+        // Allow explicit cast to 'any'
+        (x as any).whatchamacallit;
         // Allow explicit cast
         return (<string> x).toLowerCase() + (x as string).toUpperCase();
     }