Skip to content

Commit 454e6c8

Browse files
brad4dcopybara-github
authored andcommitted
RewriteClassMembers: correct creation of class name for this
The pass was assuming a CLASS node's first child is always the correct NAME node to use. PiperOrigin-RevId: 562878308
1 parent 64b4642 commit 454e6c8

File tree

2 files changed

+76
-25
lines changed

2 files changed

+76
-25
lines changed

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

+50-25
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
7171
n.getFirstChild().isEmpty() || classNameNode.matchesQualifiedName(n.getFirstChild()),
7272
"Class name shadows variable declaring the class: %s",
7373
n);
74-
classStack.push(new ClassRecord(n, classNameNode.getQualifiedName(), classInsertionPoint));
74+
classStack.push(new ClassRecord(n, classNameNode, classInsertionPoint));
7575
break;
7676
case COMPUTED_FIELD_DEF:
7777
checkState(!classStack.isEmpty());
@@ -125,40 +125,56 @@ public void exitScope(NodeTraversal t) {}
125125
public void visit(NodeTraversal t, Node n, Node parent) {
126126
switch (n.getToken()) {
127127
case CLASS:
128-
visitClass(t);
129-
return;
128+
visitClass(t, n);
129+
break;
130130
case THIS:
131-
Node rootNode = t.getClosestScopeRootNodeBindingThisOrSuper();
132-
if (rootNode.isStaticMember()
133-
&& (rootNode.isMemberFieldDef() || rootNode.isComputedFieldDef())) {
134-
Node className = rootNode.getGrandparent().getFirstChild().cloneNode();
135-
n.replaceWith(className);
136-
t.reportCodeChange(className);
137-
}
138-
return;
131+
visitThis(t, n);
132+
break;
139133
case SUPER:
140-
rootNode = t.getClosestScopeRootNodeBindingThisOrSuper();
141-
if (rootNode.isMemberFieldDef() && rootNode.isStaticMember()) {
142-
Node superclassName = rootNode.getGrandparent().getChildAtIndex(1).cloneNode();
143-
n.replaceWith(superclassName);
144-
t.reportCodeChange(superclassName);
145-
}
146-
return;
134+
visitSuper(t, n);
135+
break;
147136
default:
148-
return;
137+
break;
149138
}
150139
}
151140

152141
/** Transpile the actual class members themselves */
153-
private void visitClass(NodeTraversal t) {
142+
private void visitClass(NodeTraversal t, Node classNode) {
154143
ClassRecord currClassRecord = classStack.pop();
144+
checkState(currClassRecord.classNode == classNode, "unexpected node: %s", classNode);
155145
if (currClassRecord.cannotConvert) {
156146
return;
157147
}
158148
rewriteInstanceMembers(t, currClassRecord);
159149
rewriteStaticMembers(t, currClassRecord);
160150
}
161151

152+
private void visitThis(NodeTraversal t, Node thisNode) {
153+
Node rootNode = t.getClosestScopeRootNodeBindingThisOrSuper();
154+
if (rootNode.isStaticMember()
155+
&& (rootNode.isMemberFieldDef() || rootNode.isComputedFieldDef())) {
156+
final Node classNode = rootNode.getGrandparent();
157+
final ClassRecord classRecord = classStack.peek();
158+
checkState(
159+
classRecord.classNode == classNode,
160+
"wrong class node: %s != %s",
161+
classRecord.classNode,
162+
classNode);
163+
Node className = classRecord.createNewNameReferenceNode().srcrefTree(thisNode);
164+
thisNode.replaceWith(className);
165+
t.reportCodeChange(className);
166+
}
167+
}
168+
169+
private void visitSuper(NodeTraversal t, Node n) {
170+
Node rootNode = t.getClosestScopeRootNodeBindingThisOrSuper();
171+
if (rootNode.isMemberFieldDef() && rootNode.isStaticMember()) {
172+
Node superclassName = rootNode.getGrandparent().getChildAtIndex(1).cloneNode();
173+
n.replaceWith(superclassName);
174+
t.reportCodeChange(superclassName);
175+
}
176+
}
177+
162178
/**
163179
* Move computed fields with side effects into a computed function (Token is COMPUTED_PROP) to
164180
* preserve the original behavior of possible side effects such as printing something in the call
@@ -316,8 +332,7 @@ private void rewriteStaticMembers(NodeTraversal t, ClassRecord record) {
316332
Node staticMember = staticMembers.pop();
317333
// if the name is a property access, we want the whole chain of accesses, while for other
318334
// cases we only want the name node
319-
Node nameToUse =
320-
astFactory.createQNameWithUnknownType(record.classNameString).srcrefTree(staticMember);
335+
Node nameToUse = record.createNewNameReferenceNode().srcrefTree(staticMember);
321336

322337
Node transpiledNode;
323338

@@ -454,17 +469,17 @@ private static final class ClassRecord {
454469
ImmutableSet<Var> constructorVars = ImmutableSet.of();
455470

456471
final Node classNode;
457-
final String classNameString;
472+
final Node classNameNode;
458473

459474
// Keeps track of the statement declaring the class
460475
final Node insertionPointBeforeClass;
461476

462477
// Keeps track of the last statement inserted after the class
463478
Node insertionPointAfterClass;
464479

465-
ClassRecord(Node classNode, String classNameString, Node classInsertionPoint) {
480+
ClassRecord(Node classNode, Node classNameNode, Node classInsertionPoint) {
466481
this.classNode = classNode;
467-
this.classNameString = classNameString;
482+
this.classNameNode = classNameNode;
468483
this.insertionPointBeforeClass = classInsertionPoint;
469484
this.insertionPointAfterClass = classInsertionPoint;
470485
}
@@ -492,5 +507,15 @@ void recordConstructorScope(Scope s) {
492507
builder.addAll(argsScope.getAllSymbols());
493508
constructorVars = builder.build();
494509
}
510+
511+
Node createNewNameReferenceNode() {
512+
if (classNameNode.isName()) {
513+
// Don't cloneTree() here, because the name may have a child node, the class itself.
514+
return classNameNode.cloneNode();
515+
} else {
516+
// Must cloneTree() for a qualified name.
517+
return classNameNode.cloneTree();
518+
}
519+
}
495520
}
496521
}

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

+26
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,32 @@ public void testComputedPropInStaticField() {
412412
"C[1] = 1;",
413413
"C[2] = C[1];"));
414414

415+
test(
416+
lines(
417+
"/** @unrestricted */",
418+
"const C = class {", //
419+
" static [1] = 1;",
420+
" static [2] = this[1];",
421+
"}"),
422+
lines(
423+
"const C = class {}", //
424+
"C[1] = 1;",
425+
"C[2] = C[1];"));
426+
427+
test(
428+
lines(
429+
"/** @unrestricted */",
430+
"const C = class InnerC {", //
431+
" static [1] = 1;",
432+
" static [2] = this[1];",
433+
"}"),
434+
lines(
435+
"const testcode$classdecl$var0 = class {}", //
436+
"testcode$classdecl$var0[1] = 1;",
437+
"testcode$classdecl$var0[2] = testcode$classdecl$var0[1];",
438+
"/** @constructor */",
439+
"const C = testcode$classdecl$var0;"));
440+
415441
test(
416442
lines(
417443
"/** @unrestricted */",

0 commit comments

Comments
 (0)