Added allow-generics option to invalid-void rule (palantir#4839)

src/rules/invalidVoidRule.ts

@@ -15,35 +15,84 @@
  * limitations under the License.
  */
 
+import * as tsutils from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
 
+const OPTION_ALLOW_GENERICS = "allow-generics";
+
+interface Options {
+    allowGenerics: boolean | Set<string>;
+}
+
+type RawOptions =
+    | undefined
+    | {
+          [OPTION_ALLOW_GENERICS]?: boolean | Set<string>;
+      };
+
+type GenericReference = ts.NewExpression | ts.TypeReferenceNode;
+
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "invalid-void",
         description: Lint.Utils.dedent`
-            Disallows usage of \`void\` type outside of return type.
+            Disallows usage of \`void\` type outside of generic or return types.
             If \`void\` is used as return type, it shouldn't be a part of intersection/union type.`,
         rationale: Lint.Utils.dedent`
             The \`void\` type means "nothing" or that a function does not return any value,
             in contra with implicit \`undefined\` type which means that a function returns a value \`undefined\`.
             So "nothing" cannot be mixed with any other types.
             If you need this - use \`undefined\` type instead.`,
         hasFix: false,
-        optionsDescription: "Not configurable.",
-        options: null,
-        optionExamples: [true],
+        optionsDescription: Lint.Utils.dedent`
+            If \`${OPTION_ALLOW_GENERICS}\` is specified as \`false\`, then generic types will no longer be allowed to to be \`void\`.
+            Alternately, provide an array of strings for \`${OPTION_ALLOW_GENERICS}\` to exclusively allow generic types by those names.`,
+        options: {
+            type: "object",
+            properties: {
+                [OPTION_ALLOW_GENERICS]: {
+                    oneOf: [
+                        { type: "boolean" },
+                        { type: "array", items: { type: "string" }, minLength: 1 },
+                    ],
+                },
+            },
+            additionalProperties: false,
+        },
+        optionExamples: [
+            true,
+            [true, { [OPTION_ALLOW_GENERICS]: false }],
+            [true, { [OPTION_ALLOW_GENERICS]: ["Promise", "PromiseLike"] }],
+        ],
         type: "maintainability",
         typescriptOnly: true,
     };
     /* tslint:enable:object-literal-sort-keys */
 
-    public static FAILURE_STRING = "void is not a valid type other than return types";
+    public static FAILURE_STRING_ALLOW_GENERICS =
+        "void is only valid as a return type or generic type variable";
+    public static FAILURE_STRING_NO_GENERICS = "void is only valid as a return type";
+    public static FAILURE_WRONG_GENERIC = (genericName: string) =>
+        `${genericName} may not have void as a type variable`;
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithFunction(sourceFile, walk);
+        return this.applyWithFunction(sourceFile, walk, {
+            // tslint:disable-next-line:no-object-literal-type-assertion
+            allowGenerics: this.getAllowGenerics(this.ruleArguments[0] as RawOptions),
+        });
+    }
+
+    private getAllowGenerics(rawArgument: RawOptions) {
+        if (rawArgument == undefined) {
+            return true;
+        }
+
+        const allowGenerics = rawArgument[OPTION_ALLOW_GENERICS];
+
+        return allowGenerics instanceof Array ? new Set(allowGenerics) : Boolean(allowGenerics);
     }
 }
 
@@ -75,10 +124,60 @@ const failedKinds = new Set([
     ts.SyntaxKind.CallExpression,
 ]);
 
