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): [no-empty-func] private/protected construct #1267

Merged
Show file tree
Hide file tree
Changes from all commits
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
33 changes: 33 additions & 0 deletions packages/eslint-plugin/docs/rules/no-empty-function.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,37 @@ See the [ESLint documentation](https://eslint.org/docs/rules/no-empty-function)
}
```

## Options

This rule has an object option:

- `allow` (`string[]`)
- `"protected-constructors"` - Protected class constructors.
- `"private-constructors"` - Private class constructors.
a-tarasyuk marked this conversation as resolved.
Show resolved Hide resolved
- [See the other options allowed](https://github.com/eslint/eslint/blob/master/docs/rules/no-empty-function.md#options)

#### allow: protected-constructors

Examples of **correct** code for the `{ "allow": ["protected-constructors"] }` option:

```ts
/*eslint @typescript-eslint/no-empty-function: ["error", { "allow": ["protected-constructors"] }]*/

class Foo {
protected constructor() {}
}
```

#### allow: private-constructors

Examples of **correct** code for the `{ "allow": ["private-constructors"] }` option:

```ts
/*eslint @typescript-eslint/no-empty-function: ["error", { "allow": ["private-constructors"] }]*/

class Foo {
private constructor() {}
}
```

<sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/no-empty-function.md)</sup>
87 changes: 58 additions & 29 deletions packages/eslint-plugin/src/rules/no-empty-function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,32 @@ import * as util from '../util';
type Options = util.InferOptionsTypeFromRule<typeof baseRule>;
type MessageIds = util.InferMessageIdsTypeFromRule<typeof baseRule>;

const schema = util.deepMerge(
Array.isArray(baseRule.meta.schema)
? baseRule.meta.schema[0]
: baseRule.meta.schema,
{
properties: {
allow: {
items: {
enum: [
'functions',
'arrowFunctions',
'generatorFunctions',
'methods',
'generatorMethods',
'getters',
'setters',
'constructors',
'private-constructors',
'protected-constructors',
],
},
},
},
},
);

export default util.createRule<Options, MessageIds>({
name: 'no-empty-function',
meta: {
Expand All @@ -17,32 +43,21 @@ export default util.createRule<Options, MessageIds>({
category: 'Best Practices',
recommended: 'error',
},
schema: baseRule.meta.schema,
schema: [schema],
messages: baseRule.meta.messages,
},
defaultOptions: [
{
allow: [],
},
],
create(context) {
create(context, [{ allow = [] }]) {
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'
);
}
const isAllowedProtectedConstructors = allow.includes(
'protected-constructors',
);
const isAllowedPrivateConstructors = allow.includes('private-constructors');

/**
* Check if the method body is empty
Expand Down Expand Up @@ -74,30 +89,44 @@ export default util.createRule<Options, MessageIds>({
}

/**
* 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
* @returns true if the constructor is allowed to be empty
* @private
*/
function isConciseConstructor(
function isAllowedEmptyConstructor(
node: TSESTree.FunctionDeclaration | TSESTree.FunctionExpression,
): boolean {
// Check TypeScript specific nodes
return (
isConstructor(node) && isBodyEmpty(node) && hasParameterProperties(node)
);
const parent = node.parent;
if (
isBodyEmpty(node) &&
parent?.type === 'MethodDefinition' &&
parent.kind === 'constructor'
) {
const { accessibility } = parent;

return (
// allow protected constructors
(accessibility === 'protected' && isAllowedProtectedConstructors) ||
// allow private constructors
(accessibility === 'private' && isAllowedPrivateConstructors) ||
// allow constructors which have parameter properties
hasParameterProperties(node)
);
}

return false;
}

return {
FunctionDeclaration(node): void {
if (!isConciseConstructor(node)) {
rules.FunctionDeclaration(node);
}
rules.FunctionDeclaration(node);
},
FunctionExpression(node): void {
if (!isConciseConstructor(node)) {
rules.FunctionExpression(node);
if (isAllowedEmptyConstructor(node)) {
return;
}

rules.FunctionExpression(node);
},
};
},
Expand Down
73 changes: 73 additions & 0 deletions packages/eslint-plugin/tests/rules/no-empty-function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,29 @@ ruleTester.run('no-empty-function', rule, {
}`,
options: [{ allow: ['methods'] }],
},
{
code: `
class Foo {
private constructor() {}
}
`,
options: [{ allow: ['private-constructors'] }],
},
{
code: `
class Foo {
protected constructor() {}
}
`,
options: [{ allow: ['protected-constructors'] }],
},
{
code: `
function foo() {
const a = null;
}
`,
},
],

invalid: [
Expand Down Expand Up @@ -65,5 +88,55 @@ ruleTester.run('no-empty-function', rule, {
},
],
},
{
code: `
class Foo {
private constructor() {}
}
`,
errors: [
{
messageId: 'unexpected',
data: {
name: 'constructor',
},
line: 3,
column: 25,
},
],
},
{
code: `
class Foo {
protected constructor() {}
}
`,
errors: [
{
messageId: 'unexpected',
data: {
name: 'constructor',
},
line: 3,
column: 27,
},
],
},
{
code: `
function foo() {
}
`,
errors: [
{
messageId: 'unexpected',
data: {
name: "function 'foo'",
},
line: 2,
column: 16,
},
],
},
],
});