Fix: no-invalid-this false positive (fixes #6824) (#6827)

docs/rules/no-invalid-this.md

@@ -11,12 +11,13 @@ Basically this rule checks whether or not a function which are containing `this`
 This rule judges from following conditions whether or not the function is a constructor:
 
 * The name of the function starts with uppercase.
+* The function is assigned to a variable which starts with an uppercase letter.
 * The function is a constructor of ES2015 Classes.
 
 This rule judges from following conditions whether or not the function is a method:
 
 * The function is on an object literal.
-* The function assigns to a property.
+* The function is assigned to a property.
 * The function is a method/getter/setter of ES2015 Classes. (excepts static methods)
 
 And this rule allows `this` keywords in functions below:

lib/ast-utils.js

@@ -49,16 +49,23 @@ function isModifyingReference(reference, index, references) {
     );
 }
 
+/**
+ * Checks whether the given string starts with uppercase or not.
+ *
+ * @param {string} s - The string to check.
+ * @returns {boolean} `true` if the string starts with uppercase.
+ */
+function startsWithUpperCase(s) {
+    return s[0] !== s[0].toLocaleLowerCase();
+}
+
 /**
  * Checks whether or not a node is a constructor.
  * @param {ASTNode} node - A function node to check.
  * @returns {boolean} Wehether or not a node is a constructor.
  */
 function isES5Constructor(node) {
-    return (
-        node.id &&
-        node.id.name[0] !== node.id.name[0].toLocaleLowerCase()
-    );
+    return (node.id && startsWithUpperCase(node.id.name));
 }
 
 /**
@@ -344,9 +351,9 @@ module.exports = {
      * If the location is below, this judges `this` is valid.
      *
      * - The location is not on an object literal.
-     * - The location does not assign to a property.
+     * - The location is not assigned to a variable which starts with an uppercase letter.
      * - The location is not on an ES2015 class.
-     * - The location does not call its `bind`/`call`/`apply` method directly.
+     * - Its `bind`/`call`/`apply` method is not called directly.
      * - The function is not a callback of array methods (such as `.forEach()`) if `thisArg` is given.
      *
      * @param {ASTNode} node - A function node to check.
@@ -357,6 +364,7 @@ module.exports = {
         if (isES5Constructor(node) || hasJSDocThisTag(node, sourceCode)) {
             return false;
         }
+        const isAnonymous = node.id === null;
 
         while (node) {
             const parent = node.parent;
@@ -391,25 +399,44 @@ module.exports = {
                 // e.g.
                 //   var obj = { foo() { ... } };
                 //   var obj = { foo: function() { ... } };
-                case "Property":
-                    return false;
-
-                // e.g.
-                //   obj.foo = foo() { ... };
-                case "AssignmentExpression":
-                    return (
-                        parent.right !== node ||
-                        parent.left.type !== "MemberExpression"
-                    );
-
-                // e.g.
                 //   class A { constructor() { ... } }
                 //   class A { foo() { ... } }
                 //   class A { get foo() { ... } }
                 //   class A { set foo() { ... } }
                 //   class A { static foo() { ... } }
+                case "Property":
                 case "MethodDefinition":
-                    return false;
+                    return parent.value !== node;
+
+                // e.g.
+                //   obj.foo = function foo() { ... };
+                //   Foo = function() { ... };
+                //   [obj.foo = function foo() { ... }] = a;
+                //   [Foo = function() { ... }] = a;
+                case "AssignmentExpression":
+                case "AssignmentPattern":
+                    if (parent.right === node) {
+                        if (parent.left.type === "MemberExpression") {
+                            return false;
+                        }
+                        if (isAnonymous &&
+                            parent.left.type === "Identifier" &&
+                            startsWithUpperCase(parent.left.name)
+                        ) {
+                            return false;
+                        }
+                    }
+                    return true;
+
+                // e.g.
+                //   var Foo = function() { ... };
+                case "VariableDeclarator":
+                    return !(
+                        isAnonymous &&
+                        parent.init === node &&
+                        parent.id.type === "Identifier" &&
+                        startsWithUpperCase(parent.id.name)
+                    );
 
                 // e.g.
                 //   var foo = function foo() { ... }.bind(obj);

tests/lib/rules/no-invalid-this.js

@@ -529,7 +529,61 @@ const patterns = [
         parserOptions: { ecmaVersion: 6 },
         valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
         invalid: []
-    }
+    },
+
+    // https://github.com/eslint/eslint/issues/6824
+    {
+        code: "var Ctor = function() { console.log(this); z(x => console.log(x, this)); }",
+        parserOptions: { ecmaVersion: 6 },
+        valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
+        invalid: []
+    },
+    {
+        code: "var func = function() { console.log(this); z(x => console.log(x, this)); }",
+        parserOptions: { ecmaVersion: 6 },
+        errors: errors,
+        valid: [NORMAL],
+        invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
+    },
+    {
+        code: "Ctor = function() { console.log(this); z(x => console.log(x, this)); }",
+        parserOptions: { ecmaVersion: 6 },
+        valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
+        invalid: []
+    },
+    {
+        code: "func = function() { console.log(this); z(x => console.log(x, this)); }",
+        parserOptions: { ecmaVersion: 6 },
+        errors: errors,
+        valid: [NORMAL],
+        invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
+    },
+    {
+        code: "function foo(Ctor = function() { console.log(this); z(x => console.log(x, this)); }) {}",
+        parserOptions: { ecmaVersion: 6 },
+        valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
+        invalid: []
+    },
+    {
+        code: "function foo(func = function() { console.log(this); z(x => console.log(x, this)); }) {}",
+        parserOptions: { ecmaVersion: 6 },
+        errors: errors,
+        valid: [NORMAL],
+        invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
+    },
+    {
+        code: "[obj.method = function() { console.log(this); z(x => console.log(x, this)); }] = a",
+        parserOptions: { ecmaVersion: 6 },
+        valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
+        invalid: []
+    },
+    {
+        code: "[func = function() { console.log(this); z(x => console.log(x, this)); }] = a",
+        parserOptions: { ecmaVersion: 6 },
+        errors: errors,
+        valid: [NORMAL],
+        invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
+    },
 ];
 
 const ruleTester = new RuleTester();