no-redundant-undefined: enforce undefined for optional parameters (#…

…335)

* make optional properties require undefined

* rename to redundant-undefined

* add missed no-redundant-undefined references

* add fix for missing undefined

* improve fix to handle function types

* Remove checks on properties

Including undefined, or not, is now semantically meaningful when
exactOptionalPropertyTypes is true.

docs/redundant-undefined.md

@@ -0,0 +1,15 @@
+# redundant-undefined
+
+Avoid explicitly specifying `undefined` as a type for a parameter which is already optional.
+
+**Bad**:
+
+```ts
+function f(s?: string | undefined): void {}
+```
+
+**Good**:
+
+```ts
+function f(s?: string): void {}
+```

dtslint.json

@@ -10,6 +10,7 @@
         "no-dead-reference": true,
         "no-import-default-of-export-equals": true,
         "no-padding": true,
+        "redundant-undefined": true,
         "no-relative-import-in-test": true,
         "strict-export-declare-modifiers": true,
         "no-any-union": true,

src/rules/redundantUndefinedRule.ts

@@ -0,0 +1,40 @@
+import * as Lint from "tslint";
+import * as ts from "typescript";
+
+import { failure } from "../util";
+
+export class Rule extends Lint.Rules.AbstractRule {
+    static metadata: Lint.IRuleMetadata = {
+        ruleName: "redundant-undefined",
+        description: "Forbids optional parameters to include an explicit `undefined` in their type; requires it in optional properties.",
+        optionsDescription: "Not configurable.",
+        options: null,
+        type: "style",
+        typescriptOnly: true,
+    };
+
+    apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithFunction(sourceFile, walk);
+    }
+}
+
+function walk(ctx: Lint.WalkContext<void>): void {
+    if (ctx.sourceFile.fileName.includes('node_modules')) return;
+    ctx.sourceFile.forEachChild(function recur(node) {
+        if (node.kind === ts.SyntaxKind.UndefinedKeyword
+            && ts.isUnionTypeNode(node.parent!)
+            && isOptionalParameter(node.parent!.parent!)) {
+            ctx.addFailureAtNode(
+                node,
+                failure(
+                    Rule.metadata.ruleName,
+                    `Parameter is optional, so no need to include \`undefined\` in the type.`));
+        }
+        node.forEachChild(recur);
+    });
+}
+
+function isOptionalParameter(node: ts.Node): boolean {
+    return node.kind === ts.SyntaxKind.Parameter
+        && (node as ts.ParameterDeclaration).questionToken !== undefined;
+}

test/redundant-undefined/test.ts.lint

@@ -0,0 +1,10 @@
+function f(s?: string | undefined): void {}
+                        ~~~~~~~~~ [0]
+
+interface I {
+    ok?: string | undefined;
+    s?: string;
+    almost?: number | string;
+}
+
+[0]: Parameter is optional, so no need to include `undefined` in the type. See: https://github.com/Microsoft/dtslint/blob/master/docs/redundant-undefined.md

test/no-redundant-undefined/tslint.json → test/redundant-undefined/tslint.json

@@ -1,6 +1,6 @@
 {
   "rulesDirectory": ["../../bin/rules"],
   "rules": {
-    "no-redundant-undefined": true
+    "redundant-undefined": true
   }
 }