Relax "no-null-undefined-union" rule. (palantir#4625)

Author: nrathi

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<IOptions> {
@@ -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

src/rules/noNullUndefinedUnionRule.ts

@@ -15,16 +15,7 @@
  * limitations under the License.
  */
 
-import {
-    isParameterDeclaration,
-    isPropertyDeclaration,
-    isPropertySignature,
-    isSignatureDeclaration,
-    isTypeAliasDeclaration,
-    isTypeReference,
-    isUnionType,
-    isVariableDeclaration,
-} from "tsutils";
+import { isSignatureDeclaration, isTypeReference, isUnionType, isUnionTypeNode } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
@@ -33,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,
@@ -66,18 +61,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 {

test/rules/no-null-undefined-union/test.ts.lint

@@ -1,39 +1,40 @@
 [typescript]: >= 2.4.0
 
-interface someInterface {
-    a: number | undefined | null;
-    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
-    b: boolean;
-}
+// Catches explicit union types.
 
-const c: string | null | undefined;
-      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+type SomeType =
 
-export type SomeType =
-~~~~~~~~~~~~~~~~~~~~~~
     | null
-~~~~~~~~~~
+      ~~~~
     | undefined
 ~~~~~~~~~~~~~~~
     | boolean;
-~~~~~~~~~~~~~~ [0]
+~~~~~~~~~~~~~ [0]
+
+const c: string | null | undefined;
+         ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
 
 const someFunc = (): string | undefined | null => {}
-                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+                     ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
 
 const someFunc = (foo: null | string | undefined, bar: boolean) => {}
-                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+                       ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+
+interface SomeInterface {
+    foo: number | null | undefined;
+         ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+    bar: boolean;
+}
 
-function someFunc(): number | undefined | null {}
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+function someFunc(): Promise<number | undefined | null> {} // error
+                             ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
 
-function someFunc(): Promise<number | null | undefined> {} // error
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+function someFunc(bar: boolean, foo: undefined | number | null) {}
+                                     ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
 
-function someFunc(bar: boolean, foo: null | number | undefined) {}
-                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+// Catches implicit return types.
 
-function someFunc() {
+function testFunc() {
 ~~~~~~~~~~~~~~~~~~~~~
     const somePredicate = (): boolean => true;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -52,4 +53,33 @@ function someFunc() {
 }
 ~ [0]
 
+// Does not consider ? as shorthand for undefined.
+
+type Text = string | null
+
+interface TextInterface {
+    foo?: Text
+}
+
+interface SuperTextInterface {
+    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'.