feat(eslint-plugin): add no-implicit-this-methods rule

packages/eslint-plugin/README.md

@@ -169,6 +169,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
 | [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md)                           | Forbids the use of classes as namespaces                                                                                                            |                    |          |                   |
 | [`@typescript-eslint/no-floating-promises`](./docs/rules/no-floating-promises.md)                         | Requires Promise-like values to be handled appropriately.                                                                                           |                    |          | :thought_balloon: |
 | [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md)                                   | Disallow iterating over an array with a for-in loop                                                                                                 | :heavy_check_mark: |          | :thought_balloon: |
+| [`@typescript-eslint/no-implicit-this-methods`](./docs/rules/no-implicit-this-methods.md)                 | Requires explicit this type annotations for class and object methods                                                                                |                    |          |                   |
 | [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md)                           | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean                                        | :heavy_check_mark: | :wrench: |                   |
 | [`@typescript-eslint/no-magic-numbers`](./docs/rules/no-magic-numbers.md)                                 | Disallows magic numbers                                                                                                                             |                    |          |                   |
 | [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md)                                     | Enforce valid definition of `new` and `constructor`                                                                                                 | :heavy_check_mark: |          |                   |

packages/eslint-plugin/docs/rules/no-implicit-this-methods.md

@@ -0,0 +1,72 @@
+# No implicit `this` type for methods
+
+This rule requires that all class and object methods that use the `this` keyword must explicitly specify the `this` type, so that the typescript compiler will ensure that the function is always called with the right `this`. For example:
+
+```ts
+class Example {
+  value = 'val';
+  implicit() {
+    return this.value.toUpperCase();
+  }
+  explicit(this: this) {
+    return this.value.toUpperCase();
+  }
+}
+const implicit = new Example().implicit;
+implicit(); // Runtime error, but not a compiler error
+
+const explicit = new Example().explicit;
+explicit(); // Compile error
+```
+
+The compiler has a built-in `--noImplicitThis` option, but it allows `this` to be implicit in classes and object methods.
+
+Examples of **incorrect** code for this rule:
+
+```ts
+class Test {
+  val = 'val';
+  method() {
+    console.log(this.val);
+  }
+}
+
+const obj = {
+  val: 'val',
+  method() {
+    console.log(this.val);
+  },
+};
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+class Test {
+  val = 'val';
+  // Explicit this type
+  method1(this: this) {
+    console.log(this.val);
+  }
+  // Doesn't use this
+  method2() {
+    console.log('val');
+  }
+}
+
+const obj = {
+  val: 'val',
+  // Explicit this type
+  method1(this: { val: string }) {
+    console.log(this.val);
+  },
+  // Doesn't use this
+  method2() {
+    console.log('val');
+  },
+};
+```
+
+## When Not To Use It
+
+If you don't like the boilerplate of explicit `this` annotations, and/or don't have issue with disconnecting methods from their `this` context.

packages/eslint-plugin/src/configs/all.json

@@ -37,6 +37,7 @@
     "@typescript-eslint/no-extraneous-class": "error",
     "@typescript-eslint/no-floating-promises": "error",
     "@typescript-eslint/no-for-in-array": "error",
+    "@typescript-eslint/no-implicit-this-methods": "error",
     "@typescript-eslint/no-inferrable-types": "error",
     "no-magic-numbers": "off",
     "@typescript-eslint/no-magic-numbers": "error",

packages/eslint-plugin/src/rules/index.ts

@@ -28,6 +28,7 @@ import noExtraneousClass from './no-extraneous-class';
 import noFloatingPromises from './no-floating-promises';
 import noForInArray from './no-for-in-array';
 import noInferrableTypes from './no-inferrable-types';
+import noImplicitThisMethods from './no-implicit-this-methods';
 import noMagicNumbers from './no-magic-numbers';
 import noMisusedNew from './no-misused-new';
 import noMisusedPromises from './no-misused-promises';
