Fix ConcretizeStaticInheritanceForInlining bug adding semi-random new…

… static props In theory, these properties were always dead-code eliminated and this won't affect the optimized code. There's a small chance some code accidentally referenced the added properties & will break. Such references should have caused type errors, though. PiperOrigin-RevId: 554551433
Vertispan · Aug 7, 2023 · 7bb038f · 7bb038f
1 parent 86750a3
commit 7bb038f
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 19 deletions.
diff --git a/src/com/google/javascript/jscomp/ConcretizeStaticInheritanceForInlining.java b/src/com/google/javascript/jscomp/ConcretizeStaticInheritanceForInlining.java
@@ -19,6 +19,7 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
 import com.google.javascript.rhino.IR;
@@ -325,6 +326,9 @@ private void visitFunctionClassDef(Node n) {
       JSDocInfo classInfo = NodeUtil.getBestJSDocInfo(n);
       if (classInfo != null && classInfo.isConstructor()) {
         String name = NodeUtil.getName(n);
+        if (name == null) {
+          return;
+        }
         if (classByAlias.containsKey(name)) {
           duplicateClassNames.add(name);
         } else {
@@ -334,6 +338,8 @@ private void visitFunctionClassDef(Node n) {
     }
 
     private void setAlias(String original, String alias) {
+      Preconditions.checkNotNull(original, "original is null");
+      Preconditions.checkNotNull(alias, "alias is null");
       checkArgument(classByAlias.containsKey(original));
       classByAlias.put(alias, classByAlias.get(original));
     }
@@ -369,7 +375,7 @@ private void visitVariableDeclaration(Node n) {
         return;
       }
       String maybeOriginalName = child.getFirstChild().getQualifiedName();
-      if (classByAlias.containsKey(maybeOriginalName)) {
+      if (maybeOriginalName != null && classByAlias.containsKey(maybeOriginalName)) {
         String maybeAlias = child.getQualifiedName();
         if (maybeAlias != null) {
           setAlias(maybeOriginalName, maybeAlias);

diff --git a/test/com/google/javascript/jscomp/ConcretizeStaticInheritanceForInliningTest.java b/test/com/google/javascript/jscomp/ConcretizeStaticInheritanceForInliningTest.java
@@ -628,20 +628,9 @@ public void testAddSingletonGetter() {
 
   @Test
   public void testNonQnameConstructor_doesntPolluteListOfAssignments() {
-    // Reproduce a pretty bad bug caused by accidentally reading/writing 'null' keys from a map.
-    test(
-        lines(
-            "const ns = {};",
-            "/** @constructor */",
-            "ns['NOT_A_NAME'] = function() {};",
-            "ns['NOT_A_NAME'].staticMethod = function() { alert(1); }",
-            "",
-            "/** @constructor */",
-            "const Example = function() {}",
-            "",
-            "/** @constructor @extends {Example} */",
-            "function Subclass() {}",
-            "$jscomp.inherits(Subclass, Example);"),
+    // Reproduce a bug that once created a nonsensical assignment:
+    //   Subclass.staticMethod = Example.staticMethod;
+    testSame(
         lines(
             "const ns = {};",
             "/** @constructor */",
@@ -653,9 +642,6 @@ public void testNonQnameConstructor_doesntPolluteListOfAssignments() {
             "",
             "/** @constructor @extends {Example} */",
             "function Subclass() {}",
-            "$jscomp.inherits(Subclass, Example);",
-            // TODO(b/293320792) - stop producing this assignment, there's no
-            // actual Example.staticMethod.
-            "Subclass.staticMethod = Example.staticMethod;"));
+            "$jscomp.inherits(Subclass, Example);"));
   }
 }