[FEAT] [no-unnecessary-class] Add rule (typescript-eslint#234)

j-f1 · JamesHenry · commit 9a2d3bc437ab · 2019-01-18T07:42:16.000-05:00
* Add `no-unnecessary-class` rule

* Add metadata

* ⬆️ eslint-docs
diff --git a/packages/eslint-plugin-typescript/README.md b/packages/eslint-plugin-typescript/README.md
@@ -48,8 +48,10 @@ This guarantees 100% compatibility between the plugin and the parser.
 
 <!-- Please run `npm run docs` to update this section -->
 <!-- begin rule list -->
+
 **Key**: :heavy_check_mark: = recommended, :wrench: = fixable
 
+<!-- prettier-ignore -->
 | Name | Description | :heavy_check_mark: | :wrench: |
 | ---- | ----------- | ------------------ | -------- |
 | [`typescript/adjacent-overload-signatures`](./docs/rules/adjacent-overload-signatures.md) | Require that member overloads be consecutive (`adjacent-overload-signatures` from TSLint) |  |  |
@@ -68,6 +70,7 @@ This guarantees 100% compatibility between the plugin and the parser.
 | [`typescript/no-array-constructor`](./docs/rules/no-array-constructor.md) | Disallow generic `Array` constructors |  | :wrench: |
 | [`typescript/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces (`no-empty-interface` from TSLint) |  |  |
 | [`typescript/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type (`no-any` from TSLint) |  |  |
+| [`typescript/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces (`no-unnecessary-class` from TSLint) |  |  |
 | [`typescript/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean. (`no-inferrable-types` from TSLint) |  | :wrench: |
 | [`typescript/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor`. (`no-misused-new` from TSLint) |  |  |
 | [`typescript/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces (`no-namespace` from TSLint) |  |  |
@@ -82,4 +85,5 @@ This guarantees 100% compatibility between the plugin and the parser.
 | [`typescript/prefer-interface`](./docs/rules/prefer-interface.md) | Prefer an interface declaration over a type literal (type T = { ... }) (`interface-over-type-literal` from TSLint) |  | :wrench: |
 | [`typescript/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules. (`no-internal-module` from TSLint) |  | :wrench: |
 | [`typescript/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations (`typedef-whitespace` from TSLint) |  | :wrench: |
+
 <!-- end rule list -->
diff --git a/packages/eslint-plugin-typescript/docs/rules/no-extraneous-class.md b/packages/eslint-plugin-typescript/docs/rules/no-extraneous-class.md
@@ -0,0 +1,64 @@
+# Forbids the use of classes as namespaces (no-extraneous-class)
+
+This rule warns when a class is accidentally used as a namespace.
+
+## Rule Details
+
+From TSLint’s docs:
+
+> Users who come from a Java-style OO language may wrap their utility functions in an extra class,
+> instead of putting them at the top level.
+
+Examples of **incorrect** code for this rule:
+
+```ts
+class EmptyClass {}
+
+class ConstructorOnly {
+  constructor() {
+    foo();
+  }
+}
+
+// Use an object instead:
+class StaticOnly {
+  static version = 42;
+  static hello() {
+    console.log("Hello, world!");
+  }
+}
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+class EmptyClass extends SuperClass {}
+
+class ParameterProperties {
+  constructor(public name: string) {}
+}
+
+const StaticOnly = {
+  version: 42,
+  hello() {
+    console.log("Hello, world!");
+  },
+};
+```
+
+### Options
+
+This rule accepts a single object option.
+
+- `constructorOnly: true` will silence warnings about classes containing only a constructor.
+- `allowEmpty: true` will silence warnings about empty classes.
+- `staticOnly: true` will silence warnings about classes containing only static members.
+
+## When Not To Use It
+
+You can disable this rule if you don’t have anyone who would make these kinds of mistakes on your 
+team or if you use classes as namespaces.
+
+## Compatibility
+
+[`no-unnecessary-class`](https://palantir.github.io/tslint/rules/no-unnecessary-class/) from TSLint
diff --git a/packages/eslint-plugin-typescript/lib/rules/no-extraneous-class.js b/packages/eslint-plugin-typescript/lib/rules/no-extraneous-class.js
@@ -0,0 +1,100 @@
+/**
+ * @fileoverview Forbids the use of classes as namespaces
+ * @author Jed Fox
+ */
+"use strict";
+
+const util = require("../util");
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        docs: {
+            description: "Forbids the use of classes as namespaces",
+            extraDescription: [util.tslintRule("no-unnecessary-class")],
+            category: "Best Practices",
+            recommended: false,
+            url: util.metaDocsUrl("no-extraneous-class"),
+        },
+        fixable: null,
+        schema: [
+            {
+                type: "object",
+                additionalProperties: false,
+                properties: {
+                    allowConstructorOnly: {
+                        type: "boolean",
+                    },
+                    allowEmpty: {
+                        type: "boolean",
+                    },
+                    allowStaticOnly: {
+                        type: "boolean",
+                    },
+                },
+            },
+        ],
+        messages: {
+            empty: "Unexpected empty class.",
+            onlyStatic: "Unexpected class with only static properties.",
+            onlyConstructor: "Unexpected class with only a constructor.",
+        },
+    },
+
+    create(context) {
+        const { allowConstructorOnly, allowEmpty, allowStaticOnly } =
+            context.options[0] || {};
+
+        return {
+            ClassBody(node) {
+                const { id, superClass } = node.parent;
+
+                if (superClass) return;
+
+                if (node.body.length === 0) {
+                    if (allowEmpty) return;
+                    context.report({ node: id, messageId: "empty" });
+                    return;
+                }
+
+                let onlyStatic = true;
+                let onlyConstructor = true;
+
+                for (const prop of node.body) {
+                    if (prop.kind === "constructor") {
+                        if (
+                            prop.value.params.some(
+                                param => param.type === "TSParameterProperty"
+                            )
+                        ) {
+                            onlyConstructor = false;
+                            onlyStatic = false;
+                        }
+                    } else {
+                        onlyConstructor = false;
+                        if (!prop.static) {
+                            onlyStatic = false;
+                        }
+                    }
+                    if (!(onlyStatic || onlyConstructor)) break;
+                }
+
+                if (onlyConstructor) {
+                    if (!allowConstructorOnly) {
+                        context.report({
+                            node: id,
+                            messageId: "onlyConstructor",
+                        });
+                    }
+                    return;
+                }
+                if (onlyStatic && !allowStaticOnly) {
+                    context.report({ node: id, messageId: "onlyStatic" });
+                }
+            },
+        };
+    },
+};
diff --git a/packages/eslint-plugin-typescript/package.json b/packages/eslint-plugin-typescript/package.json
@@ -28,7 +28,7 @@
     "eslint": "^5.9.0",
     "eslint-config-eslint": "^5.0.1",
     "eslint-config-prettier": "^3.3.0",
-    "eslint-docs": "^0.2.3",
+    "eslint-docs": "^0.2.6",
     "eslint-plugin-eslint-plugin": "^2.0.0",
     "eslint-plugin-node": "^8.0.0",
     "eslint-plugin-prettier": "^3.0.0",
diff --git a/packages/eslint-plugin-typescript/tests/lib/rules/no-extraneous-class.js b/packages/eslint-plugin-typescript/tests/lib/rules/no-extraneous-class.js
@@ -0,0 +1,125 @@
+/**
+ * @fileoverview Forbids the use of classes as namespaces
+ * Some tests adapted from  https://github.com/palantir/tslint/tree/c7fc99b5/test/rules/no-unnecessary-class
+ * @author Jed Fox
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../lib/rules/no-extraneous-class"),
+    RuleTester = require("eslint").RuleTester;
+
+const empty = { messageId: "empty", type: "Identifier" };
+const onlyStatic = { messageId: "onlyStatic", type: "Identifier" };
+const onlyConstructor = { messageId: "onlyConstructor", type: "Identifier" };
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+    parser: "typescript-eslint-parser",
+});
+
+ruleTester.run("no-extraneous-class", rule, {
+    valid: [
+        `
+class Foo {
+    public prop = 1;
+    constructor() {}
+}
+`.trim(),
+        `
+export class CClass extends BaseClass {
+    public static helper(): void {}
+    private static privateHelper(): boolean {
+        return true;
+    }
+    constructor() {}
+}
+`.trim(),
+        `
+class Foo {
+   constructor(
+     public bar: string
+   ) {}
+}
+`.trim(),
+        {
+            code: "class Foo {}",
+            options: [{ allowEmpty: true }],
+        },
+        {
+            code: `
+class Foo {
+    constructor() {}
+}
+`.trim(),
+            options: [{ allowConstructorOnly: true }],
+        },
+        {
+            code: `
+export class Bar {
+    public static helper(): void {}
+    private static privateHelper(): boolean {
+        return true;
+    }
+}
+`.trim(),
+            options: [{ allowStaticOnly: true }],
+        },
+    ],
+
+    invalid: [
+        {
+            code: "class Foo {}",
+            errors: [empty],
+        },
+        {
+            code: `
+class Foo {
+    public prop = 1;
+    constructor() {
+        class Bar {
+            static PROP = 2;
+        }
+    }
+}
+export class Bar {
+    public static helper(): void {}
+    private static privateHelper(): boolean {
+        return true;
+    }
+}
+`.trim(),
+            errors: [onlyStatic, onlyStatic],
+        },
+        {
+            code: `
+class Foo {
+    constructor() {}
+}
+`.trim(),
+            errors: [onlyConstructor],
+        },
+        {
+            code: `
+export class AClass {
+    public static helper(): void {}
+    private static privateHelper(): boolean {
+        return true;
+    }
+    constructor() {
+        class nestedClass {
+        }
+    }
+}
+
+`.trim(),
+            errors: [onlyStatic, empty],
+        },
+    ],
+});
diff --git a/packages/eslint-plugin-typescript/yarn.lock b/packages/eslint-plugin-typescript/yarn.lock
@@ -707,10 +707,10 @@ eslint-config-prettier@^3.3.0:
   dependencies:
     get-stdin "^6.0.0"
 
-eslint-docs@^0.2.3:
-  version "0.2.3"
-  resolved "https://registry.yarnpkg.com/eslint-docs/-/eslint-docs-0.2.3.tgz#71a583443c1d74bfc7c1af31b3249e258037309c"
-  integrity sha512-6afUYuK65TOPliMul0yIGwCiKWNEqk54RbuNZhJKt1mTVHyG0D2Zbbpa7yTT1/TiCfnUkfQa7TA/l7Jgu998Dw==
+eslint-docs@^0.2.6:
+  version "0.2.6"
+  resolved "https://registry.yarnpkg.com/eslint-docs/-/eslint-docs-0.2.6.tgz#40bfbf62e22f948f02893e90280abf36ff260293"
+  integrity sha512-XyuBu/5d51ZecfYY+cantvWDLloo5j38sHJZE3VNU0u4PJCaOYvscWXBX/9ADzXBDJF5TvBEXCqPDnK/WrOiTA==
   dependencies:
     chalk "^2.4.1"
     detect-newline "^2.1.0"
