From 38db136b1bbfe4b464081c348e072cc22e17088d Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Wed, 20 Mar 2019 11:54:41 +0000 Subject: [PATCH 1/6] Add "no-null-undefined-union" rule. --- src/configs/all.ts | 1 + src/rules/noNullUndefinedUnionRule.ts | 100 ++++++++++++++++++ .../no-null-undefined-union/test.ts.lint | 44 ++++++++ .../no-null-undefined-union/tsconfig.json | 19 ++++ .../rules/no-null-undefined-union/tslint.json | 5 + 5 files changed, 169 insertions(+) create mode 100644 src/rules/noNullUndefinedUnionRule.ts create mode 100644 test/rules/no-null-undefined-union/test.ts.lint create mode 100644 test/rules/no-null-undefined-union/tsconfig.json create mode 100644 test/rules/no-null-undefined-union/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index 3845aff90ea..fb98cc7f242 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -126,6 +126,7 @@ export const rules = { // "no-invalid-this": Won't this be deprecated? "no-misused-new": true, "no-null-keyword": true, + "no-null-undefined-union": true, "no-object-literal-type-assertion": true, "no-return-await": true, "no-shadowed-variable": true, diff --git a/src/rules/noNullUndefinedUnionRule.ts b/src/rules/noNullUndefinedUnionRule.ts new file mode 100644 index 00000000000..aed695ad360 --- /dev/null +++ b/src/rules/noNullUndefinedUnionRule.ts @@ -0,0 +1,100 @@ +/** + * @license Copyright 2018 Palantir Technologies, Inc. All rights reserved. + */ + +import * as Lint from "tslint"; +import { isTypeReference, isUnionType } from "tsutils"; +import * as ts from "typescript"; + +export class Rule extends Lint.Rules.TypedRule { + /* tslint:disable:object-literal-sort-keys */ + public static metadata: Lint.IRuleMetadata = { + ruleName: "no-null-undefined-union", + description: "Disallows union types with both `null` and `undefined` as members.", + rationale: Lint.Utils.dedent` + A union type that includes both \`null\` and \`undefined\` is either redundant or fragile. + Enforcing the choice between the two allows the \`triple-equals\` rule to exist without + exceptions, and is essentially a more flexible version of the \`no-null-keyword\` rule. + `, + optionsDescription: "True if the rule should be enabled.", + options: null, + optionExamples: [true], + type: "functionality", + typescriptOnly: true, + requiresTypeInfo: true, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static FAILURE_STRING = "Union type cannot include both 'null' and 'undefined'."; + + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithFunction(sourceFile, walk, undefined, program.getTypeChecker()); + } +} + +function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker): void { + return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void { + if (isFunctionLikeDeclaration(node) && checkFunctionLikeDeclaration(node, tc)) { + ctx.addFailureAtNode(node, Rule.FAILURE_STRING); + } else if (isVariableLikeDeclaration(node) && checkVariableLikeDeclaration(node, tc)) { + ctx.addFailureAtNode(node, Rule.FAILURE_STRING); + } + return ts.forEachChild(node, cb); + }); +} + +function isFunctionLikeDeclaration(node: ts.Node): node is ts.FunctionLikeDeclaration { + return ( + ts.isFunctionDeclaration(node) || + ts.isMethodDeclaration(node) || + ts.isGetAccessor(node) || + ts.isSetAccessor(node) || + ts.isConstructorDeclaration(node) || + ts.isFunctionExpression(node) || + ts.isArrowFunction(node) + ); +} + +function isVariableLikeDeclaration(node: ts.Node): node is ts.VariableLikeDeclaration { + return ( + ts.isVariableDeclaration(node) || + ts.isParameter(node) || + ts.isBindingElement(node) || + ts.isPropertyDeclaration(node) || + ts.isPropertyAssignment(node) || + ts.isPropertySignature(node) || + ts.isJsxAttribute(node) || + ts.isShorthandPropertyAssignment(node) || + ts.isEnumMember(node) || + ts.isJSDocPropertyTag(node) || + ts.isJSDocParameterTag(node) + ); +} + +function checkFunctionLikeDeclaration(node: ts.FunctionLikeDeclaration, tc: ts.TypeChecker): boolean { + const signature = tc.getSignatureFromDeclaration(node); + return signature !== undefined ? isNullUndefinedUnion(signature.getReturnType()) : false; +} + +function checkVariableLikeDeclaration(node: ts.VariableLikeDeclaration, tc: ts.TypeChecker): boolean { + return isNullUndefinedUnion(tc.getTypeAtLocation(node)); +} + +function isNullUndefinedUnion(type: ts.Type): boolean { + if (isTypeReference(type) && type.typeArguments !== undefined) { + return type.typeArguments.some(isNullUndefinedUnion); + } + + if (isUnionType(type)) { + let hasNull = false; + let hasUndefined = false; + for (const subType of type.types) { + hasNull = hasNull || subType.getFlags() === ts.TypeFlags.Null; + hasUndefined = hasUndefined || subType.getFlags() === ts.TypeFlags.Undefined; + if (hasNull && hasUndefined) { + return true; + } + } + } + return false; +} diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint new file mode 100644 index 00000000000..3e8bd7b909d --- /dev/null +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -0,0 +1,44 @@ +interface someInterface { + a: number | undefined | null; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + b: boolean; +} + +const c: string | null | undefined; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +const someFunc = (): string | undefined | null => {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +const someFunc = (foo: null | string | undefined, bar: boolean) => {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +function someFunc(): number | undefined | null {} +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +function someFunc(): Promise {} // error +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +function someFunc(bar: boolean, foo: null | number | undefined) {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +function someFunc() { +~~~~~~~~~~~~~~~~~~~~~ + const somePredicate = (): boolean => true; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + const someFunc = (): string | null => null; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + let foo; +~~~~~~~~~~~~ + if (somePredicate()) { +~~~~~~~~~~~~~~~~~~~~~~~~~~ + foo = someFunc(); +~~~~~~~~~~~~~~~~~~~~~~~~~ + } +~~~~~ + return foo; +~~~~~~~~~~~~~~~ +} +~ [0] + +[0]: Union type cannot include both 'null' and 'undefined'. diff --git a/test/rules/no-null-undefined-union/tsconfig.json b/test/rules/no-null-undefined-union/tsconfig.json new file mode 100644 index 00000000000..d81c1202adb --- /dev/null +++ b/test/rules/no-null-undefined-union/tsconfig.json @@ -0,0 +1,19 @@ +{ + "compilerOptions": { + "downlevelIteration": true, + "experimentalDecorators": true, + "importHelpers": true, + "jsx": "react", + "lib": [ + "es5", + "es2015", + "es2016", + "es2017" + ], + "module": "commonjs", + "moduleResolution": "node", + "strict": true, + "stripInternal": true, + "target": "es5" + } +} diff --git a/test/rules/no-null-undefined-union/tslint.json b/test/rules/no-null-undefined-union/tslint.json new file mode 100644 index 00000000000..f41c08c7b72 --- /dev/null +++ b/test/rules/no-null-undefined-union/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-null-undefined-union": true + } +} From c239ec37e50d057460e1d839c49b23d1c78e62a4 Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Mon, 25 Mar 2019 02:45:17 +0000 Subject: [PATCH 2/6] fixed --- src/configuration.ts | 5 ++ src/rules/noNullUndefinedUnionRule.ts | 57 +++++++------------ src/rules/noUnusedVariableRule.ts | 2 +- .../no-null-undefined-union/test.ts.lint | 9 +++ 4 files changed, 35 insertions(+), 38 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index 486761c38b7..6639ab0f1ef 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -378,6 +378,7 @@ export function extendConfigurationFile( * * @deprecated use `path.resolve` instead */ +// tslint:disable-next-line no-null-undefined-union export function getRelativePath(directory?: string | null, relativeTo?: string) { if (directory != undefined) { const basePath = relativeTo !== undefined ? relativeTo : process.cwd(); @@ -428,6 +429,7 @@ export function getRulesDirectories( * @param ruleConfigValue The raw option setting of a rule */ function parseRuleOptions( + // tslint:disable-next-line no-null-undefined-union ruleConfigValue: RawRuleConfig, rawDefaultRuleSeverity: string | undefined, ): Partial { @@ -506,8 +508,11 @@ export interface RawConfigFile { jsRules?: RawRulesConfig | boolean; } export interface RawRulesConfig { + // tslint:disable-next-line no-null-undefined-union [key: string]: RawRuleConfig; } + +// tslint:disable-next-line no-null-undefined-union export type RawRuleConfig = | null | undefined diff --git a/src/rules/noNullUndefinedUnionRule.ts b/src/rules/noNullUndefinedUnionRule.ts index aed695ad360..d9875a1dd6a 100644 --- a/src/rules/noNullUndefinedUnionRule.ts +++ b/src/rules/noNullUndefinedUnionRule.ts @@ -2,10 +2,11 @@ * @license Copyright 2018 Palantir Technologies, Inc. All rights reserved. */ -import * as Lint from "tslint"; -import { isTypeReference, isUnionType } from "tsutils"; +import * as utils from "tsutils"; import * as ts from "typescript"; +import * as Lint from "../index"; + export class Rule extends Lint.Rules.TypedRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { @@ -34,58 +35,40 @@ export class Rule extends Lint.Rules.TypedRule { function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker): void { return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void { - if (isFunctionLikeDeclaration(node) && checkFunctionLikeDeclaration(node, tc)) { - ctx.addFailureAtNode(node, Rule.FAILURE_STRING); - } else if (isVariableLikeDeclaration(node) && checkVariableLikeDeclaration(node, tc)) { + const type = getType(node, tc); + if (type !== undefined && isNullUndefinedUnion(type)) { ctx.addFailureAtNode(node, Rule.FAILURE_STRING); } return ts.forEachChild(node, cb); }); } -function isFunctionLikeDeclaration(node: ts.Node): node is ts.FunctionLikeDeclaration { - return ( - ts.isFunctionDeclaration(node) || - ts.isMethodDeclaration(node) || - ts.isGetAccessor(node) || - ts.isSetAccessor(node) || - ts.isConstructorDeclaration(node) || - ts.isFunctionExpression(node) || - ts.isArrowFunction(node) - ); +function getType(node: ts.Node, tc: ts.TypeChecker): ts.Type | undefined { + if (isVariableLikeDeclaration(node) && node.name.kind === ts.SyntaxKind.Identifier) { + return tc.getTypeAtLocation(node); + } else if (utils.isSignatureDeclaration(node)) { + const signature = tc.getSignatureFromDeclaration(node); + return signature === undefined ? undefined : signature.getReturnType(); + } else { + return undefined; + } } function isVariableLikeDeclaration(node: ts.Node): node is ts.VariableLikeDeclaration { return ( - ts.isVariableDeclaration(node) || - ts.isParameter(node) || - ts.isBindingElement(node) || - ts.isPropertyDeclaration(node) || - ts.isPropertyAssignment(node) || - ts.isPropertySignature(node) || - ts.isJsxAttribute(node) || - ts.isShorthandPropertyAssignment(node) || - ts.isEnumMember(node) || - ts.isJSDocPropertyTag(node) || - ts.isJSDocParameterTag(node) + utils.isVariableDeclaration(node) || + utils.isParameterDeclaration(node) || + utils.isPropertySignature(node) || + utils.isTypeAliasDeclaration(node) ); } -function checkFunctionLikeDeclaration(node: ts.FunctionLikeDeclaration, tc: ts.TypeChecker): boolean { - const signature = tc.getSignatureFromDeclaration(node); - return signature !== undefined ? isNullUndefinedUnion(signature.getReturnType()) : false; -} - -function checkVariableLikeDeclaration(node: ts.VariableLikeDeclaration, tc: ts.TypeChecker): boolean { - return isNullUndefinedUnion(tc.getTypeAtLocation(node)); -} - function isNullUndefinedUnion(type: ts.Type): boolean { - if (isTypeReference(type) && type.typeArguments !== undefined) { + if (utils.isTypeReference(type) && type.typeArguments !== undefined) { return type.typeArguments.some(isNullUndefinedUnion); } - if (isUnionType(type)) { + if (utils.isUnionType(type)) { let hasNull = false; let hasUndefined = false; for (const subType of type.types) { diff --git a/src/rules/noUnusedVariableRule.ts b/src/rules/noUnusedVariableRule.ts index 75f222a250b..e77f8745431 100644 --- a/src/rules/noUnusedVariableRule.ts +++ b/src/rules/noUnusedVariableRule.ts @@ -94,7 +94,7 @@ function parseOptions(options: any[]): Options { let ignorePattern: RegExp | undefined; for (const o of options) { if (typeof o === "object") { - // tslint:disable-next-line no-unsafe-any + // tslint:disable-next-line no-unsafe-any no-null-undefined-union const ignore = o[OPTION_IGNORE_PATTERN] as string | null | undefined; if (ignore != undefined) { ignorePattern = new RegExp(ignore); diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint index 3e8bd7b909d..a1717a06017 100644 --- a/test/rules/no-null-undefined-union/test.ts.lint +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -7,6 +7,15 @@ interface someInterface { const c: string | null | undefined; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +export type SomeType = +~~~~~~~~~~~~~~~~~~~~~~ + | null +~~~~~~~~~~ + | undefined +~~~~~~~~~~~~~~~ + | boolean; +~~~~~~~~~~~~~~ [0] + const someFunc = (): string | undefined | null => {} ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] From 9ab695d78f511526678ca7046c6af4926e0548d9 Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Mon, 25 Mar 2019 09:09:12 +0000 Subject: [PATCH 3/6] ts version --- test/rules/no-null-undefined-union/test.ts.lint | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint index a1717a06017..27604370ea3 100644 --- a/test/rules/no-null-undefined-union/test.ts.lint +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -1,3 +1,5 @@ +[typescript]: >= 2.4.0 + interface someInterface { a: number | undefined | null; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] From a465b26bfcd35780fabf0c3d1f6753a3d18c80ea Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Mon, 25 Mar 2019 10:56:32 +0000 Subject: [PATCH 4/6] consistent --- src/rules/noNullUndefinedUnionRule.ts | 52 +++++++++++++++++++-------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/rules/noNullUndefinedUnionRule.ts b/src/rules/noNullUndefinedUnionRule.ts index d9875a1dd6a..8176afdd7e2 100644 --- a/src/rules/noNullUndefinedUnionRule.ts +++ b/src/rules/noNullUndefinedUnionRule.ts @@ -1,8 +1,30 @@ /** - * @license Copyright 2018 Palantir Technologies, Inc. All rights reserved. + * @license + * Copyright 2019 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ -import * as utils from "tsutils"; +import { + isParameterDeclaration, + isPropertyDeclaration, + isPropertySignature, + isSignatureDeclaration, + isTypeAliasDeclaration, + isTypeReference, + isUnionType, + isVariableDeclaration, +} from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; @@ -44,9 +66,18 @@ function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker): void { } function getType(node: ts.Node, tc: ts.TypeChecker): ts.Type | undefined { - if (isVariableLikeDeclaration(node) && node.name.kind === ts.SyntaxKind.Identifier) { + // NOTE: This is a comprehensive intersection between `HasType` and has property `name`. + // The node name kind must be identifier, or else this rule will throw errors while descending. + if ( + (isVariableDeclaration(node) || + isParameterDeclaration(node) || + isPropertySignature(node) || + isPropertyDeclaration(node) || + isTypeAliasDeclaration(node)) && + node.name.kind === ts.SyntaxKind.Identifier + ) { return tc.getTypeAtLocation(node); - } else if (utils.isSignatureDeclaration(node)) { + } else if (isSignatureDeclaration(node)) { const signature = tc.getSignatureFromDeclaration(node); return signature === undefined ? undefined : signature.getReturnType(); } else { @@ -54,21 +85,12 @@ function getType(node: ts.Node, tc: ts.TypeChecker): ts.Type | undefined { } } -function isVariableLikeDeclaration(node: ts.Node): node is ts.VariableLikeDeclaration { - return ( - utils.isVariableDeclaration(node) || - utils.isParameterDeclaration(node) || - utils.isPropertySignature(node) || - utils.isTypeAliasDeclaration(node) - ); -} - function isNullUndefinedUnion(type: ts.Type): boolean { - if (utils.isTypeReference(type) && type.typeArguments !== undefined) { + if (isTypeReference(type) && type.typeArguments !== undefined) { return type.typeArguments.some(isNullUndefinedUnion); } - if (utils.isUnionType(type)) { + if (isUnionType(type)) { let hasNull = false; let hasUndefined = false; for (const subType of type.types) { From 7c5e5bc6c06b6e56fd3f01670ac07ddb1e76ce2b Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 27 Mar 2019 09:56:39 +0000 Subject: [PATCH 5/6] Update src/rules/noNullUndefinedUnionRule.ts Co-Authored-By: nrathi --- src/rules/noNullUndefinedUnionRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/noNullUndefinedUnionRule.ts b/src/rules/noNullUndefinedUnionRule.ts index 8176afdd7e2..d25b2f61186 100644 --- a/src/rules/noNullUndefinedUnionRule.ts +++ b/src/rules/noNullUndefinedUnionRule.ts @@ -39,7 +39,7 @@ export class Rule extends Lint.Rules.TypedRule { Enforcing the choice between the two allows the \`triple-equals\` rule to exist without exceptions, and is essentially a more flexible version of the \`no-null-keyword\` rule. `, - optionsDescription: "True if the rule should be enabled.", + optionsDescription: "Not configurable.", options: null, optionExamples: [true], type: "functionality", From e15be7e8c22410b13cedc507fc9540b28cebd079 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 27 Mar 2019 09:56:45 +0000 Subject: [PATCH 6/6] Update src/rules/noNullUndefinedUnionRule.ts Co-Authored-By: nrathi --- src/rules/noNullUndefinedUnionRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/noNullUndefinedUnionRule.ts b/src/rules/noNullUndefinedUnionRule.ts index d25b2f61186..d543594b2c4 100644 --- a/src/rules/noNullUndefinedUnionRule.ts +++ b/src/rules/noNullUndefinedUnionRule.ts @@ -66,7 +66,7 @@ function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker): void { } function getType(node: ts.Node, tc: ts.TypeChecker): ts.Type | undefined { - // NOTE: This is a comprehensive intersection between `HasType` and has property `name`. + // This is a comprehensive intersection between `HasType` and has property `name`. // The node name kind must be identifier, or else this rule will throw errors while descending. if ( (isVariableDeclaration(node) ||