diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index ff6a11b3726..1d0e220adbd 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -142,6 +142,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | | | [`@typescript-eslint/no-angle-bracket-type-assertion`](./docs/rules/no-angle-bracket-type-assertion.md) | Enforces the use of `as Type` assertions instead of `` assertions | :heavy_check_mark: | | | | [`@typescript-eslint/no-array-constructor`](./docs/rules/no-array-constructor.md) | Disallow generic `Array` constructors | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/no-empty-function`](./docs/rules/no-empty-function.md) | Disallow empty functions | | | | | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | | | | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | | | | [`@typescript-eslint/no-extra-parens`](./docs/rules/no-extra-parens.md) | Disallow unnecessary parentheses | | :wrench: | | diff --git a/packages/eslint-plugin/docs/rules/no-empty-function.md b/packages/eslint-plugin/docs/rules/no-empty-function.md new file mode 100644 index 00000000000..e06961d4259 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-empty-function.md @@ -0,0 +1,47 @@ +# Disallow Empty Functions (@typescript-eslint/no-empty-function) + +Empty functions can reduce readability because readers need to guess whether it’s intentional or not. So writing a clear comment for empty functions is a good practice. + +## Rule Details + +The `@typescript-eslint/no-empty-function` rule extends the `no-empty-function` rule from ESLint core, and adds support for handling Typescript specific code that would otherwise trigger the rule. + +One example of valid Typescript specific code that would otherwise trigger the `no-empty-function` rule is the use of [parameter properties](https://www.typescriptlang.org/docs/handbook/classes.html#parameter-properties) in constructor functions: + +```typescript +class Person { + constructor(private firstName: string, private surname: string) {} +} +``` + +The above code is functionally equivalent to: + +```typescript +class Person { + private firstName: string; + private surname: string; + + constructor(firstName: string, surname: string) { + this.firstName = firstName; + this.surname = surname; + } +} +``` + +Parameter properties enable both the _declaration_ and _initialization_ of member properties in a single location, avoiding the boilerplate & duplication that is common when initializing member properties from parameter values in a constructor function. + +In these cases, although the constructor has an empty function body, it is technically valid and should not trigger an error. + +See the [ESLint documentation](https://eslint.org/docs/rules/no-empty-function) for more details on the `no-empty-function` rule. + +## Rule Changes + +```cjson +{ + // note you must disable the base rule as it can report incorrect errors + "no-empty-function": "off", + "@typescript-eslint/no-empty-function": "error" +} +``` + +Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/no-empty-function.md) diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index c92f6a36d8b..ec00de43335 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -16,6 +16,7 @@ import memberNaming from './member-naming'; import memberOrdering from './member-ordering'; import noAngleBracketTypeAssertion from './no-angle-bracket-type-assertion'; import noArrayConstructor from './no-array-constructor'; +import noEmptyFunction from './no-empty-function'; import noEmptyInterface from './no-empty-interface'; import noExplicitAny from './no-explicit-any'; import noExtraParens from './no-extra-parens'; @@ -73,6 +74,7 @@ export default { 'member-ordering': memberOrdering, 'no-angle-bracket-type-assertion': noAngleBracketTypeAssertion, 'no-array-constructor': noArrayConstructor, + 'no-empty-function': noEmptyFunction, 'no-empty-interface': noEmptyInterface, 'no-explicit-any': noExplicitAny, 'no-extra-parens': noExtraParens, diff --git a/packages/eslint-plugin/src/rules/no-empty-function.ts b/packages/eslint-plugin/src/rules/no-empty-function.ts new file mode 100644 index 00000000000..d64568a874f --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-empty-function.ts @@ -0,0 +1,104 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import baseRule from 'eslint/lib/rules/no-empty-function'; +import * as util from '../util'; + +type Options = util.InferOptionsTypeFromRule; +type MessageIds = util.InferMessageIdsTypeFromRule; + +export default util.createRule({ + name: 'no-empty-function', + meta: { + type: 'suggestion', + docs: { + description: 'Disallow empty functions', + category: 'Best Practices', + recommended: false, + }, + schema: baseRule.meta.schema, + messages: baseRule.meta.messages, + }, + defaultOptions: [ + { + allow: [], + }, + ], + create(context) { + const rules = baseRule.create(context); + + /** + * Checks if the node is a constructor + * @param node the node to ve validated + * @returns true if the node is a constructor + * @private + */ + function isConstructor( + node: TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, + ): boolean { + return !!( + node.parent && + node.parent.type === 'MethodDefinition' && + node.parent.kind === 'constructor' + ); + } + + /** + * Check if the method body is empty + * @param node the node to be validated + * @returns true if the body is empty + * @private + */ + function isBodyEmpty( + node: TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, + ): boolean { + return !node.body || node.body.body.length === 0; + } + + /** + * Check if method has parameter properties + * @param node the node to be validated + * @returns true if the body has parameter properties + * @private + */ + function hasParameterProperties( + node: TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, + ): boolean { + return ( + node.params && + node.params.some( + param => param.type === AST_NODE_TYPES.TSParameterProperty, + ) + ); + } + + /** + * Checks if the method is a concise constructor (no function body, but has parameter properties) + * @param node the node to be validated + * @returns true if the method is a concise constructor + * @private + */ + function isConciseConstructor( + node: TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, + ) { + // Check TypeScript specific nodes + return ( + isConstructor(node) && isBodyEmpty(node) && hasParameterProperties(node) + ); + } + + return { + FunctionDeclaration(node: TSESTree.FunctionDeclaration) { + if (!isConciseConstructor(node)) { + rules.FunctionDeclaration(node); + } + }, + FunctionExpression(node: TSESTree.FunctionExpression) { + if (!isConciseConstructor(node)) { + rules.FunctionExpression(node); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-empty-function.test.ts b/packages/eslint-plugin/tests/rules/no-empty-function.test.ts new file mode 100644 index 00000000000..4b40c170867 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-empty-function.test.ts @@ -0,0 +1,69 @@ +import rule from '../../src/rules/no-empty-function'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-empty-function', rule, { + valid: [ + { + code: `class Person { + private name: string + constructor(name: string) { + this.name = name; + } + }`, + }, + { + code: `class Person { + constructor(private name: string) {} + }`, + }, + { + code: `class Person { + constructor(name: string) {} + }`, + options: [{ allow: ['constructors'] }], + }, + { + code: `class Person { + otherMethod(name: string) {} + }`, + options: [{ allow: ['methods'] }], + }, + ], + + invalid: [ + { + code: `class Person { + constructor(name: string) {} + }`, + errors: [ + { + messageId: 'unexpected', + data: { + name: 'constructor', + }, + line: 2, + column: 35, + }, + ], + }, + { + code: `class Person { + otherMethod(name: string) {} + }`, + errors: [ + { + messageId: 'unexpected', + data: { + name: "method 'otherMethod'", + }, + line: 2, + column: 35, + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index 774f0842aa5..df9131dae4d 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -155,6 +155,24 @@ declare module 'eslint/lib/rules/no-dupe-args' { export = rule; } +declare module 'eslint/lib/rules/no-empty-function' { + import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; + + const rule: TSESLint.RuleModule< + 'unexpected', + [ + { + allow?: string[]; + } + ], + { + FunctionDeclaration(node: TSESTree.FunctionDeclaration): void; + FunctionExpression(node: TSESTree.FunctionExpression): void; + } + >; + export = rule; +} + declare module 'eslint/lib/rules/no-implicit-globals' { import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';