Skip to content

Commit 7bb038f

Browse files
lauraharkercopybara-github
authored andcommitted
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
1 parent 86750a3 commit 7bb038f

File tree

2 files changed

+11
-19
lines changed

2 files changed

+11
-19
lines changed

src/com/google/javascript/jscomp/ConcretizeStaticInheritanceForInlining.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.base.Preconditions.checkArgument;
2020
import static com.google.common.base.Preconditions.checkState;
2121

22+
import com.google.common.base.Preconditions;
2223
import com.google.common.collect.ImmutableSet;
2324
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
2425
import com.google.javascript.rhino.IR;
@@ -325,6 +326,9 @@ private void visitFunctionClassDef(Node n) {
325326
JSDocInfo classInfo = NodeUtil.getBestJSDocInfo(n);
326327
if (classInfo != null && classInfo.isConstructor()) {
327328
String name = NodeUtil.getName(n);
329+
if (name == null) {
330+
return;
331+
}
328332
if (classByAlias.containsKey(name)) {
329333
duplicateClassNames.add(name);
330334
} else {
@@ -334,6 +338,8 @@ private void visitFunctionClassDef(Node n) {
334338
}
335339

336340
private void setAlias(String original, String alias) {
341+
Preconditions.checkNotNull(original, "original is null");
342+
Preconditions.checkNotNull(alias, "alias is null");
337343
checkArgument(classByAlias.containsKey(original));
338344
classByAlias.put(alias, classByAlias.get(original));
339345
}
@@ -369,7 +375,7 @@ private void visitVariableDeclaration(Node n) {
369375
return;
370376
}
371377
String maybeOriginalName = child.getFirstChild().getQualifiedName();
372-
if (classByAlias.containsKey(maybeOriginalName)) {
378+
if (maybeOriginalName != null && classByAlias.containsKey(maybeOriginalName)) {
373379
String maybeAlias = child.getQualifiedName();
374380
if (maybeAlias != null) {
375381
setAlias(maybeOriginalName, maybeAlias);

test/com/google/javascript/jscomp/ConcretizeStaticInheritanceForInliningTest.java

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -628,20 +628,9 @@ public void testAddSingletonGetter() {
628628

629629
@Test
630630
public void testNonQnameConstructor_doesntPolluteListOfAssignments() {
631-
// Reproduce a pretty bad bug caused by accidentally reading/writing 'null' keys from a map.
632-
test(
633-
lines(
634-
"const ns = {};",
635-
"/** @constructor */",
636-
"ns['NOT_A_NAME'] = function() {};",
637-
"ns['NOT_A_NAME'].staticMethod = function() { alert(1); }",
638-
"",
639-
"/** @constructor */",
640-
"const Example = function() {}",
641-
"",
642-
"/** @constructor @extends {Example} */",
643-
"function Subclass() {}",
644-
"$jscomp.inherits(Subclass, Example);"),
631+
// Reproduce a bug that once created a nonsensical assignment:
632+
// Subclass.staticMethod = Example.staticMethod;
633+
testSame(
645634
lines(
646635
"const ns = {};",
647636
"/** @constructor */",
@@ -653,9 +642,6 @@ public void testNonQnameConstructor_doesntPolluteListOfAssignments() {
653642
"",
654643
"/** @constructor @extends {Example} */",
655644
"function Subclass() {}",
656-
"$jscomp.inherits(Subclass, Example);",
657-
// TODO(b/293320792) - stop producing this assignment, there's no
658-
// actual Example.staticMethod.
659-
"Subclass.staticMethod = Example.staticMethod;"));
645+
"$jscomp.inherits(Subclass, Example);"));
660646
}
661647
}

0 commit comments

Comments
 (0)