From 4fc2873a08b32b11c629400751815213e6d33f9b Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Tue, 2 Apr 2019 12:22:53 +0100 Subject: [PATCH 1/7] done --- src/rules/noNullUndefinedUnionRule.ts | 20 ++------- .../no-null-undefined-union/test.ts.lint | 44 +++++++++++-------- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/rules/noNullUndefinedUnionRule.ts b/src/rules/noNullUndefinedUnionRule.ts index 2353a4243de..35ae4da3d70 100644 --- a/src/rules/noNullUndefinedUnionRule.ts +++ b/src/rules/noNullUndefinedUnionRule.ts @@ -16,14 +16,10 @@ */ import { - isParameterDeclaration, - isPropertyDeclaration, - isPropertySignature, isSignatureDeclaration, - isTypeAliasDeclaration, isTypeReference, isUnionType, - isVariableDeclaration, + isUnionTypeNode, } from "tsutils"; import * as ts from "typescript"; @@ -66,18 +62,10 @@ function walk(ctx: Lint.WalkContext, tc: ts.TypeChecker): void { } function getType(node: ts.Node, tc: ts.TypeChecker): ts.Type | undefined { - // 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 - ) { + if (isUnionTypeNode(node)) { return tc.getTypeAtLocation(node); - } else if (isSignatureDeclaration(node)) { + } else if (isSignatureDeclaration(node) && node.type === undefined) { + // Explicit types should be handled by the first case. const signature = tc.getSignatureFromDeclaration(node); return signature === undefined ? undefined : signature.getReturnType(); } else { diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint index 27604370ea3..de369130914 100644 --- a/test/rules/no-null-undefined-union/test.ts.lint +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -1,39 +1,45 @@ [typescript]: >= 2.4.0 -interface someInterface { - a: number | undefined | null; - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] - b: boolean; -} - -const c: string | null | undefined; - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] - export type SomeType = -~~~~~~~~~~~~~~~~~~~~~~ + | null -~~~~~~~~~~ + ~~~~ | undefined ~~~~~~~~~~~~~~~ | boolean; -~~~~~~~~~~~~~~ [0] +~~~~~~~~~~~~~ [0] + +const x: SomeType; // Ignore implicit types of variables. + +const someFunc = (): SomeType => {} + +function(foo: SomeType) {} + +const y: string | null | undefined; + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +interface someInterface { + foo: number | undefined | null; + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + bar: boolean; +} const someFunc = (): string | undefined | null => {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] const someFunc = (foo: null | string | undefined, bar: boolean) => {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] function someFunc(): number | undefined | null {} -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] function someFunc(): Promise {} // error -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] function someFunc(bar: boolean, foo: null | number | undefined) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] -function someFunc() { +function testFunc() { ~~~~~~~~~~~~~~~~~~~~~ const somePredicate = (): boolean => true; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -52,4 +58,6 @@ function someFunc() { } ~ [0] +const z = testFunc(); + [0]: Union type cannot include both 'null' and 'undefined'. From bf3a0ab767efb2a6a042963a33f203e873757799 Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Tue, 2 Apr 2019 12:31:17 +0100 Subject: [PATCH 2/7] lint --- src/configuration.ts | 4 +--- src/rules/noNullUndefinedUnionRule.ts | 7 +------ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index 6639ab0f1ef..712f8cc55eb 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -429,7 +429,6 @@ 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 { @@ -508,12 +507,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 = + // tslint:disable-next-line no-null-undefined-union | null | undefined | boolean diff --git a/src/rules/noNullUndefinedUnionRule.ts b/src/rules/noNullUndefinedUnionRule.ts index 35ae4da3d70..d67d5803cbf 100644 --- a/src/rules/noNullUndefinedUnionRule.ts +++ b/src/rules/noNullUndefinedUnionRule.ts @@ -15,12 +15,7 @@ * limitations under the License. */ -import { - isSignatureDeclaration, - isTypeReference, - isUnionType, - isUnionTypeNode, -} from "tsutils"; +import { isSignatureDeclaration, isTypeReference, isUnionType, isUnionTypeNode } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; From bea475b26d775c9cd75c17660c0d83c6280e6651 Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Tue, 2 Apr 2019 12:32:13 +0100 Subject: [PATCH 3/7] remove comment --- test/rules/no-null-undefined-union/test.ts.lint | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint index de369130914..30601f15d52 100644 --- a/test/rules/no-null-undefined-union/test.ts.lint +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -9,7 +9,7 @@ export type SomeType = | boolean; ~~~~~~~~~~~~~ [0] -const x: SomeType; // Ignore implicit types of variables. +const x: SomeType; const someFunc = (): SomeType => {} From 0afcc06a5457678222d2ab53a96d54c0257784a0 Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Tue, 2 Apr 2019 12:36:38 +0100 Subject: [PATCH 4/7] better test --- test/rules/no-null-undefined-union/test.ts.lint | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint index 30601f15d52..426587600cc 100644 --- a/test/rules/no-null-undefined-union/test.ts.lint +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -13,17 +13,17 @@ const x: SomeType; const someFunc = (): SomeType => {} -function(foo: SomeType) {} - -const y: string | null | undefined; - ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] - -interface someInterface { +interface SomeInterface { foo: number | undefined | null; ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] bar: boolean; } +function(foo: SomeInterface) {} + +const y: string | null | undefined; + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + const someFunc = (): string | undefined | null => {} ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] From 94094f0e95ba498ebc58d15f9d44bd3a03c9f5c5 Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Tue, 2 Apr 2019 14:53:54 +0100 Subject: [PATCH 5/7] more tests --- test/rules/no-null-undefined-union/test.ts.lint | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint index 426587600cc..7b069479104 100644 --- a/test/rules/no-null-undefined-union/test.ts.lint +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -1,6 +1,6 @@ [typescript]: >= 2.4.0 -export type SomeType = +type SomeType = | null ~~~~ @@ -60,4 +60,16 @@ function testFunc() { const z = testFunc(); +type Text = string | null + +interface TextInterface { + foo?: Text +} + +interface SuperTextInterface { + bar?: Text | number +} + +function someFunc(foo?: Text, bar?: Text | number) {} + [0]: Union type cannot include both 'null' and 'undefined'. From 4971c6cab41765682074a12ca92b9545f80ecdd6 Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Thu, 4 Apr 2019 15:56:56 +0100 Subject: [PATCH 6/7] done --- src/rules/noNullUndefinedUnionRule.ts | 6 ++- .../no-null-undefined-union/test.ts.lint | 50 +++++++++++-------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/rules/noNullUndefinedUnionRule.ts b/src/rules/noNullUndefinedUnionRule.ts index d67d5803cbf..312e796cb7b 100644 --- a/src/rules/noNullUndefinedUnionRule.ts +++ b/src/rules/noNullUndefinedUnionRule.ts @@ -24,11 +24,15 @@ 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.", + description: Lint.Utils.dedent` + Disallows explicitly declared or implicitly returned 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. + Optional parameters are not considered to have the type \`undefined\`. `, optionsDescription: "Not configurable.", options: null, diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint index 7b069479104..3de1734ad04 100644 --- a/test/rules/no-null-undefined-union/test.ts.lint +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -1,5 +1,7 @@ [typescript]: >= 2.4.0 +// Catches explicit union types. + type SomeType = | null @@ -9,19 +11,7 @@ type SomeType = | boolean; ~~~~~~~~~~~~~ [0] -const x: SomeType; - -const someFunc = (): SomeType => {} - -interface SomeInterface { - foo: number | undefined | null; - ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] - bar: boolean; -} - -function(foo: SomeInterface) {} - -const y: string | null | undefined; +const c: string | null | undefined; ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] const someFunc = (): string | undefined | null => {} @@ -30,15 +20,20 @@ const someFunc = (): string | undefined | null => {} const someFunc = (foo: null | string | undefined, bar: boolean) => {} ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] -function someFunc(): number | undefined | null {} - ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +interface SomeInterface { + foo: number | null | undefined; + ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + bar: boolean; +} -function someFunc(): Promise {} // error +function someFunc(): Promise {} // error ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] -function someFunc(bar: boolean, foo: null | number | undefined) {} +function someFunc(bar: boolean, foo: undefined | number | null) {} ~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +// Catches implicit return types. + function testFunc() { ~~~~~~~~~~~~~~~~~~~~~ const somePredicate = (): boolean => true; @@ -58,18 +53,33 @@ function testFunc() { } ~ [0] -const z = testFunc(); +// Does not consider ? as shorthand for undefined. type Text = string | null interface TextInterface { - foo?: Text + foo?: Text } interface SuperTextInterface { - bar?: Text | number + bar?: Text | number } function someFunc(foo?: Text, bar?: Text | number) {} +// Ignores implicit union types. + +const x: SomeType; + +const someFunc = (): SomeType => {} + +function(foo: SomeInterface) {} + +const y = testFunc(); + +// Unless they are explicitly unioned. + +const z: Text | undefined; + ~~~~~~~~~~~~~~~~ [0]; + [0]: Union type cannot include both 'null' and 'undefined'. From f09665baac69911bad896f4d5efd23122cbaa6be Mon Sep 17 00:00:00 2001 From: Neha Rathi Date: Thu, 4 Apr 2019 16:00:23 +0100 Subject: [PATCH 7/7] typo --- test/rules/no-null-undefined-union/test.ts.lint | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rules/no-null-undefined-union/test.ts.lint b/test/rules/no-null-undefined-union/test.ts.lint index 3de1734ad04..6361e08be1b 100644 --- a/test/rules/no-null-undefined-union/test.ts.lint +++ b/test/rules/no-null-undefined-union/test.ts.lint @@ -80,6 +80,6 @@ const y = testFunc(); // Unless they are explicitly unioned. const z: Text | undefined; - ~~~~~~~~~~~~~~~~ [0]; + ~~~~~~~~~~~~~~~~ [0] [0]: Union type cannot include both 'null' and 'undefined'.