Skip to content

Commit

Permalink
feat(eslint-plugin): [class-methods-use-this] add extension rule (#6457)
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher committed Jul 18, 2023
1 parent f813147 commit 18ea3b1
Show file tree
Hide file tree
Showing 12 changed files with 1,053 additions and 4 deletions.
4 changes: 2 additions & 2 deletions packages/eslint-plugin/TSLINT_RULE_ALTERNATIVES.md
Expand Up @@ -185,7 +185,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th
| [`one-line`] | 🌟 | [`brace-style`][brace-style] or [Prettier] |
| [`one-variable-per-declaration`] | 🌟 | [`one-var`][one-var] |
| [`ordered-imports`] | 🌓 | [`import/order`] |
| [`prefer-function-over-method`] | 🌟 | [`class-methods-use-this`][class-methods-use-this] |
| [`prefer-function-over-method`] | 🌟 | [`@typescript-eslint/class-methods-use-this`] |
| [`prefer-method-signature`] || [`@typescript-eslint/method-signature-style`] |
| [`prefer-switch`] | 🛑 | N/A |
| [`prefer-template`] | 🌟 | [`prefer-template`][prefer-template] |
Expand Down Expand Up @@ -566,7 +566,6 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[object-shorthand]: https://eslint.org/docs/rules/object-shorthand
[brace-style]: https://eslint.org/docs/rules/brace-style
[one-var]: https://eslint.org/docs/rules/one-var
[class-methods-use-this]: https://eslint.org/docs/rules/class-methods-use-this
[prefer-template]: https://eslint.org/docs/rules/prefer-template
[quotes]: https://eslint.org/docs/rules/quotes
[semi]: https://eslint.org/docs/rules/semi
Expand Down Expand Up @@ -598,6 +597,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@typescript-eslint/await-thenable`]: https://typescript-eslint.io/rules/await-thenable
[`@typescript-eslint/ban-types`]: https://typescript-eslint.io/rules/ban-types
[`@typescript-eslint/ban-ts-comment`]: https://typescript-eslint.io/rules/ban-ts-comment
[`@typescript-eslint/class-methods-use-this`]: https://typescript-eslint.io/rules/class-methods-use-this
[`@typescript-eslint/consistent-type-assertions`]: https://typescript-eslint.io/rules/consistent-type-assertions
[`@typescript-eslint/consistent-type-definitions`]: https://typescript-eslint.io/rules/consistent-type-definitions
[`@typescript-eslint/explicit-member-accessibility`]: https://typescript-eslint.io/rules/explicit-member-accessibility
Expand Down
6 changes: 5 additions & 1 deletion packages/eslint-plugin/docs/rules/TEMPLATE.md
@@ -1,6 +1,10 @@
---
description: '<Description from rule metadata here>'
---

> 🛑 This file is source code, not the primary documentation location! 🛑
>
> See **https://typescript-eslint.io/rules/your-rule-name** for documentation.
> See **https://typescript-eslint.io/rules/RULE_NAME_REPLACEME** for documentation.
## Examples

Expand Down
57 changes: 57 additions & 0 deletions packages/eslint-plugin/docs/rules/class-methods-use-this.md
@@ -0,0 +1,57 @@
---
description: 'Enforce that class methods utilize `this`.'
---

> 🛑 This file is source code, not the primary documentation location! 🛑
>
> See **https://typescript-eslint.io/rules/class-methods-use-this** for documentation.
## Examples

This rule extends the base [`eslint/class-methods-use-this`](https://eslint.org/docs/rules/class-methods-use-this) rule.
It adds support for ignoring `override` methods or methods on classes that implement an interface.

## Options

This rule adds the following options:

```ts
interface Options extends BaseClassMethodsUseThisOptions {
ignoreOverrideMethods?: boolean;
ignoreClassesThatImplementAnInterface?: boolean;
}

const defaultOptions: Options = {
...baseClassMethodsUseThisOptions,
ignoreOverrideMethods: false,
ignoreClassesThatImplementAnInterface: false,
};
```

### `ignoreOverrideMethods`

Makes the rule to ignores any class member explicitly marked with `override`.

Example of a correct code when `ignoreOverrideMethods` is set to `true`:

```ts
class X {
override method() {}
override property = () => {};
}
```

### `ignoreClassesThatImplementAnInterface`

Makes the rule ignore all class members that are defined within a class that `implements` a type.

It's important to note that this option does not only apply to members defined in the interface as that would require type information.

Example of a correct code when `ignoreClassesThatImplementAnInterface` is set to `true`:

```ts
class X implements Y {
method() {}
property = () => {};
}
```
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -19,6 +19,8 @@ export = {
'brace-style': 'off',
'@typescript-eslint/brace-style': 'error',
'@typescript-eslint/class-literal-property-style': 'error',
'class-methods-use-this': 'off',
'@typescript-eslint/class-methods-use-this': 'error',
'comma-dangle': 'off',
'@typescript-eslint/comma-dangle': 'error',
'comma-spacing': 'off',
Expand Down
265 changes: 265 additions & 0 deletions packages/eslint-plugin/src/rules/class-methods-use-this.ts
@@ -0,0 +1,265 @@
import type { TSESTree } from '@typescript-eslint/utils';
import { AST_NODE_TYPES } from '@typescript-eslint/utils';

import * as util from '../util';

type Options = [
{
exceptMethods?: string[];
enforceForClassFields?: boolean;
ignoreOverrideMethods?: boolean;
ignoreClassesThatImplementAnInterface?: boolean;
},
];
type MessageIds = 'missingThis';

export default util.createRule<Options, MessageIds>({
name: 'class-methods-use-this',
meta: {
type: 'suggestion',
docs: {
description: 'Enforce that class methods utilize `this`',
extendsBaseRule: true,
requiresTypeChecking: false,
},
fixable: 'code',
hasSuggestions: false,
schema: [
{
type: 'object',
properties: {
exceptMethods: {
type: 'array',
description:
'Allows specified method names to be ignored with this rule',
items: {
type: 'string',
},
},
enforceForClassFields: {
type: 'boolean',
description:
'Enforces that functions used as instance field initializers utilize `this`',
default: true,
},
ignoreOverrideMethods: {
type: 'boolean',
description: 'Ingore members marked with the `override` modifier',
},
ignoreClassesThatImplementAnInterface: {
type: 'boolean',
description:
'Ignore classes that specifically implement some interface',
},
},
additionalProperties: false,
},
],
messages: {
missingThis: "Expected 'this' to be used by class {{name}}.",
},
},
defaultOptions: [
{
enforceForClassFields: true,
exceptMethods: [],
ignoreClassesThatImplementAnInterface: false,
ignoreOverrideMethods: false,
},
],
create(
context,
[
{
enforceForClassFields,
exceptMethods: exceptMethodsRaw,
ignoreClassesThatImplementAnInterface,
ignoreOverrideMethods,
},
],
) {
const exceptMethods = new Set(exceptMethodsRaw);
type Stack =
| {
member: null;
class: null;
parent: Stack | undefined;
usesThis: boolean;
}
| {
member: TSESTree.MethodDefinition | TSESTree.PropertyDefinition;
class: TSESTree.ClassDeclaration | TSESTree.ClassExpression;
parent: Stack | undefined;
usesThis: boolean;
};
let stack: Stack | undefined;

const sourceCode = context.getSourceCode();

function pushContext(
member?: TSESTree.MethodDefinition | TSESTree.PropertyDefinition,
): void {
if (member?.parent.type === AST_NODE_TYPES.ClassBody) {
stack = {
member,
class: member.parent.parent as
| TSESTree.ClassDeclaration
| TSESTree.ClassExpression,
usesThis: false,
parent: stack,
};
} else {
stack = {
member: null,
class: null,
usesThis: false,
parent: stack,
};
}
}

function enterFunction(
node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
): void {
if (
node.parent.type === AST_NODE_TYPES.MethodDefinition ||
node.parent.type === AST_NODE_TYPES.PropertyDefinition
) {
pushContext(node.parent);
} else {
pushContext();
}
}

/**
* Pop `this` used flag from the stack.
*/
function popContext(): Stack | undefined {
const oldStack = stack;
stack = stack?.parent;
return oldStack;
}

/**
* Check if the node is an instance method not excluded by config
*/
function isIncludedInstanceMethod(
node: NonNullable<Stack['member']>,
): node is NonNullable<Stack['member']> {
if (
node.static ||
(node.type === AST_NODE_TYPES.MethodDefinition &&
node.kind === 'constructor') ||
(node.type === AST_NODE_TYPES.PropertyDefinition &&
!enforceForClassFields)
) {
return false;
}

if (node.computed || exceptMethods.size === 0) {
return true;
}

const hashIfNeeded =
node.key.type === AST_NODE_TYPES.PrivateIdentifier ? '#' : '';
const name =
node.key.type === AST_NODE_TYPES.Literal
? util.getStaticStringValue(node.key)
: node.key.name || '';

return !exceptMethods.has(hashIfNeeded + (name ?? ''));
}

/**
* Checks if we are leaving a function that is a method, and reports if 'this' has not been used.
* Static methods and the constructor are exempt.
* Then pops the context off the stack.
*/
function exitFunction(
node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
): void {
const stackContext = popContext();
if (
stackContext?.member == null ||
stackContext.class == null ||
stackContext.usesThis ||
(ignoreOverrideMethods && stackContext.member.override) ||
(ignoreClassesThatImplementAnInterface &&
stackContext.class.implements != null)
) {
return;
}

if (isIncludedInstanceMethod(stackContext.member)) {
context.report({
node,
loc: util.getFunctionHeadLoc(node, sourceCode),
messageId: 'missingThis',
data: {
name: util.getFunctionNameWithKind(node),
},
});
}
}

return {
// function declarations have their own `this` context
FunctionDeclaration(): void {
pushContext();
},
'FunctionDeclaration:exit'(): void {
popContext();
},

FunctionExpression(node): void {
enterFunction(node);
},
'FunctionExpression:exit'(node): void {
exitFunction(node);
},
...(enforceForClassFields
? {
'PropertyDefinition > ArrowFunctionExpression.value'(
node: TSESTree.ArrowFunctionExpression,
): void {
enterFunction(node);
},
'PropertyDefinition > ArrowFunctionExpression.value:exit'(
node: TSESTree.ArrowFunctionExpression,
): void {
exitFunction(node);
},
}
: {}),

/*
* Class field value are implicit functions.
*/
'PropertyDefinition > *.key:exit'(): void {
pushContext();
},
'PropertyDefinition:exit'(): void {
popContext();
},

/*
* Class static blocks are implicit functions. They aren't required to use `this`,
* but we have to push context so that it captures any use of `this` in the static block
* separately from enclosing contexts, because static blocks have their own `this` and it
* shouldn't count as used `this` in enclosing contexts.
*/
StaticBlock(): void {
pushContext();
},
'StaticBlock:exit'(): void {
popContext();
},

'ThisExpression, Super'(): void {
if (stack) {
stack.usesThis = true;
}
},
};
},
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -7,6 +7,7 @@ import banTypes from './ban-types';
import blockSpacing from './block-spacing';
import braceStyle from './brace-style';
import classLiteralPropertyStyle from './class-literal-property-style';
import classMethodsUseThis from './class-methods-use-this';
import commaDangle from './comma-dangle';
import commaSpacing from './comma-spacing';
import consistentGenericConstructors from './consistent-generic-constructors';
Expand Down Expand Up @@ -141,6 +142,7 @@ export default {
'block-spacing': blockSpacing,
'brace-style': braceStyle,
'class-literal-property-style': classLiteralPropertyStyle,
'class-methods-use-this': classMethodsUseThis,
'comma-dangle': commaDangle,
'comma-spacing': commaSpacing,
'consistent-generic-constructors': consistentGenericConstructors,
Expand Down

0 comments on commit 18ea3b1

Please sign in to comment.