Skip to content

Commit

Permalink
feat(eslint-plugin): added new rule unbound-method (#204)
Browse files Browse the repository at this point in the history
  • Loading branch information
Josh Goldberg authored and JamesHenry committed Apr 3, 2019
1 parent ab3c1a1 commit 6718906
Show file tree
Hide file tree
Showing 5 changed files with 491 additions and 1 deletion.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -152,6 +152,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
| [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async. (`promise-function-async` from TSLint) | | | :thought_balloon: |
| [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string. (`restrict-plus-operands` from TSLint) | | | :thought_balloon: |
| [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations (`typedef-whitespace` from TSLint) | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/unbound-method`](./docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope. (`no-unbound-method` from TSLint) | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/unified-signatures`](./docs/rules/unified-signatures.md) | Warns for any two overloads that could be unified into one. (`unified-signatures` from TSLint) | | | |

<!-- end rule list -->
3 changes: 2 additions & 1 deletion packages/eslint-plugin/ROADMAP.md
Expand Up @@ -77,7 +77,7 @@
| [`no-submodule-imports`] | 🌓 | [`import/no-internal-modules`] (slightly different) |
| [`no-switch-case-fall-through`] | 🌟 | [`no-fallthrough`][no-fallthrough] |
| [`no-this-assignment`] || [`@typescript-eslint/no-this-alias`] |
| [`no-unbound-method`] | 🛑 | N/A |
| [`no-unbound-method`] | | [`@typescript-eslint/unbound-method`] |
| [`no-unnecessary-class`] || [`@typescript-eslint/no-extraneous-class`] |
| [`no-unsafe-any`] | 🛑 | N/A |
| [`no-unsafe-finally`] | 🌟 | [`no-unsafe-finally`][no-unsafe-finally] |
Expand Down Expand Up @@ -586,6 +586,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@typescript-eslint/no-namespace`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-namespace.md
[`@typescript-eslint/no-non-null-assertion`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-non-null-assertion.md
[`@typescript-eslint/no-triple-slash-reference`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-triple-slash-reference.md
[`@typescript-eslint/unbound-method`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unbound-method.md
[`@typescript-eslint/no-unnecessary-type-assertion`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unnecessary-type-assertion.md
[`@typescript-eslint/no-var-requires`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-var-requires.md
[`@typescript-eslint/type-annotation-spacing`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/type-annotation-spacing.md
Expand Down
92 changes: 92 additions & 0 deletions packages/eslint-plugin/docs/rules/unbound-method.md
@@ -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/)
123 changes: 123 additions & 0 deletions packages/eslint-plugin/src/rules/unbound-method.ts
@@ -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;
}

0 comments on commit 6718906

Please sign in to comment.