From dd263d4bc25f745cbf4a0938cb80ddc6fb186509 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 3 Feb 2019 16:57:23 -0500 Subject: [PATCH 1/7] feat(eslint-plugin): added new rule no-unbound-method Adds the equivalent of TSLint's `no-unbound-method` rule. --- packages/eslint-plugin/README.md | 1 + packages/eslint-plugin/ROADMAP.md | 6 +- .../docs/rules/no-unbound-method.md | 92 ++++++ .../lib/rules/no-unbound-method.js | 131 ++++++++ .../tests/lib/rules/no-unbound-method.js | 287 ++++++++++++++++++ 5 files changed, 514 insertions(+), 3 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-unbound-method.md create mode 100644 packages/eslint-plugin/lib/rules/no-unbound-method.js create mode 100644 packages/eslint-plugin/tests/lib/rules/no-unbound-method.js diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 1ec6df21604..10fcffd18a6 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -121,6 +121,7 @@ Install [`eslint-config-prettier`](https://github.com/prettier/eslint-config-pre | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` (`no-this-assignment` from TSLint) | | | | [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments (`no-reference` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases (`interface-over-type-literal` from TSLint) | | | +| [`@typescript-eslint/no-unbound-method`](./docs/rules/no-unbound-method.md) | Enforces unbound methods are called with their expected scope. (`no-unbound-method` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression (`no-unnecessary-type-assertion` from TSLint) | | :wrench: | | [`@typescript-eslint/no-unused-vars`](./docs/rules/no-unused-vars.md) | Disallow unused variables (`no-unused-variable` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-use-before-define`](./docs/rules/no-use-before-define.md) | Disallow the use of variables before they are defined | :heavy_check_mark: | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index f26d0e80175..1ea1cdc2904 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -1,10 +1,10 @@ # Roadmap -✅ (28) = done
+✅ (29) = done
🌟 (79) = in ESLint core
🔌 (33) = in another plugin
🌓 (16) = implementations differ or ESLint version is missing functionality
-🛑 (70) = unimplemented +🛑 (69) = unimplemented ## TSLint rules @@ -75,7 +75,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/no-unbound-method`] | | [`no-unnecessary-class`] | ✅ | [`@typescript-eslint/no-extraneous-class`] | | [`no-unsafe-any`] | 🛑 | N/A | | [`no-unsafe-finally`] | 🌟 | [`no-unsafe-finally`][no-unsafe-finally] | diff --git a/packages/eslint-plugin/docs/rules/no-unbound-method.md b/packages/eslint-plugin/docs/rules/no-unbound-method.md new file mode 100644 index 00000000000..37dadeb0f6b --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unbound-method.md @@ -0,0 +1,92 @@ +# Enforces unbound methods are called with their expected scope (no-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: ["Thenable"] }`: + +```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/no-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/) diff --git a/packages/eslint-plugin/lib/rules/no-unbound-method.js b/packages/eslint-plugin/lib/rules/no-unbound-method.js new file mode 100644 index 00000000000..57c7bec65fd --- /dev/null +++ b/packages/eslint-plugin/lib/rules/no-unbound-method.js @@ -0,0 +1,131 @@ +/** + * @fileoverview Enforces unbound methods are called with their expected scope. + * @author Josh Goldberg + */ +'use strict'; + +const tsutils = require('tsutils'); +const ts = require('typescript'); + +const util = require('../util'); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const defaultOptions = [ + { + ignoreStatic: false + } +]; + +/** + * @type {import("eslint").Rule.RuleModule} + */ +module.exports = { + meta: { + docs: { + description: + 'Enforces unbound methods are called with their expected scope.', + extraDescription: [util.tslintRule('no-unbound-method')], + category: 'TypeScript', + url: util.metaDocsUrl('no-unbound-method'), + recommended: 'error' + }, + fixable: null, + messages: { + unbound: + 'Avoid referencing unbound methods which may cause unintentional scoping of `this`.' + }, + schema: [ + { + type: 'object', + properties: { + ignoreStatic: { + type: 'boolean' + } + }, + additionalProperties: false + } + ], + type: 'problem' + }, + + create(context) { + const { ignoreStatic } = util.applyDefault( + defaultOptions, + context.options + )[0]; + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + + return { + MemberExpression(node) { + if (isSafeUse(node)) { + return; + } + + const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); + const symbol = checker.getSymbolAtLocation(originalNode); + + if (isDangerousMethod(symbol, ignoreStatic)) { + context.report({ + messageId: 'unbound', + node + }); + } + } + }; + } +}; + +function isDangerousMethod(symbol, ignoreStatic) { + const declaration = + typeof symbol === 'undefined' ? undefined : symbol.valueDeclaration; + if (typeof declaration === 'undefined') { + return false; + } + + switch (declaration.kind) { + case ts.SyntaxKind.MethodDeclaration: + case ts.SyntaxKind.MethodSignature: + return !( + ignoreStatic && + tsutils.hasModifier(declaration.modifiers, ts.SyntaxKind.StaticKeyword) + ); + } + + return false; +} + +function isSafeUse(node) { + const { parent } = node; + + switch (parent.type) { + case 'IfStatement': + case 'ForStatement': + case 'MemberExpression': + case 'UpdateExpression': + case 'WhileStatement': + return true; + + case 'CallExpression': + return parent.callee === node; + + case 'ConditionalExpression': + return parent.test === node; + + case 'LogicalExpression': + return parent.operator !== '||'; + + case 'TaggedTemplateExpression': + return parent.tag === node; + + case 'TSNonNullExpression': + case 'TSAsExpression': + case 'TSTypeAssertionExpression': + return isSafeUse(parent); + } + + return false; +} diff --git a/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js b/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js new file mode 100644 index 00000000000..e70dc9f1c5e --- /dev/null +++ b/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js @@ -0,0 +1,287 @@ +/** + * @fileoverview Enforces unbound methods are called with their expected scope. + * @author Josh Goldberg + */ +'use strict'; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-unbound-method'), + RuleTester = require('eslint').RuleTester, + path = require('path'); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const rootDir = path.join(process.cwd(), 'tests/fixtures/'); +const parserOptions = { + tsconfigRootDir: rootDir, + project: './tsconfig.json' +}; +const ruleTester = new RuleTester({ + parserOptions, + parser: '@typescript-eslint/parser' +}); + +ruleTester.run('no-unbound-method', rule, { + valid: [ + ` +class ContainsMethods { + bound?: () => void; + unbound?(): void; + + static boundStatic?: () => void; + static unboundStatic?(): void; +} + +let instance = new ContainsMethods(); + +instance.bound(); +instance.unbound(); + +ContainsMethods.boundStatic(); +ContainsMethods.unboundStatic(); + +{ + const bound = instance.bound; + const boundStatic = ContainsMethods; +} +{ + const { bound } = instance; + const { boundStatic } = ContainsMethods; +} + +(instance.bound)(); +(instance.unbound)(); + +(ContainsMethods.boundStatic)(); +(ContainsMethods.unboundStatic)(); + +instance.bound\`\`; +instance.unbound\`\`; + +if (instance.bound) { } +if (instance.unbound) { } + +if (ContainsMethods.boundStatic) { } +if (ContainsMethods.unboundStatic) { } + +while (instance.bound) { } +while (instance.unbound) { } + +while (ContainsMethods.boundStatic) { } +while (ContainsMethods.unboundStatic) { } + +instance.bound as any; +ContainsMethods.boundStatic as any; + +instance.bound++; ++instance.bound; +++instance.bound; +instance.bound--; +-instance.bound; +--instance.bound; +instance.bound += 1; +instance.bound -= 1; +instance.bound *= 1; +instance.bound /= 1; + +instance.bound || 0; +instane.bound && 0; + +instance.bound ? 1 : 0; +instance.unbound ? 1 : 0; + +ContainsMethods.boundStatic++; ++ContainsMethods.boundStatic; +++ContainsMethods.boundStatic; +ContainsMethods.boundStatic--; +-ContainsMethods.boundStatic; +--ContainsMethods.boundStatic; +ContainsMethods.boundStatic += 1; +ContainsMethods.boundStatic -= 1; +ContainsMethods.boundStatic *= 1; +ContainsMethods.boundStatic /= 1; + +ContainsMethods.boundStatic || 0; +instane.boundStatic && 0; + +ContainsMethods.boundStatic ? 1 : 0; +ContainsMethods.unboundStatic ? 1 : 0; +` + ], + invalid: [ + { + code: ` +class ContainsMethods { + bound?: () => void; + unbound?(): void; + static boundStatic?: () => void; + static unboundStatic?(): void; +} + +const instance = new ContainsMethods(); + +{ + const unbound = instance.unbound; + const unboundStatic = ContainsMethods.unboundStatic; +} +{ + const { unbound } = instance.unbound; + const { unboundStatic } = ContainsMethods.unboundStatic; +} + +instance.unbound; +instance.unbound as any; + +ContainsMethods.unboundStatic; +ContainsMethods.unboundStatic as any; + +instance.unbound++; ++instance.unbound; +++instance.unbound; +instance.unbound--; +-instance.unbound; +--instance.unbound; +instance.unbound += 1; +instance.unbound -= 1; +instance.unbound *= 1; +instance.unbound /= 1; + +instance.unbound || 0; +instance.unbound && 0; + +ContainsMethods.unboundStatic++; ++ContainsMethods.unboundStatic; +++ContainsMethods.unboundStatic; +ContainsMethods.unboundStatic--; +-ContainsMethods.unboundStatic; +--ContainsMethods.unboundStatic; +ContainsMethods.unboundStatic += 1; +ContainsMethods.unboundStatic -= 1; +ContainsMethods.unboundStatic *= 1; +ContainsMethods.unboundStatic /= 1; + +ContainsMethods.unboundStatic || 0; +ContainsMethods.unboundStatic && 0; +`, + errors: [ + { + line: 12, + messageId: 'unbound' + }, + { + line: 13, + messageId: 'unbound' + }, + { + line: 16, + messageId: 'unbound' + }, + { + line: 17, + messageId: 'unbound' + }, + { + line: 20, + messageId: 'unbound' + }, + { + line: 21, + messageId: 'unbound' + }, + { + line: 23, + messageId: 'unbound' + }, + { + line: 24, + messageId: 'unbound' + }, + { + line: 27, + messageId: 'unbound' + }, + { + line: 30, + messageId: 'unbound' + }, + { + line: 32, + messageId: 'unbound' + }, + { + line: 33, + messageId: 'unbound' + }, + { + line: 34, + messageId: 'unbound' + }, + { + line: 35, + messageId: 'unbound' + }, + { + line: 37, + messageId: 'unbound' + }, + { + line: 41, + messageId: 'unbound' + }, + { + line: 44, + messageId: 'unbound' + }, + { + line: 46, + messageId: 'unbound' + }, + { + line: 47, + messageId: 'unbound' + }, + { + line: 48, + messageId: 'unbound' + }, + { + line: 49, + messageId: 'unbound' + }, + { + line: 51, + messageId: 'unbound' + } + ] + }, + { + code: ` +class ContainsMethods { + unbound?(): void; + + static unboundStatic?(): void; +} + +new ContainsMethods().unbound; + +ContainsMethods.unboundStatic; +`, + options: [ + { + ignoreStatic: true + } + ], + errors: [ + { + line: 8, + messageId: 'unbound' + } + ] + } + ] +}); From f1f915913cfa27882fc74b4bedb4136032eb5101 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 4 Feb 2019 20:58:38 -0500 Subject: [PATCH 2/7] Added missing reference to ROADMAP.md --- packages/eslint-plugin/ROADMAP.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 25d6424a2f6..a249eefeb8e 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -567,6 +567,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/no-unbound-method`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-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 From c624d05b1382cfedcecaa131169b75b5b5ff88dc Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 5 Feb 2019 21:50:59 -0500 Subject: [PATCH 3/7] ignoreStatic: ["Thenable"] typo --- packages/eslint-plugin/docs/rules/no-unbound-method.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-unbound-method.md b/packages/eslint-plugin/docs/rules/no-unbound-method.md index 37dadeb0f6b..b3e989248a8 100644 --- a/packages/eslint-plugin/docs/rules/no-unbound-method.md +++ b/packages/eslint-plugin/docs/rules/no-unbound-method.md @@ -55,7 +55,7 @@ The rule accepts an options object with the following property: ### `ignoreStatic` -Examples of **correct** code for this rule with `{ ignoreStatic: ["Thenable"] }`: +Examples of **correct** code for this rule with `{ ignoreStatic: true }`: ```ts class OtherClass { From dfd5935be5d4abd69ed3b3f073782e692d4e4c0c Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 17 Feb 2019 22:12:45 -0500 Subject: [PATCH 4/7] Finished rename to unbound-method --- packages/eslint-plugin/README.md | 2 +- packages/eslint-plugin/ROADMAP.md | 2 +- .../docs/rules/{no-unbound-method.md => unbound-method.md} | 4 ++-- packages/eslint-plugin/tests/lib/rules/no-unbound-method.js | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) rename packages/eslint-plugin/docs/rules/{no-unbound-method.md => unbound-method.md} (96%) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 90fa7dab466..5ccc71a74a0 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -140,7 +140,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` (`no-this-assignment` from TSLint) | | | | [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments (`no-reference` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases (`interface-over-type-literal` from TSLint) | | | -| [`@typescript-eslint/no-unbound-method`](./docs/rules/no-unbound-method.md) | Enforces unbound methods are called with their expected scope. (`no-unbound-method` from TSLint) | :heavy_check_mark: | | +| [`@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: | | | [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary (`no-unnecessary-qualifier` from TSLint) | | :wrench: | | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression (`no-unnecessary-type-assertion` from TSLint) | | :wrench: | | [`@typescript-eslint/no-unused-vars`](./docs/rules/no-unused-vars.md) | Disallow unused variables (`no-unused-variable` from TSLint) | :heavy_check_mark: | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index da4308dac53..ac643b966c4 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -586,7 +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/no-unbound-method`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unbound-method.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 diff --git a/packages/eslint-plugin/docs/rules/no-unbound-method.md b/packages/eslint-plugin/docs/rules/unbound-method.md similarity index 96% rename from packages/eslint-plugin/docs/rules/no-unbound-method.md rename to packages/eslint-plugin/docs/rules/unbound-method.md index b3e989248a8..d0268dadeec 100644 --- a/packages/eslint-plugin/docs/rules/no-unbound-method.md +++ b/packages/eslint-plugin/docs/rules/unbound-method.md @@ -1,4 +1,4 @@ -# Enforces unbound methods are called with their expected scope (no-unbound-method) +# Enforces unbound methods are called with their expected scope (unbound-method) Warns when a method is used outside of a method call. @@ -74,7 +74,7 @@ log(); ```json { - "@typescript-eslint/no-unbound-method": [ + "@typescript-eslint/unbound-method": [ "error", { "ignoreStatic": true diff --git a/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js b/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js index e70dc9f1c5e..3af684f871b 100644 --- a/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js +++ b/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js @@ -8,7 +8,7 @@ // Requirements //------------------------------------------------------------------------------ -const rule = require('../../../lib/rules/no-unbound-method'), +const rule = require('../../../lib/rules/unbound-method'), RuleTester = require('eslint').RuleTester, path = require('path'); @@ -26,7 +26,7 @@ const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser' }); -ruleTester.run('no-unbound-method', rule, { +ruleTester.run('unbound-method', rule, { valid: [ ` class ContainsMethods { From a3e5bc4e5f9fb8304457c0aae80401e50a745f50 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 23 Feb 2019 13:03:47 -0500 Subject: [PATCH 5/7] Removed unecessary docs; switched TS style casing --- .../eslint-plugin/src/rules/unbound-method.ts | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index fb08f466919..4bb4ef70024 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -1,10 +1,4 @@ -/** - * @fileoverview Enforces unbound methods are called with their expected scope. - * @author Josh Goldberg - */ -'use strict'; - -import { TSESTree } from '@typescript-eslint/typescript-estree'; +import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; import * as tsutils from 'tsutils'; import * as ts from 'typescript'; @@ -22,9 +16,6 @@ type Options = [Config]; type MessageIds = 'unbound'; -/** - * @type {import("eslint").Rule.RuleModule} - */ export default util.createRule({ name: 'unbound-method', meta: { @@ -99,32 +90,32 @@ function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean) { return false; } -function isSafeUse(node: TSESTree.BaseNode): boolean { +function isSafeUse(node: TSESTree.Node): boolean { const parent = node.parent!; switch (parent.type) { - case 'IfStatement': - case 'ForStatement': - case 'MemberExpression': - case 'UpdateExpression': - case 'WhileStatement': + 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 'CallExpression': + case AST_NODE_TYPES.CallExpression: return parent.callee === node; - case 'ConditionalExpression': + case AST_NODE_TYPES.ConditionalExpression: return parent.test === node; - case 'LogicalExpression': + case AST_NODE_TYPES.LogicalExpression: return parent.operator !== '||'; - case 'TaggedTemplateExpression': + case AST_NODE_TYPES.TaggedTemplateExpression: return parent.tag === node; - case 'TSNonNullExpression': - case 'TSAsExpression': - case 'TSTypeAssertion': + case AST_NODE_TYPES.TSNonNullExpression: + case AST_NODE_TYPES.TSAsExpression: + case AST_NODE_TYPES.TSTypeAssertion: return isSafeUse(parent); } From b6f16ffea22b8cb8592fe5a2d9ff13bb76bae1c3 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 23 Feb 2019 14:01:27 -0500 Subject: [PATCH 6/7] Formatted; converted test to .ts --- .../eslint-plugin/src/rules/unbound-method.ts | 26 ++--- .../unbound-method.test.ts} | 100 ++++++++---------- 2 files changed, 56 insertions(+), 70 deletions(-) rename packages/eslint-plugin/tests/{lib/rules/no-unbound-method.js => rules/unbound-method.test.ts} (71%) diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index 4bb4ef70024..9918a0e48fc 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -24,29 +24,29 @@ export default util.createRule({ description: 'Enforces unbound methods are called with their expected scope.', tslintName: 'no-unbound-method', - recommended: 'error' + recommended: 'error', }, messages: { unbound: - 'Avoid referencing unbound methods which may cause unintentional scoping of `this`.' + 'Avoid referencing unbound methods which may cause unintentional scoping of `this`.', }, schema: [ { type: 'object', properties: { ignoreStatic: { - type: 'boolean' - } + type: 'boolean', + }, }, - additionalProperties: false - } + additionalProperties: false, + }, ], - type: 'problem' + type: 'problem', }, defaultOptions: [ { - ignoreStatic: false - } + ignoreStatic: false, + }, ], create(context, [{ ignoreStatic }]) { const parserServices = util.getParserServices(context); @@ -64,12 +64,12 @@ export default util.createRule({ if (symbol && isDangerousMethod(symbol, ignoreStatic)) { context.report({ messageId: 'unbound', - node + node, }); } - } + }, }; - } + }, }); function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean) { @@ -82,7 +82,7 @@ function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean) { ignoreStatic && tsutils.hasModifier( valueDeclaration.modifiers, - ts.SyntaxKind.StaticKeyword + ts.SyntaxKind.StaticKeyword, ) ); } diff --git a/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js b/packages/eslint-plugin/tests/rules/unbound-method.test.ts similarity index 71% rename from packages/eslint-plugin/tests/lib/rules/no-unbound-method.js rename to packages/eslint-plugin/tests/rules/unbound-method.test.ts index 3af684f871b..8b4ed003fd4 100644 --- a/packages/eslint-plugin/tests/lib/rules/no-unbound-method.js +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -1,29 +1,15 @@ -/** - * @fileoverview Enforces unbound methods are called with their expected scope. - * @author Josh Goldberg - */ -'use strict'; - -//------------------------------------------------------------------------------ -// Requirements -//------------------------------------------------------------------------------ - -const rule = require('../../../lib/rules/unbound-method'), - RuleTester = require('eslint').RuleTester, - path = require('path'); - -//------------------------------------------------------------------------------ -// Tests -//------------------------------------------------------------------------------ - -const rootDir = path.join(process.cwd(), 'tests/fixtures/'); -const parserOptions = { - tsconfigRootDir: rootDir, - project: './tsconfig.json' -}; +import path from 'path'; +import rule from '../../src/rules/unbound-method'; +import { RuleTester } from '../RuleTester'; + +const rootPath = path.join(process.cwd(), 'tests/fixtures/'); + const ruleTester = new RuleTester({ - parserOptions, - parser: '@typescript-eslint/parser' + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, }); ruleTester.run('unbound-method', rule, { @@ -111,7 +97,7 @@ instane.boundStatic && 0; ContainsMethods.boundStatic ? 1 : 0; ContainsMethods.unboundStatic ? 1 : 0; -` +`, ], invalid: [ { @@ -171,93 +157,93 @@ ContainsMethods.unboundStatic && 0; errors: [ { line: 12, - messageId: 'unbound' + messageId: 'unbound', }, { line: 13, - messageId: 'unbound' + messageId: 'unbound', }, { line: 16, - messageId: 'unbound' + messageId: 'unbound', }, { line: 17, - messageId: 'unbound' + messageId: 'unbound', }, { line: 20, - messageId: 'unbound' + messageId: 'unbound', }, { line: 21, - messageId: 'unbound' + messageId: 'unbound', }, { line: 23, - messageId: 'unbound' + messageId: 'unbound', }, { line: 24, - messageId: 'unbound' + messageId: 'unbound', }, { line: 27, - messageId: 'unbound' + messageId: 'unbound', }, { line: 30, - messageId: 'unbound' + messageId: 'unbound', }, { line: 32, - messageId: 'unbound' + messageId: 'unbound', }, { line: 33, - messageId: 'unbound' + messageId: 'unbound', }, { line: 34, - messageId: 'unbound' + messageId: 'unbound', }, { line: 35, - messageId: 'unbound' + messageId: 'unbound', }, { line: 37, - messageId: 'unbound' + messageId: 'unbound', }, { line: 41, - messageId: 'unbound' + messageId: 'unbound', }, { line: 44, - messageId: 'unbound' + messageId: 'unbound', }, { line: 46, - messageId: 'unbound' + messageId: 'unbound', }, { line: 47, - messageId: 'unbound' + messageId: 'unbound', }, { line: 48, - messageId: 'unbound' + messageId: 'unbound', }, { line: 49, - messageId: 'unbound' + messageId: 'unbound', }, { line: 51, - messageId: 'unbound' - } - ] + messageId: 'unbound', + }, + ], }, { code: ` @@ -273,15 +259,15 @@ ContainsMethods.unboundStatic; `, options: [ { - ignoreStatic: true - } + ignoreStatic: true, + }, ], errors: [ { line: 8, - messageId: 'unbound' - } - ] - } - ] + messageId: 'unbound', + }, + ], + }, + ], }); From b2fc0b9ff99143a8b5d37f3782f849a511b0ef06 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 23 Feb 2019 14:03:05 -0500 Subject: [PATCH 7/7] Used computed property --- packages/eslint-plugin/src/rules/unbound-method.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index 9918a0e48fc..621e95ec7c5 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -53,7 +53,7 @@ export default util.createRule({ const checker = parserServices.program.getTypeChecker(); return { - MemberExpression(node) { + [AST_NODE_TYPES.MemberExpression](node: TSESTree.MemberExpression) { if (isSafeUse(node)) { return; }