From a15541db4fa61d220ba47af1d6f4c0ca712c77e9 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 8 Sep 2019 22:23:25 -0400 Subject: [PATCH] Added allow-generics option to invalid-void rule (#4839) --- src/rules/invalidVoidRule.ts | 115 ++++++++++++++++-- .../allow-generics/false/test.ts.lint | 16 +++ .../allow-generics/false/tslint.json | 7 ++ .../allow-generics/true/test.ts.lint | 11 ++ .../allow-generics/true/tslint.json | 7 ++ .../allow-generics/whitelist/test.ts.lint | 12 ++ .../allow-generics/whitelist/tslint.json | 7 ++ .../invalid-void/{ => default}/test.ts.lint | 14 ++- .../invalid-void/{ => default}/tslint.json | 0 9 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 test/rules/invalid-void/allow-generics/false/test.ts.lint create mode 100644 test/rules/invalid-void/allow-generics/false/tslint.json create mode 100644 test/rules/invalid-void/allow-generics/true/test.ts.lint create mode 100644 test/rules/invalid-void/allow-generics/true/tslint.json create mode 100644 test/rules/invalid-void/allow-generics/whitelist/test.ts.lint create mode 100644 test/rules/invalid-void/allow-generics/whitelist/tslint.json rename test/rules/invalid-void/{ => default}/test.ts.lint (90%) rename test/rules/invalid-void/{ => default}/tslint.json (100%) diff --git a/src/rules/invalidVoidRule.ts b/src/rules/invalidVoidRule.ts index 2baec30a50f..83c0380c807 100644 --- a/src/rules/invalidVoidRule.ts +++ b/src/rules/invalidVoidRule.ts @@ -15,16 +15,31 @@ * 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; +} + +type RawOptions = + | undefined + | { + [OPTION_ALLOW_GENERICS]?: boolean | Set; + }; + +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, @@ -32,18 +47,52 @@ export class Rule extends Lint.Rules.AbstractRule { 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): 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); diff --git a/test/rules/invalid-void/allow-generics/false/test.ts.lint b/test/rules/invalid-void/allow-generics/false/test.ts.lint new file mode 100644 index 00000000000..2ad5f1ef10f --- /dev/null +++ b/test/rules/invalid-void/allow-generics/false/test.ts.lint @@ -0,0 +1,16 @@ +type Generic = [T]; +type GenericVoid = Generic; + ~~~~ [0] + +function takeVoid(thing: void) { } + ~~~~ [0] + +let voidPromise: Promise = new Promise(() => {}); + ~~~~ [0] + ~~~~ [0] + +let voidMap: Map = new Map(); + ~~~~ [0] + ~~~~ [0] + +[0]: void is only valid as a return type diff --git a/test/rules/invalid-void/allow-generics/false/tslint.json b/test/rules/invalid-void/allow-generics/false/tslint.json new file mode 100644 index 00000000000..baddd96c2b4 --- /dev/null +++ b/test/rules/invalid-void/allow-generics/false/tslint.json @@ -0,0 +1,7 @@ +{ + "rules": { + "invalid-void": [true, { + "allow-generics": false + }] + } +} diff --git a/test/rules/invalid-void/allow-generics/true/test.ts.lint b/test/rules/invalid-void/allow-generics/true/test.ts.lint new file mode 100644 index 00000000000..900c5f54946 --- /dev/null +++ b/test/rules/invalid-void/allow-generics/true/test.ts.lint @@ -0,0 +1,11 @@ +type Generic = [T]; +type GenericVoid = Generic; + +function takeVoid(thing: void) { } + ~~~~ [0] + +let voidPromise: Promise = new Promise(() => {}); + +let voidMap: Map = new Map(); + +[0]: void is only valid as a return type or generic type variable diff --git a/test/rules/invalid-void/allow-generics/true/tslint.json b/test/rules/invalid-void/allow-generics/true/tslint.json new file mode 100644 index 00000000000..d7dec9afca2 --- /dev/null +++ b/test/rules/invalid-void/allow-generics/true/tslint.json @@ -0,0 +1,7 @@ +{ + "rules": { + "invalid-void": [true, { + "allow-generics": true + }] + } +} diff --git a/test/rules/invalid-void/allow-generics/whitelist/test.ts.lint b/test/rules/invalid-void/allow-generics/whitelist/test.ts.lint new file mode 100644 index 00000000000..3734b406e2b --- /dev/null +++ b/test/rules/invalid-void/allow-generics/whitelist/test.ts.lint @@ -0,0 +1,12 @@ +type Allowed = [T]; +type AllowedVoid = Allowed; + +type Banned = [T]; +type BannedVoid = Banned; + ~~~~ [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 diff --git a/test/rules/invalid-void/allow-generics/whitelist/tslint.json b/test/rules/invalid-void/allow-generics/whitelist/tslint.json new file mode 100644 index 00000000000..44c4d38f9ef --- /dev/null +++ b/test/rules/invalid-void/allow-generics/whitelist/tslint.json @@ -0,0 +1,7 @@ +{ + "rules": { + "invalid-void": [true, { + "allow-generics": ["Allowed"] + }] + } +} diff --git a/test/rules/invalid-void/test.ts.lint b/test/rules/invalid-void/default/test.ts.lint similarity index 90% rename from test/rules/invalid-void/test.ts.lint rename to test/rules/invalid-void/default/test.ts.lint index 27c065770b1..1f1fef5fc85 100644 --- a/test/rules/invalid-void/test.ts.lint +++ b/test/rules/invalid-void/default/test.ts.lint @@ -75,9 +75,9 @@ class ClassName { ~~~~ [0] } -let invalidMap: Map = new Map(); - ~~~~ [0] - ~~~~ [0] +let voidPromise: Promise = new Promise(() => {}); + +let voidMap: Map = new Map(); let letVoid: void; ~~~~ [0] @@ -99,6 +99,12 @@ type UnionType3 = string | (number & any | (string | void)); type IntersectionType = string & number & void; ~~~~ [0] +function returnsVoidPromiseDirectly(): Promise { + return Promise.resolve(); +} + +async function returnsVoidPromiseAsync(): Promise {} + #if typescript >= 2.8.0 type MappedType = { [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 diff --git a/test/rules/invalid-void/tslint.json b/test/rules/invalid-void/default/tslint.json similarity index 100% rename from test/rules/invalid-void/tslint.json rename to test/rules/invalid-void/default/tslint.json