Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): add new rule no-empty-function #626

Merged
merged 2 commits into from Jun 20, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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