-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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): added new rule unbound-method (#204)
- Loading branch information
1 parent
ab3c1a1
commit 6718906
Showing
5 changed files
with
491 additions
and
1 deletion.
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
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,92 @@ | ||
# Enforces unbound methods are called with their expected scope (unbound-method) | ||
|
||
Warns when a method is used outside of a method call. | ||
|
||
Class functions don't preserve the class scope when passed as standalone variables. | ||
|
||
## Rule Details | ||
|
||
Examples of **incorrect** code for this rule | ||
|
||
```ts | ||
class MyClass { | ||
public log(): void { | ||
console.log(this); | ||
} | ||
} | ||
|
||
const instance = new MyClass(); | ||
|
||
// This logs the global scope (`window`/`global`), not the class instance | ||
const myLog = instance.log; | ||
myLog(); | ||
|
||
// This log might later be called with an incorrect scope | ||
const { log } = instance; | ||
``` | ||
|
||
Examples of **correct** code for this rule | ||
|
||
```ts | ||
class MyClass { | ||
public logUnbound(): void { | ||
console.log(this); | ||
} | ||
|
||
public logBound = () => console.log(this); | ||
} | ||
|
||
const instance = new MyClass(); | ||
|
||
// logBound will always be bound with the correct scope | ||
const { logBound } = instance; | ||
logBound(); | ||
|
||
// .bind and lambdas will also add a correct scope | ||
const dotBindLog = instance.log.bind(instance); | ||
const innerLog = () => instance.log(); | ||
``` | ||
|
||
## Options | ||
|
||
The rule accepts an options object with the following property: | ||
|
||
- `ignoreStatic` to not check whether `static` methods are correctly bound | ||
|
||
### `ignoreStatic` | ||
|
||
Examples of **correct** code for this rule with `{ ignoreStatic: true }`: | ||
|
||
```ts | ||
class OtherClass { | ||
static log() { | ||
console.log(OtherClass); | ||
} | ||
} | ||
|
||
// With `ignoreStatic`, statics are assumed to not rely on a particular scope | ||
const { log } = OtherClass; | ||
|
||
log(); | ||
``` | ||
|
||
### Example | ||
|
||
```json | ||
{ | ||
"@typescript-eslint/unbound-method": [ | ||
"error", | ||
{ | ||
"ignoreStatic": true | ||
} | ||
] | ||
} | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
If your code intentionally waits to bind methods after use, such as by passing a `scope: this` along with the method, you can disable this rule. | ||
|
||
## Related To | ||
|
||
- TSLint: [no-unbound-method](https://palantir.github.io/tslint/rules/no-unbound-method/) |
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,123 @@ | ||
import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; | ||
import * as tsutils from 'tsutils'; | ||
import * as ts from 'typescript'; | ||
|
||
import * as util from '../util'; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Rule Definition | ||
//------------------------------------------------------------------------------ | ||
|
||
interface Config { | ||
ignoreStatic: boolean; | ||
} | ||
|
||
type Options = [Config]; | ||
|
||
type MessageIds = 'unbound'; | ||
|
||
export default util.createRule<Options, MessageIds>({ | ||
name: 'unbound-method', | ||
meta: { | ||
docs: { | ||
category: 'Best Practices', | ||
description: | ||
'Enforces unbound methods are called with their expected scope.', | ||
tslintName: 'no-unbound-method', | ||
recommended: 'error', | ||
}, | ||
messages: { | ||
unbound: | ||
'Avoid referencing unbound methods which may cause unintentional scoping of `this`.', | ||
}, | ||
schema: [ | ||
{ | ||
type: 'object', | ||
properties: { | ||
ignoreStatic: { | ||
type: 'boolean', | ||
}, | ||
}, | ||
additionalProperties: false, | ||
}, | ||
], | ||
type: 'problem', | ||
}, | ||
defaultOptions: [ | ||
{ | ||
ignoreStatic: false, | ||
}, | ||
], | ||
create(context, [{ ignoreStatic }]) { | ||
const parserServices = util.getParserServices(context); | ||
const checker = parserServices.program.getTypeChecker(); | ||
|
||
return { | ||
[AST_NODE_TYPES.MemberExpression](node: TSESTree.MemberExpression) { | ||
if (isSafeUse(node)) { | ||
return; | ||
} | ||
|
||
const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); | ||
const symbol = checker.getSymbolAtLocation(originalNode); | ||
|
||
if (symbol && isDangerousMethod(symbol, ignoreStatic)) { | ||
context.report({ | ||
messageId: 'unbound', | ||
node, | ||
}); | ||
} | ||
}, | ||
}; | ||
}, | ||
}); | ||
|
||
function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean) { | ||
const { valueDeclaration } = symbol; | ||
|
||
switch (valueDeclaration.kind) { | ||
case ts.SyntaxKind.MethodDeclaration: | ||
case ts.SyntaxKind.MethodSignature: | ||
return !( | ||
ignoreStatic && | ||
tsutils.hasModifier( | ||
valueDeclaration.modifiers, | ||
ts.SyntaxKind.StaticKeyword, | ||
) | ||
); | ||
} | ||
|
||
return false; | ||
} | ||
|
||
function isSafeUse(node: TSESTree.Node): boolean { | ||
const parent = node.parent!; | ||
|
||
switch (parent.type) { | ||
case AST_NODE_TYPES.IfStatement: | ||
case AST_NODE_TYPES.ForStatement: | ||
case AST_NODE_TYPES.MemberExpression: | ||
case AST_NODE_TYPES.UpdateExpression: | ||
case AST_NODE_TYPES.WhileStatement: | ||
return true; | ||
|
||
case AST_NODE_TYPES.CallExpression: | ||
return parent.callee === node; | ||
|
||
case AST_NODE_TYPES.ConditionalExpression: | ||
return parent.test === node; | ||
|
||
case AST_NODE_TYPES.LogicalExpression: | ||
return parent.operator !== '||'; | ||
|
||
case AST_NODE_TYPES.TaggedTemplateExpression: | ||
return parent.tag === node; | ||
|
||
case AST_NODE_TYPES.TSNonNullExpression: | ||
case AST_NODE_TYPES.TSAsExpression: | ||
case AST_NODE_TYPES.TSTypeAssertion: | ||
return isSafeUse(parent); | ||
} | ||
|
||
return false; | ||
} |
Oops, something went wrong.