Skip to content

Commit 2ba8fbc

Browse files
trevoradecopybara-github
trevorade
authored and
copybara-github
committed
No public description
PiperOrigin-RevId: 830954224
1 parent dc0e099 commit 2ba8fbc

File tree

9 files changed

+548
-240
lines changed

9 files changed

+548
-240
lines changed

‎src/com/google/javascript/jscomp/DefaultPassConfig.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ protected PassListBuilder getTranspileOnlyPasses() {
162162

163163
// Passes below this point may rely on normalization and must maintain normalization.
164164
passes.maybeAdd(normalize);
165+
166+
passes.maybeAdd(gatherGettersAndSetters);
167+
165168
TranspilationPasses.addTranspilationPasses(passes, options);
166169
// The transpilation passes may rely on normalize making all variables unique,
167170
// but we're doing only transpilation, so we want to put back the original variable names
@@ -176,6 +179,9 @@ protected PassListBuilder getTranspileOnlyPasses() {
176179

177180
passes.maybeAdd(unwrapClosureUnawareCode);
178181

182+
// Es6ConvertSuper may add this so it needs to be removed for transpile-only mode.
183+
passes.maybeAdd(removePropertyRenamingCalls);
184+
179185
passes.assertAllOneTimePasses();
180186
assertValidOrderForChecks(passes);
181187
return passes;
@@ -485,6 +491,7 @@ protected PassListBuilder getOptimizations() {
485491
passes.maybeAdd(closureProvidesRequires);
486492
passes.maybeAdd(processDefinesOptimize);
487493
passes.maybeAdd(normalize);
494+
passes.maybeAdd(gatherGettersAndSetters);
488495
TranspilationPasses.addTranspilationPasses(passes, options);
489496
passes.maybeAdd(gatherExternPropertiesOptimize);
490497
passes.maybeAdd(createEmptyPass(PassNames.BEFORE_STANDARD_OPTIMIZATIONS));
@@ -904,11 +911,11 @@ private PassListBuilder getEarlyOptimizationPasses() {
904911

905912
passes.maybeAdd(normalize);
906913

914+
passes.maybeAdd(gatherGettersAndSetters);
915+
907916
// TODO(b/329447979): Add an early removeUnusedCode pass here
908917
TranspilationPasses.addTranspilationPasses(passes, options);
909918

910-
passes.maybeAdd(gatherGettersAndSetters);
911-
912919
if (options.j2clPassMode.shouldAddJ2clPasses()) {
913920
passes.maybeAdd(j2clUtilGetDefineRewriterPass);
914921
}

‎src/com/google/javascript/jscomp/Es6ConvertSuper.java‎

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616
package com.google.javascript.jscomp;
1717

1818
import static com.google.common.base.Preconditions.checkState;
19+
import static com.google.javascript.jscomp.AstFactory.type;
1920
import static com.google.javascript.jscomp.TranspilationUtil.CANNOT_CONVERT_YET;
2021

22+
import com.google.javascript.jscomp.colors.StandardColors;
2123
import com.google.javascript.jscomp.parsing.parser.FeatureSet;
2224
import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
25+
import com.google.javascript.rhino.IR;
2326
import com.google.javascript.rhino.JSDocInfo;
2427
import com.google.javascript.rhino.Node;
2528

@@ -154,7 +157,20 @@ private void visitSuperPropertyCall(Node node, Node parent, Node enclosingMember
154157
}
155158

156159
Node callTarget = parent;
157-
if (!enclosingMemberDef.isStaticMember()) {
160+
if (enclosingMemberDef.isStaticMember()) {
161+
Node expandedSuper = superName.cloneTree().srcrefTree(node);
162+
expandedSuper.setOriginalName("super");
163+
node.replaceWith(expandedSuper);
164+
callTarget = astFactory.createGetPropWithUnknownType(callTarget.detach(), "call");
165+
grandparent.addChildToFront(callTarget);
166+
Node thisNode = astFactory.createThis(type(clazz));
167+
thisNode.makeNonIndexable(); // no direct correlation with original source
168+
thisNode.insertAfter(callTarget);
169+
grandparent.srcrefTreeIfMissing(parent);
170+
171+
// We added a `this` reference to the call. We need to make sure this member is not collapsed.
172+
ensureNoCollapseOnStaticMemberDef(enclosingMemberDef);
173+
} else {
158174
// Replace super node to give
159175
// super.method(...) -> SuperClass.prototype.method(...)
160176
Node expandedSuper = astFactory.createPrototypeAccess(superName.cloneTree()).srcrefTree(node);
@@ -191,16 +207,69 @@ private void visitSuperPropertyAccess(Node node, Node parent, Node enclosingMemb
191207
return;
192208
}
193209

194-
if (!enclosingMemberDef.isStaticMember()) {
210+
Node newProp;
211+
if (enclosingMemberDef.isStaticMember()) {
212+
// super.prop -> SuperClass.prop
213+
newProp = superName.cloneTree().srcrefTree(node);
214+
} else {
195215
// super.prop -> SuperClass.prototype.prop
196-
Node newprop = astFactory.createPrototypeAccess(superName.cloneTree()).srcrefTree(node);
197-
newprop.setOriginalName("super");
198-
node.replaceWith(newprop);
216+
newProp = astFactory.createPrototypeAccess(superName.cloneTree()).srcrefTree(node);
199217
}
200218

219+
if (parent.isGetProp()
220+
&& compiler.getAccessorSummary().getKind(parent.getString()).hasGetter()) {
221+
// Rewrites
222+
// super.x
223+
// as
224+
// Reflect.get(super, JSCompiler_renameProperty('x', ParentClass), this)
225+
Node superPlaceholder = IR.name("sp"); // No need for type info as this will be replaced.
226+
227+
Node reflectGetCall =
228+
astFactory
229+
.createCall(
230+
astFactory.createGetPropWithUnknownType(
231+
astFactory.createConstantName("Reflect", type(StandardColors.UNKNOWN)),
232+
"get"),
233+
type(parent),
234+
superPlaceholder,
235+
parent.isGetProp()
236+
? astFactory.createCall(
237+
astFactory.createName(
238+
NodeUtil.JSC_PROPERTY_NAME_FN, type(StandardColors.UNKNOWN)),
239+
type(StandardColors.STRING),
240+
astFactory.createString(parent.getString()),
241+
superName.cloneTree().srcrefTree(node))
242+
:
243+
// For getElem, we just use the accessor value in the Reflect.get call.
244+
node.getNext().detach(),
245+
astFactory.createThis(type(clazz)))
246+
.srcrefTreeIfMissing(parent);
247+
248+
// Replace the getProp/getElem with the Reflect.get call.
249+
parent.replaceWith(reflectGetCall);
250+
// Swap in the `n` (super) where it goes in the Reflect.get call.
251+
superPlaceholder.replaceWith(node.detach());
252+
253+
// We added a `this` reference to the call. We need to make sure this member is not collapsed.
254+
ensureNoCollapseOnStaticMemberDef(enclosingMemberDef);
255+
}
256+
257+
node.replaceWith(newProp);
258+
newProp.setOriginalName("super");
259+
201260
compiler.reportChangeToEnclosingScope(grandparent);
202261
}
203262

263+
private void ensureNoCollapseOnStaticMemberDef(Node memberDef) {
264+
if (!memberDef.isStaticMember()) {
265+
return;
266+
}
267+
JSDocInfo.Builder builder = JSDocInfo.Builder.maybeCopyFrom(memberDef.getJSDocInfo());
268+
if (builder.recordNoCollapse()) {
269+
memberDef.setJSDocInfo(builder.build());
270+
}
271+
}
272+
204273
@Override
205274
public void process(Node externs, Node root) {
206275
// Might need to synthesize constructors for ambient classes in .d.ts externs

‎src/com/google/javascript/jscomp/Es6NormalizeClasses.java‎

Lines changed: 14 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,9 @@ private String generateUniqueStaticInitMethodName(NodeTraversal t) {
9494
private final ExpressionDecomposer expressionDecomposer;
9595
private final SynthesizeExplicitConstructors ctorCreator;
9696
private final boolean transpileClassFields;
97-
private final boolean rewriteStaticSuperStrictly;
97+
98+
private final boolean doStaticInheritanceRewrites;
99+
private final boolean doStaticSuperRewrites;
98100

99101
Es6NormalizeClasses(AbstractCompiler compiler) {
100102
this.compiler = compiler;
@@ -105,14 +107,11 @@ private String generateUniqueStaticInitMethodName(NodeTraversal t) {
105107
var options = compiler.getOptions();
106108
transpileClassFields = options.needsTranspilationOf(Feature.PUBLIC_CLASS_FIELDS);
107109

108-
// Written this way for historical consistency. Before this comment was added, if SUPER needed
109-
// transpilation, Es6ConvertSuper would do a strict rewrite. Otherwise if the options indicate
110-
// to not skipNonTranspilationPasses and assumeStaticInheritanceIsNotUsed,
111-
// StaticSuperPropReplacer (now deleted) would do a non-strict rewrite.
112-
rewriteStaticSuperStrictly =
113-
options.needsTranspilationOf(Feature.SUPER)
114-
|| options.skipNonTranspilationPasses
115-
|| !options.getAssumeStaticInheritanceIsNotUsed();
110+
doStaticInheritanceRewrites =
111+
options.getAssumeStaticInheritanceIsNotUsed() && !options.skipNonTranspilationPasses;
112+
113+
// Es6ConvertSuper handles static super rewrites otherwise.
114+
doStaticSuperRewrites = !options.needsTranspilationOf(Feature.SUPER);
116115
}
117116

118117
@Override
@@ -737,16 +736,6 @@ private void rewriteStaticMembers(NodeTraversal t, ClassRecord record) {
737736
// Record the feature corresponding to a static method.
738737
NodeUtil.addFeatureToScript(t.getCurrentScript(), Feature.MEMBER_DECLARATIONS, compiler);
739738

740-
// If the new method has any `this` usages adds @nocollapse.
741-
// Note: while all `this` references in static initialization contexts were converted to
742-
// `ClassName`, in the case of rewriteStaticSuperStrictly, calls like `super.foo()` were
743-
// converted to `ParentClass.foo.call(this)`.
744-
if (rewriteStaticSuperStrictly && NodeUtil.referencesOwnReceiver(staticInitMethodFunc)) {
745-
JSDocInfo.Builder builder = JSDocInfo.Builder.maybeCopyFrom(staticInitMethod.getJSDocInfo());
746-
builder.recordNoCollapse();
747-
staticInitMethod.setJSDocInfo(builder.build());
748-
}
749-
750739
// Add a call to the static initialization method after the class definition.
751740
Node staticInitCall =
752741
astFactory
@@ -806,6 +795,10 @@ private void maybeUpdateClassSelfRef(NodeTraversal t, Node nameNode) {
806795
* <p>Note: This runs before {@link #rewriteStaticMembers} so the static members are still there.
807796
*/
808797
private void visitThisAndSuper(NodeTraversal t, Node n) {
798+
if (!doStaticInheritanceRewrites) {
799+
return;
800+
}
801+
809802
Node rootNode = t.getClosestScopeRootNodeBindingThisOrSuper();
810803
Node rootParent = rootNode.getParent();
811804
if (rootNode.isFunction()
@@ -833,30 +826,10 @@ private void visitThisAndSuper(NodeTraversal t, Node n) {
833826
// For static this, we can only universally replace with the class name in a static
834827
// initialization context.
835828
newNameNode = classRecord.createNewNameReferenceNode().srcrefTree(n);
836-
} else if (n.isSuper()) {
837-
// For static super, can always replace the super class name.
838-
// b/454921132: Technically, for `super.getter`, `getter` in `ParentClass` may have a
839-
// polymorphic `this` usage which `ParentClass.getter` would break. The correct fix is to
840-
// rewrite property accesses as `Reflect.get(ParentClass, 'getter', ClassName)`.
829+
} else if (doStaticSuperRewrites && n.isSuper()) {
830+
// For a static super, we can always rewrite the super as something else.
841831
checkState(classRecord.superClassNameNode.isPresent(), classRecord.classNode);
842832
newNameNode = classRecord.superClassNameNode.get().cloneTree();
843-
844-
if (rewriteStaticSuperStrictly
845-
&& n.getGrandparent().isCall()
846-
&& NodeUtil.isInvocationTarget(n.getParent())) {
847-
// If this is a call, we need to replace `super.m(x)` with `super.m.call(this, x)` (`super`
848-
// will be replaced with `ParentClassName` (newNameNode) afterwards).
849-
Node call = n.getGrandparent();
850-
Node callTarget = n.getParent();
851-
callTarget =
852-
astFactory
853-
.createGetPropWithUnknownType(callTarget.detach(), "call")
854-
.srcrefTreeIfMissing(callTarget);
855-
call.addChildToFront(callTarget);
856-
Node thisNode = astFactory.createThis(type(classNode)).srcrefTreeIfMissing(n);
857-
thisNode.makeNonIndexable();
858-
thisNode.insertAfter(callTarget);
859-
}
860833
}
861834

862835
if (newNameNode != null) {

‎src/com/google/javascript/jscomp/PassNames.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public final class PassNames {
5757
public static final String DESERIALIZE_COMPILER_STATE = "deserializeCompilerState";
5858
public static final String DEVIRTUALIZE_METHODS = "devirtualizeMethods";
5959
public static final String DISAMBIGUATE_PROPERTIES = "disambiguateProperties";
60-
public static final String ES6_NORMALIZE_CLASSES = "ES6_NORMALIZE_CLASSES";
60+
public static final String ES6_NORMALIZE_CLASSES = "es6NormalizeClasses";
6161
public static final String EXPLOIT_ASSIGN = "exploitAssign";
6262
public static final String EXPORT_TEST_FUNCTIONS = "exportTestFunctions";
6363
public static final String EXTERN_EXPORTS = "externExports";

‎test/com/google/javascript/jscomp/Es6ConvertSuperTest.java‎

Lines changed: 110 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -478,9 +478,13 @@ class B extends A {
478478
expected(
479479
"""
480480
class B extends A {
481-
constructor() { super(); }
482-
483-
static f() { A.g.call(this, 3); }
481+
constructor() {
482+
super();
483+
}
484+
/** @nocollapse */
485+
static f() {
486+
A.g.call(this, 3);
487+
}
484488
}
485489
"""));
486490

@@ -572,8 +576,10 @@ class B extends A {
572576
constructor() {
573577
super();
574578
}
575-
576-
static ['f']() { A['g'].call(this, 4); }
579+
/** @nocollapse */
580+
static ["f"]() {
581+
A["g"].call(this, 4);
582+
}
577583
}
578584
"""));
579585

@@ -974,6 +980,105 @@ class B extends A {
974980
assertNode(thisNode).hasColorThat().isEqualTo(classBInstanceType);
975981
}
976982

983+
@Test
984+
public void testAccessingStaticGetterViaSuper() {
985+
test(
986+
externs(
987+
"""
988+
class Parent {
989+
constructor() {}
990+
/** @return {string} */
991+
static getName() {}
992+
/** @return {string} */
993+
static get greeting() {}
994+
}
995+
"""),
996+
srcs(
997+
"""
998+
class Child extends Parent {
999+
constructor() {
1000+
super();
1001+
}
1002+
static getName() {
1003+
return 'Child';
1004+
}
1005+
static getGreeting() {
1006+
return super.greeting;
1007+
}
1008+
}
1009+
"""),
1010+
expected(
1011+
"""
1012+
class Child extends Parent {
1013+
constructor() {
1014+
super();
1015+
}
1016+
static getName() {
1017+
return 'Child';
1018+
}
1019+
/** @nocollapse */
1020+
static getGreeting() {
1021+
return Reflect.get(
1022+
Parent, JSCompiler_renameProperty('greeting', Parent), this);
1023+
}
1024+
}
1025+
"""));
1026+
1027+
// get types we need to check
1028+
Color parentType = findClassDefinition(getLastCompiler(), "Parent").getRootNode().getColor();
1029+
Color childType = findClassDefinition(getLastCompiler(), "Child").getRootNode().getColor();
1030+
1031+
// return Reflect.get(Parent, JSCompiler_renameProperty("greeting", Parent), this);
1032+
Node returnStmt =
1033+
findClassDefinition(getLastCompiler(), "Child")
1034+
.findMethodDefinition("getGreeting")
1035+
.getRootNode() // MEMBER_FUNCTION_DEF
1036+
.getOnlyChild() // FUNCTION
1037+
.getLastChild() // BLOCK
1038+
.getOnlyChild(); // RETURN
1039+
assertNode(returnStmt).hasToken(Token.RETURN);
1040+
1041+
// Reflect.get(Parent, JSCompiler_renameProperty("greeting", Parent), this)
1042+
Node reflectGetCall = returnStmt.getOnlyChild();
1043+
assertNode(reflectGetCall).hasToken(Token.CALL).hasColorThat().isEqualTo(StandardColors.STRING);
1044+
1045+
// Reflect.get
1046+
Node callee = reflectGetCall.getFirstChild();
1047+
assertNode(callee).matchesQualifiedName("Reflect.get");
1048+
1049+
// Parent
1050+
Node parentName = callee.getNext();
1051+
assertNode(parentName)
1052+
.matchesQualifiedName("Parent")
1053+
.hasOriginalName("super")
1054+
.hasColorThat()
1055+
.isEqualTo(parentType);
1056+
1057+
// JSCompiler_renameProperty("greeting", Parent)
1058+
Node renamePropertyCall = parentName.getNext();
1059+
assertNode(renamePropertyCall)
1060+
.hasToken(Token.CALL)
1061+
.hasColorThat()
1062+
.isEqualTo(StandardColors.STRING);
1063+
1064+
{
1065+
// "greeting"
1066+
Node greetingString = renamePropertyCall.getSecondChild();
1067+
assertNode(greetingString)
1068+
.isString("greeting")
1069+
.hasColorThat()
1070+
.isEqualTo(StandardColors.STRING);
1071+
1072+
// Parent
1073+
Node parentNameArg = greetingString.getNext();
1074+
assertNode(parentNameArg).matchesQualifiedName("Parent").hasColorThat().isEqualTo(parentType);
1075+
}
1076+
1077+
// this
1078+
Node thisNode = renamePropertyCall.getNext();
1079+
assertNode(thisNode).hasToken(Token.THIS).hasColorThat().isEqualTo(childType);
1080+
}
1081+
9771082
// Constructor synthesis
9781083

9791084
@Test

0 commit comments

Comments
 (0)