Fix removal of ES6 class member functions.

Previously, they would inadvertantly remove the entire class, since that is the closest enclosing statement. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=180861525
google · Jan 5, 2018 · 3a69df6 · 3a69df6
1 parent bebe76b
commit 3a69df6
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 3 deletions.
diff --git a/src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java
@@ -517,8 +517,7 @@ private RemovalType shouldRemove(PotentialDeclaration decl) {
       }
       Node nameNode = decl.getLhs();
       Node rhs = decl.getRhs();
-      Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode);
-      JSDocInfo jsdoc = jsdocNode.getJSDocInfo();
+      JSDocInfo jsdoc = decl.getJsDoc();
       boolean isExport = isExportLhs(nameNode);
       if (rhs == null) {
         return RemovalType.SIMPLIFY_RHS;
@@ -538,10 +537,12 @@ private RemovalType shouldRemove(PotentialDeclaration decl) {
         return RemovalType.SIMPLIFY_RHS;
       }
       if (isConstToBeInferred(jsdoc, nameNode, isExport)) {
+        Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode);
         jsdocNode.setJSDocInfo(JsdocUtil.pullJsdocTypeFromAst(compiler, jsdoc, nameNode));
         return RemovalType.SIMPLIFY_RHS;
       }
       if (jsdoc == null || !jsdoc.containsDeclaration()) {
+        Node jsdocNode = NodeUtil.getBestJSDocInfoNode(nameNode);
         if (!isDeclaration(nameNode)
             && !currentFile.isPrefixProvided(fullyQualifiedName)
             && !currentFile.isStrictPrefixDeclared(fullyQualifiedName)) {

diff --git a/src/com/google/javascript/jscomp/ijs/PotentialDeclaration.java b/src/com/google/javascript/jscomp/ijs/PotentialDeclaration.java
@@ -105,10 +105,14 @@ private boolean isDetached() {
    * Remove this "potential declaration" completely.
    * Usually, this is because the same symbol has already been declared in this file.
    */
-  void remove(AbstractCompiler compiler) {
+  final void remove(AbstractCompiler compiler) {
     if (isDetached()) {
       return;
     }
+    doRemove(compiler);
+  }
+
+  void doRemove(AbstractCompiler compiler) {
     Node statement = getStatement();
     NodeUtil.deleteNode(statement, compiler);
     statement.removeChildren();
@@ -248,6 +252,11 @@ private static class MethodDeclaration extends PotentialDeclaration {
 
     @Override
     void simplify(AbstractCompiler compiler) {}
+
+    @Override
+    void doRemove(AbstractCompiler compiler) {
+      NodeUtil.deleteNode(getLhs(), compiler);
+    }
   }
 
   static boolean isTypedRhs(Node rhs) {

diff --git a/test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java b/test/com/google/javascript/jscomp/ijs/ConvertToTypedInterfaceTest.java
@@ -1337,6 +1337,47 @@ public void testSameNamedStaticAndNonstaticMethodsDontCrash() {
             ""));
   }
 
+  public void testRedeclarationOfClassMethodDoesntCrash() {
+    test(
+        lines(
+            "class Foo {",
+            "  constructor() {",
+            "    /** @private */",
+            "    this.handleEvent_ = this.handleEvent_.bind(this);",
+            "  }",
+            "  /** @private @param {Event} e */",
+            "  handleEvent_(e) {}",
+            "}",
+            ""),
+        lines(
+            "class Foo {",
+            "  constructor() {}",
+            "}",
+            // TODO(blickly): Would be better if this included the type anntoation.
+            "/** @private */",
+            "Foo.prototype.handleEvent_",
+            ""));
+
+    test(
+        lines(
+            "class Foo {",
+            "  constructor() {",
+            "    /** @param {Event} e */",
+            "    this.handleEvent_ = function (e) {};",
+            "  }",
+            "  handleEvent_(e) {}",
+            "}",
+            ""),
+        lines(
+            "class Foo {",
+            "  constructor() {",
+            "    /** @param {Event} e */",
+            "    this.handleEvent_ = function (e) {};",
+            "  }",
+            "}",
+            ""));
+  }
+
 
   public void testDescAnnotationCountsAsTyped() {
     test(