From aa55684b679c8a7ab16d4cc4946bd83fe2e7ff81 Mon Sep 17 00:00:00 2001
From: tbreisacher <tbreisacher@google.com>
Date: Wed, 7 Jun 2017 14:42:00 -0700
Subject: [PATCH] Don't bother creating a 'this' node for empty functions.

Unblocks some upcoming changes to ES6 class transpilation.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158320031
---
 .../google/javascript/jscomp/SymbolTable.java | 42 ++++++++-----------
 .../javascript/jscomp/SymbolTableTest.java    |  6 +--
 2 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/src/com/google/javascript/jscomp/SymbolTable.java b/src/com/google/javascript/jscomp/SymbolTable.java
index 5f1e6a347c3..a3a096fa80d 100644
--- a/src/com/google/javascript/jscomp/SymbolTable.java
+++ b/src/com/google/javascript/jscomp/SymbolTable.java
@@ -1504,32 +1504,26 @@ public void enterScope(NodeTraversal t) {
         }
       } else {
         // Otherwise, declare a "this" property when possible.
-        SymbolScope scope = scopes.get(t.getScopeRoot());
-        Preconditions.checkNotNull(scope, "No scope found for node: %s", t.getScopeRoot());
-        Symbol scopeSymbol = getSymbolForScope(scope);
-        if (scopeSymbol != null) {
-          SymbolScope propScope = scopeSymbol.getPropertyScope();
-          if (propScope != null) {
-            // If a function is assigned multiple times, we only want
-            // one addressable "this" symbol.
-            symbol = propScope.getOwnSlot("this");
-            if (symbol == null) {
-              JSType rootType = t.getScopeRoot().getJSType();
-              FunctionType fnType = rootType == null
-                  ? null : rootType.toMaybeFunctionType();
-              JSType type = fnType == null
-                  ? null : fnType.getTypeOfThis();
-              symbol = addSymbol(
-                  "this",
-                  type,
-                  false /* declared */,
-                  scope,
-                  t.getScopeRoot());
+        Node scopeRoot = t.getScopeRoot();
+        SymbolScope scope = scopes.get(scopeRoot);
+        if (NodeUtil.getFunctionBody(scopeRoot).hasChildren()) {
+          Symbol scopeSymbol = getSymbolForScope(scope);
+          if (scopeSymbol != null) {
+            SymbolScope propScope = scopeSymbol.getPropertyScope();
+            if (propScope != null) {
+              // If a function is assigned multiple times, we only want
+              // one addressable "this" symbol.
+              symbol = propScope.getOwnSlot("this");
+              if (symbol == null) {
+                JSType rootType = t.getScopeRoot().getJSType();
+                FunctionType fnType = rootType == null ? null : rootType.toMaybeFunctionType();
+                JSType type = fnType == null ? null : fnType.getTypeOfThis();
+                symbol = addSymbol("this", type, false /* declared */, scope, t.getScopeRoot());
+              }
             }
-
-            // TODO(nicksantos): It's non-obvious where the declaration of
-            // the 'this' symbol should be. Figure this out later.
           }
+        } else {
+          logger.warning("Skipping empty function: " + scopeRoot);
         }
       }
 
diff --git a/test/com/google/javascript/jscomp/SymbolTableTest.java b/test/com/google/javascript/jscomp/SymbolTableTest.java
index d6419ec3313..8da476e3693 100644
--- a/test/com/google/javascript/jscomp/SymbolTableTest.java
+++ b/test/com/google/javascript/jscomp/SymbolTableTest.java
@@ -189,6 +189,7 @@ public void testLocalThisReferences2() throws Exception {
     assertThat(refs).hasSize(2);
   }
 
+  // No 'this' reference is created for empty functions.
   public void testLocalThisReferences3() throws Exception {
     SymbolTable table = createSymbolTable(
         "/** @constructor */ function F() {}");
@@ -197,10 +198,7 @@ public void testLocalThisReferences3() throws Exception {
     assertNotNull(baz);
 
     Symbol t = table.getParameterInFunction(baz, "this");
-    assertNotNull(t);
-
-    List<Reference> refs = table.getReferenceList(t);
-    assertThat(refs).isEmpty();
+    assertNull(t);
   }
 
   public void testNamespacedReferences() throws Exception {