Skip to content

Commit

Permalink
feat(eslint-plugin): add new rule no-empty-function (#626)
Browse files Browse the repository at this point in the history
  • Loading branch information
scottohara authored and bradzacher committed Jun 20, 2019
1 parent aa206c4 commit 747bfcb
Show file tree
Hide file tree
Showing 6 changed files with 241 additions and 0 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -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 `<Type>` 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: | |
Expand Down
47 changes: 47 additions & 0 deletions 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"
}
```

<sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/no-empty-function.md)</sup>
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
104 changes: 104 additions & 0 deletions 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<typeof baseRule>;
type MessageIds = util.InferMessageIdsTypeFromRule<typeof baseRule>;

export default util.createRule<Options, MessageIds>({
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);
}
},
};
},
});
69 changes: 69 additions & 0 deletions 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,
},
],
},
],
});
18 changes: 18 additions & 0 deletions packages/eslint-plugin/typings/eslint-rules.d.ts
Expand Up @@ -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';

Expand Down

0 comments on commit 747bfcb

Please sign in to comment.