Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(eslint-plugin): [class-methods-use-this] add extension rule (#6457)
- Loading branch information
1 parent
f813147
commit 18ea3b1
Showing
12 changed files
with
1,053 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
packages/eslint-plugin/docs/rules/class-methods-use-this.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 = () => {}; | ||
} | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
265 changes: 265 additions & 0 deletions
265
packages/eslint-plugin/src/rules/class-methods-use-this.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} | ||
}, | ||
}; | ||
}, | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.