Correct logic for NodeUtil.isEs6Constructor()

It is possible for a class to have a static member named "constructor". ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=231829535
google · Feb 1, 2019 · bb170cd · bb170cd
1 parent 1836f5b
commit bb170cd
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 4 deletions.
diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java
@@ -5697,10 +5697,21 @@ static boolean isConstructor(Node fnNode) {
   }
 
   private static boolean isEs6Constructor(Node fnNode) {
-    return fnNode.isFunction()
-        && fnNode.getGrandparent() != null
-        && fnNode.getGrandparent().isClassMembers()
-        && fnNode.getParent().matchesQualifiedName("constructor");
+    if (!fnNode.isFunction()) {
+      return false;
+    }
+    Node memberFunctionDef = fnNode.getParent();
+    if (memberFunctionDef == null || !memberFunctionDef.isMemberFunctionDef()) {
+      return false; // not a member function
+    }
+    if (memberFunctionDef.isStaticMember()) {
+      return false; // it is possible to have a static method named "constructor"
+    }
+    Node classMembers = memberFunctionDef.getParent();
+    if (classMembers == null || !classMembers.isClassMembers()) {
+      return false; // method definition in an object literal
+    }
+    return memberFunctionDef.matchesQualifiedName("constructor");
   }
 
   static boolean isGetterOrSetter(Node propNode) {

diff --git a/test/com/google/javascript/jscomp/NodeUtilTest.java b/test/com/google/javascript/jscomp/NodeUtilTest.java
@@ -3582,6 +3582,11 @@ public void testIsConstructor() {
                 getFunctionNode("var x = {}; /** @constructor */ x.Foo = function() {}")))
         .isTrue();
     assertThat(NodeUtil.isConstructor(getFunctionNode("class Foo { constructor() {} }"))).isTrue();
+    assertThat(NodeUtil.isConstructor(getFunctionNode("class Foo { static constructor() {} }")))
+        .isFalse();
+    assertThat(NodeUtil.isConstructor(getFunctionNode("class Foo { get constructor() {} }")))
+        .isFalse();
+    assertThat(NodeUtil.isConstructor(getFunctionNode("let Foo = { constructor() {} }"))).isFalse();
 
     assertThat(NodeUtil.isConstructor(getFunctionNode("function Foo() {}"))).isFalse();
     assertThat(NodeUtil.isConstructor(getFunctionNode("var Foo = function() {}"))).isFalse();