From 8167440c69c775f210e5dc52bef99b2185f95118 Mon Sep 17 00:00:00 2001
From: Andrew MacLeay <amacleay@cloudhealthtech.com>
Date: Thu, 12 Jul 2018 10:35:10 -0400
Subject: [PATCH] no-invalid-this: false positive for method syntax

* Keep track of evaluation context: look to the closest parent function
  and decide if it is allowed to refer to `this`
* Fix a false negative in the tests from #3267
---
 src/rules/noInvalidThisRule.ts                | 61 +++++++++++++------
 .../no-invalid-this/enabled/test.ts.lint      | 40 ++++++++++++
 2 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/src/rules/noInvalidThisRule.ts b/src/rules/noInvalidThisRule.ts
index 4226ad3ecc3..585faaad4f9 100644
--- a/src/rules/noInvalidThisRule.ts
+++ b/src/rules/noInvalidThisRule.ts
@@ -59,39 +59,64 @@ export class Rule extends Lint.Rules.AbstractRule {
 
 function walk(ctx: Lint.WalkContext<boolean>): void {
     const { sourceFile, options: checkFuncInMethod } = ctx;
+
+    const enum parentType {
+        None,
+        Class,
+        ClassMethod,
+        BoundRegularFunction,
+        UnboundRegularFunction,
+    }
+
+    function thisIsAllowed(parent: parentType): boolean {
+        return [parentType.ClassMethod, parentType.BoundRegularFunction]
+            .some((t) => parent === t);
+    }
+    let currentParent: parentType = parentType.None;
     let inClass = false;
-    let inFunctionInClass = false;
 
     ts.forEachChild(sourceFile, function cb(node: ts.Node) {
+        const originalParent = currentParent;
+        const originalInClass = inClass;
         switch (node.kind) {
             case ts.SyntaxKind.ClassDeclaration:
             case ts.SyntaxKind.ClassExpression:
-                if (!inClass) {
-                    inClass = true;
-                    ts.forEachChild(node, cb);
-                    inClass = false;
-                    return;
-                }
-                break;
+                inClass = true;
+                currentParent = parentType.Class;
+                ts.forEachChild(node, cb);
+                currentParent = originalParent;
+                inClass = originalInClass;
+                return;
 
+            case ts.SyntaxKind.MethodDeclaration:
+            case ts.SyntaxKind.GetAccessor:
+            case ts.SyntaxKind.SetAccessor:
+            case ts.SyntaxKind.Constructor:
+            case ts.SyntaxKind.PropertyDeclaration:
             case ts.SyntaxKind.FunctionDeclaration:
             case ts.SyntaxKind.FunctionExpression:
-                if ((node as ts.FunctionLikeDeclaration).parameters.some(isThisParameter)) {
+                if (currentParent === parentType.Class) {
+                    currentParent = parentType.ClassMethod;
+                    ts.forEachChild(node, cb);
+                    currentParent = originalParent;
                     return;
-                }
-                if (inClass) {
-                    inFunctionInClass = true;
+                } else {
+                    currentParent
+                        = (node as ts.FunctionLikeDeclaration).parameters.some(isThisParameter)
+                        ? parentType.BoundRegularFunction
+                        : parentType.UnboundRegularFunction;
                     ts.forEachChild(node, cb);
-                    inFunctionInClass = false;
+                    currentParent = originalParent;
                     return;
                 }
-                break;
 
             case ts.SyntaxKind.ThisKeyword:
-                if (!inClass) {
-                    ctx.addFailureAtNode(node, Rule.FAILURE_STRING_OUTSIDE);
-                } else if (checkFuncInMethod && inFunctionInClass) {
-                    ctx.addFailureAtNode(node, Rule.FAILURE_STRING_INSIDE);
+                if (!thisIsAllowed(currentParent)) {
+                    if (!inClass) {
+                        ctx.addFailureAtNode(node, Rule.FAILURE_STRING_OUTSIDE);
+                    } else if (checkFuncInMethod) {
+                        ctx.addFailureAtNode(node, Rule.FAILURE_STRING_INSIDE);
+                    }
                 }
                 return;
         }
diff --git a/test/rules/no-invalid-this/enabled/test.ts.lint b/test/rules/no-invalid-this/enabled/test.ts.lint
index 166d8cc074e..47f50f57aa2 100644
--- a/test/rules/no-invalid-this/enabled/test.ts.lint
+++ b/test/rules/no-invalid-this/enabled/test.ts.lint
@@ -1,3 +1,23 @@
+function setSomeProperty(this: { someProperty: number }) {
+    this.someProperty = 7;
+}
+
+function bindProperty(this: { someProperty: number }, fn: Function) {
+    return fn.bind(this);
+}
+
+const objectLiteralStyle = {
+    bindProperty: function(this: { someProperty: number }, fn: Function) {
+        return fn.bind(this);
+    },
+};
+
+const objectMethodStyle = {
+    bindProperty(this: { someProperty: number }, fn: Function) {
+        return fn.bind(this);
+    },
+};
+
 export function f<T>(this: Observable<T>): Observable<T> {
     const nestedFunction = function(this) => {
         console.log(this)
@@ -10,6 +30,7 @@ class ContextualThisClass {
         let nestedFunction: (this: number[]) => number[] = function(this) {
             [3,4].forEach(function(nr){
                 console.log(this);
+                            ~~~~           [the "this" keyword is disallowed in function bodies inside class methods, use arrow functions instead]
             });
             return this.map(i=>i);
         };
@@ -67,6 +88,25 @@ class AClass {
         [3,4].forEach(badFunction);
         let badFunction = nr => console.log(this.x === nr);
     }
+
+    public fMethod() {
+        const objectLiteralStyle = {
+            bindProperty: function(this: { someProperty: number }, fn: Function) {
+                return fn.bind(this);
+            },
+        };
+
+        const objectMethodStyle = {
+            bindProperty(this: { someProperty: number }, fn: Function) {
+                return fn.bind(this);
+            },
+        };
+
+        return {
+            objectLiteralStyle,
+            objectMethodStyle,
+        };
+    }
 }
 
 const AClassExpression = class {