@@ -104,6 +105,7 @@ export default {
   'no-floating-promises': noFloatingPromises,
   'no-for-in-array': noForInArray,
   'no-inferrable-types': noInferrableTypes,
+  'no-implicit-this-methods': noImplicitThisMethods,
   'no-magic-numbers': noMagicNumbers,
   'no-misused-new': noMisusedNew,
   'no-misused-promises': noMisusedPromises,

packages/eslint-plugin/src/rules/no-implicit-this-methods.ts

@@ -0,0 +1,76 @@
+import { createRule } from '../util';
+import {
+  Property,
+  FunctionExpression,
+  Node,
+  MethodDefinition,
+} from '../../../typescript-estree/dist/ts-estree/ts-estree';
+import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils';
+
+export type MessageId = 'implicitThis';
+
+export default createRule({
+  name: 'no-implicit-this-methods',
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description:
+        'Requires explicit this type annotations for class and object methods',
+      category: 'Best Practices',
+      recommended: false,
+      requiresTypeChecking: false,
+    },
+    schema: [],
+    messages: {
+      implicitThis: '',
+    },
+  },
+  defaultOptions: [],
+  create(context) {
+    const methodStack = [{ usesThis: false }];
+
+    const isConstructor = (node: Property | MethodDefinition): boolean =>
+      node.key.type === 'Identifier' && node.key.name === 'constructor';
+
+    const isFunctionExpression = (node: Node): node is FunctionExpression =>
+      node.type === AST_NODE_TYPES.FunctionExpression;
+
+    const hasExplicitThisParam = ({
+      params: [maybeThisParam],
+    }: FunctionExpression): boolean =>
+      maybeThisParam &&
+      maybeThisParam.type === 'Identifier' &&
+      maybeThisParam.name === 'this';
+
+    const checkMethodForImplicitThis = (
+      node: Property | MethodDefinition,
+    ): void => {
+      if (isFunctionExpression(node.value) && !isConstructor(node)) {
+        if (
+          methodStack.shift()!.usesThis &&
+          !hasExplicitThisParam(node.value)
+        ) {
+          context.report({ node: node.value, messageId: 'implicitThis' });
+        }
+      }
+    };
+
+    return {
+      ThisExpression: (): void => {
+        methodStack[0].usesThis = true;
+      },
+      Property: (node): void => {
+        if (isFunctionExpression(node.value) && !isConstructor(node)) {
+          methodStack.unshift({ usesThis: false });
+        }
+      },
+      'Property:exit': checkMethodForImplicitThis as never, // the types don't support ":exit" methods
+      MethodDefinition: (node): void => {
+        if (isFunctionExpression(node.value) && !isConstructor(node)) {
+          methodStack.unshift({ usesThis: false });
+        }
+      },
+      'MethodDefinition:exit': checkMethodForImplicitThis as never, // the types don't support ":exit" methods
+    };
+  },
+});

packages/eslint-plugin/tests/rules/no-implicit-this-methods.test.ts

@@ -0,0 +1,80 @@
+import path from 'path';
+import rule, { MessageId } from '../../src/rules/no-implicit-this-methods';
+import { RuleTester } from '../RuleTester';
+import { TestCaseError } from '@typescript-eslint/experimental-utils/dist/ts-eslint';
+
+const rootPath = path.join(process.cwd(), 'tests/fixtures/');
+
+const ruleTester = new RuleTester({
+  parser: '@typescript-eslint/parser',
+  parserOptions: {
+    tsconfigRootDir: rootPath,
+    project: './tsconfig.json',
+  },
+});
+
+const ruleError = (
+  line: number,
+  column: number,
+  messageId: MessageId,
+): TestCaseError<MessageId> => ({ messageId, line, column });
+
+ruleTester.run('no-implicit-this-methods', rule, {
+  valid: [
+    // Object properties without this
+    `
+const obj = {
+  foo() { return 'foo'; },
+  bar: function() { return 'bar'; },
+}`,
+    // Class methods without this
+    `
+class Test {
+  foo() { return 'foo'; }
+  bar = function() { return 'bar'; }
+}`,
+    // Object with explicit this annotations
+    `
+const obj = {
+  v: 'val',
+  foo(this: {v: string}) { return this.v },
+  bar: function(this: {v: string}) { return this.v; },
+}`,
+    // Class with explicit this annotations
+    `
+class Test {
+  v = 'val';
+  foo(this: Test) { return this.v; }
+}`,
+    // Ignores constructors
+    `
+class Test {
+  value: string;
+  constructor() { this.value = "val"; }
+}`,
+  ],
+  invalid: [
+    // Object needing explicit this annotation
+    {
+      code: `
+const obj = {
+  v: 'val',
+  foo() { return this.v },
+  bar: function() { return this.v; },
+}`,
+      errors: [
+        ruleError(4, 6, 'implicitThis'),
+        ruleError(5, 8, 'implicitThis'),
+      ],
+    },
+    // Class needing explicit this annotation
+    {
+      code: `
+class Test {
+  v = "val";
+  foo() { return this.v }
+}`,
+      errors: [ruleError(4, 6, 'implicitThis')],
+    },
+  ],
+});