diff --git a/packages/eslint-plugin-typescript/README.md b/packages/eslint-plugin-typescript/README.md index 3a1d6426eac..937d8ee73e1 100644 --- 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. + **Key**: :heavy_check_mark: = recommended, :wrench: = fixable + | 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: | + diff --git a/packages/eslint-plugin-typescript/docs/rules/no-extraneous-class.md b/packages/eslint-plugin-typescript/docs/rules/no-extraneous-class.md new file mode 100644 index 00000000000..277bd38b7f8 --- /dev/null +++ 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 new file mode 100644 index 00000000000..ca97e200ca1 --- /dev/null +++ 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 index 668b18c2309..fcaa3264f13 100644 --- 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 new file mode 100644 index 00000000000..59d24a05f4a --- /dev/null +++ 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 index a899dd83257..91ea910bc45 100644 --- 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"