-function walk(ctx: Lint.WalkContext): void {
+function walk(ctx: Lint.WalkContext<Options>): void {
+    const defaultFailureString = ctx.options.allowGenerics
+        ? Rule.FAILURE_STRING_ALLOW_GENERICS
+        : Rule.FAILURE_STRING_NO_GENERICS;
+
+    const getGenericReferenceName = (node: GenericReference) => {
+        const rawName = tsutils.isNewExpression(node) ? node.expression : node.typeName;
+
+        return tsutils.isIdentifier(rawName) ? rawName.text : rawName.getText(ctx.sourceFile);
+    };
+
+    const getTypeReferenceFailure = (node: GenericReference) => {
+        if (!(ctx.options.allowGenerics instanceof Set)) {
+            return ctx.options.allowGenerics ? undefined : defaultFailureString;
+        }
+
+        const genericName = getGenericReferenceName(node);
+
+        return ctx.options.allowGenerics.has(genericName)
+            ? undefined
+            : Rule.FAILURE_WRONG_GENERIC(genericName);
+    };
+
+    const checkTypeReference = (parent: GenericReference, node: ts.Node) => {
+        const failure = getTypeReferenceFailure(parent);
+
+        if (failure !== undefined) {
+            ctx.addFailureAtNode(node, failure);
+        }
+    };
+
+    const isParentGenericReference = (
+        parent: ts.Node,
+        node: ts.Node,
+    ): parent is GenericReference => {
+        if (tsutils.isTypeReferenceNode(parent)) {
+            return true;
+        }
+
+        return (
+            tsutils.isNewExpression(parent) &&
+            parent.typeArguments !== undefined &&
+            ts.isTypeNode(node) &&
+            parent.typeArguments.indexOf(node) !== -1
+        );
+    };
+
     ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node) {
         if (node.kind === ts.SyntaxKind.VoidKeyword && failedKinds.has(node.parent.kind)) {
-            ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
+            if (isParentGenericReference(node.parent, node)) {
+                checkTypeReference(node.parent, node);
+            } else {
+                ctx.addFailureAtNode(node, defaultFailureString);
+            }
         }
 
         ts.forEachChild(node, cb);

test/rules/invalid-void/allow-generics/false/test.ts.lint

@@ -0,0 +1,16 @@
+type Generic<T> = [T];
+type GenericVoid = Generic<void>;
+                           ~~~~ [0]
+
+function takeVoid(thing: void) { }
+                         ~~~~ [0]
+
+let voidPromise: Promise<void> = new Promise<void>(() => {});
+                         ~~~~ [0]
+                                             ~~~~ [0]
+
+let voidMap: Map<string, void> = new Map<string, void>();
+                         ~~~~ [0]
+                                                 ~~~~ [0]
+
+[0]: void is only valid as a return type

test/rules/invalid-void/allow-generics/false/tslint.json

@@ -0,0 +1,7 @@
+{
+  "rules": {
+    "invalid-void": [true, {
+      "allow-generics": false
+    }]
+  }
+}

test/rules/invalid-void/allow-generics/true/test.ts.lint

@@ -0,0 +1,11 @@
+type Generic<T> = [T];
+type GenericVoid = Generic<void>;
+
+function takeVoid(thing: void) { }
+                         ~~~~ [0]
+
+let voidPromise: Promise<void> = new Promise<void>(() => {});
+
+let voidMap: Map<string, void> = new Map<string, void>();
+
+[0]: void is only valid as a return type or generic type variable

test/rules/invalid-void/allow-generics/true/tslint.json

@@ -0,0 +1,7 @@
+{
+  "rules": {
+    "invalid-void": [true, {
+      "allow-generics": true
+    }]
+  }
+}

test/rules/invalid-void/allow-generics/whitelist/test.ts.lint

@@ -0,0 +1,12 @@
+type Allowed<T> = [T];
+type AllowedVoid = Allowed<void>;
+
+type Banned<T> = [T];
+type BannedVoid = Banned<void>;
+                         ~~~~ [Generic % ('Banned')]
+
+function takeVoid(thing: void) { }
+                         ~~~~ [0]
+
+[0]: void is only valid as a return type or generic type variable
+[Generic]: %s may not have void as a type variable

test/rules/invalid-void/allow-generics/whitelist/tslint.json

@@ -0,0 +1,7 @@
+{
+  "rules": {
+    "invalid-void": [true, {
+      "allow-generics": ["Allowed"]
+    }]
+  }
+}

test/rules/invalid-void/test.ts.lint → test/rules/invalid-void/default/test.ts.lint

@@ -75,9 +75,9 @@ class ClassName {
                                ~~~~ [0]
 }
 
-let invalidMap: Map<string, void> = new Map<string, void>();
-                            ~~~~ [0]
-                                                    ~~~~ [0]
+let voidPromise: Promise<void> = new Promise<void>(() => {});
+
+let voidMap: Map<string, void> = new Map<string, void>();
 
 let letVoid: void;
              ~~~~ [0]
@@ -99,6 +99,12 @@ type UnionType3 = string | (number & any | (string | void));
 type IntersectionType = string & number & void;
                                           ~~~~ [0]
 
+function returnsVoidPromiseDirectly(): Promise<void> {
+    return Promise.resolve();
+}
+
+async function returnsVoidPromiseAsync(): Promise<void> {}
+
 #if typescript >= 2.8.0
 type MappedType<T> = {
     [K in keyof T]: void;
@@ -118,4 +124,4 @@ function foo(arr: readonly void[]) { }
                            ~~~~ [0]
 #endif
 
-[0]: void is not a valid type other than return types
+[0]: void is only valid as a return type or generic type variable