Skip to content

Commit

Permalink
Generated constructors should call super(...arguments);
Browse files Browse the repository at this point in the history
They were using `super.apply(this, arguments)` which is invalid
ES6 syntax.

Also cleans up some obsolete code that was only needed for NTI.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202377340
  • Loading branch information
brad4d committed Jun 28, 2018
1 parent 59d1e32 commit cebb785
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 49 deletions.
9 changes: 5 additions & 4 deletions src/com/google/javascript/jscomp/Es6ConvertSuper.java
Expand Up @@ -84,10 +84,11 @@ private void addSyntheticConstructor(Node classNode) {
// A call to super() shouldn't actually exist for these cases and is problematic to
// transpile, so don't generate it.
if (!classNode.isFromExterns() && !isInterface(classNode)) {
Node exprResult = IR.exprResult(IR.call(
IR.getprop(IR.superNode(), IR.string("apply")),
IR.thisNode(),
IR.name("arguments")));
// Generate required call to super()
// `super(...arguments);`
// Note that transpilation of spread args must occur after this pass for this to work.
Node exprResult =
IR.exprResult(NodeUtil.newCallNode(IR.superNode(), IR.spread(IR.name("arguments"))));
body.addChildToFront(exprResult);
}
Node constructor = IR.function(
Expand Down
Expand Up @@ -276,7 +276,10 @@ private Node createNewSuperCall(String superClassQName, Node superCall) {
newSuperCall.putBooleanProp(Node.FREE_CALL, false); // callee is now a getprop
newSuperCall.addChildAfter(IR.thisNode().useSourceInfoFrom(callee), superClassDotCall);
} else {
// super.apply(null|this, ...) -> SuperClass.apply(this, ...)
// spread transpilation does
// super(...arguments) -> super.apply(null, arguments)
// Now we must do
// super.apply(null, arguments) -> SuperClass.apply(this, arguments)
checkState(callee.isGetProp(), callee);
Node applyNode = checkNotNull(callee.getSecondChild());
checkState(applyNode.getString().equals("apply"), applyNode);
Expand All @@ -286,15 +289,10 @@ private Node createNewSuperCall(String superClassQName, Node superCall) {
callee.replaceChild(
superNode,
NodeUtil.newQName(compiler, superClassQName).useSourceInfoFromForTree(superNode));
// super.apply(null, ...) is generated by spread transpilation
// super.apply(this, arguments) is used by Es6ConvertSuper in automatically-generated
// constructors.
Node nullOrThisNode = superCall.getFirstChild();
if (!nullOrThisNode.isThis()) {
checkState(nullOrThisNode.isNull(), nullOrThisNode);
superCall.removeChild(nullOrThisNode);
newSuperCall.addChildToBack(IR.thisNode().useSourceInfoFrom(nullOrThisNode));
}
Node nullNode = superCall.getFirstChild();
checkState(nullNode.isNull(), nullNode);
superCall.removeChild(nullNode);
newSuperCall.addChildToBack(IR.thisNode().useSourceInfoFrom(nullNode));
}
while (superCall.hasChildren()) {
newSuperCall.addChildToBack(superCall.removeFirstChild());
Expand Down
10 changes: 1 addition & 9 deletions src/com/google/javascript/jscomp/Es6RewriteClass.java
Expand Up @@ -44,9 +44,6 @@ public final class Es6RewriteClass implements NodeTraversal.Callback, HotSwapCom
private static final FeatureSet features =
FeatureSet.BARE_MINIMUM.with(Feature.CLASSES, Feature.NEW_TARGET);

// Whether to add $jscomp.inherits(Parent, Child) for each subclass.
private final boolean shouldAddInheritsPolyfill;

static final DiagnosticType DYNAMIC_EXTENDS_TYPE = DiagnosticType.error(
"JSC_DYNAMIC_EXTENDS_TYPE",
"The class in an extends clause must be a qualified name.");
Expand All @@ -63,12 +60,7 @@ public final class Es6RewriteClass implements NodeTraversal.Callback, HotSwapCom
static final String INHERITS = "$jscomp.inherits";

public Es6RewriteClass(AbstractCompiler compiler) {
this(compiler, true);
}

public Es6RewriteClass(AbstractCompiler compiler, boolean shouldAddInheritsPolyfill) {
this.compiler = compiler;
this.shouldAddInheritsPolyfill = shouldAddInheritsPolyfill;
}

@Override
Expand Down Expand Up @@ -224,7 +216,7 @@ private void visitClass(final NodeTraversal t, final Node classNode, final Node
IR.string(superClassString)),
metadata.superClassNameNode.getSourceFileName()));
} else {
if (shouldAddInheritsPolyfill && !classNode.isFromExterns()) {
if (!classNode.isFromExterns()) {
Node classNameNode = NodeUtil.newQName(compiler, metadata.fullClassName)
.useSourceInfoIfMissingFrom(metadata.classNameNode);
Node superClassNameNode = metadata.superClassNameNode.cloneTree();
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/TranspilationPasses.java
Expand Up @@ -266,7 +266,7 @@ protected FeatureSet featureSet() {
new HotSwapPassFactory("Es6RewriteClass") {
@Override
protected HotSwapCompilerPass create(AbstractCompiler compiler) {
return new Es6RewriteClass(compiler, true);
return new Es6RewriteClass(compiler);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion test/com/google/javascript/jscomp/Es6ConvertSuperTest.java
Expand Up @@ -425,7 +425,7 @@ public void testSynthesizingConstructorOfDerivedClassInSource() {
"",
"class B extends A {",
" /** @param {...?} var_args */",
" constructor(var_args) { super.apply(this, arguments); }",
" constructor(var_args) { super(...arguments); }",
"}")));
}

Expand Down
32 changes: 9 additions & 23 deletions test/com/google/javascript/jscomp/Es6RewriteClassTest.java
Expand Up @@ -28,8 +28,6 @@

public final class Es6RewriteClassTest extends CompilerTestCase {

private boolean shouldAddInheritsPolyfill;

private static final String EXTERNS_BASE =
lines(
"/** @constructor @template T */",
Expand Down Expand Up @@ -73,7 +71,6 @@ protected void setUp() throws Exception {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_2015);
setLanguageOut(LanguageMode.ECMASCRIPT3);
enableRunTypeCheckAfterProcessing();
shouldAddInheritsPolyfill = true;
}

protected final PassFactory makePassFactory(
Expand All @@ -97,13 +94,15 @@ protected CompilerPass getProcessor(final Compiler compiler) {
optimizer.addOneTimePass(
makePassFactory("es6ConvertSuper", new Es6ConvertSuper(compiler)));
optimizer.addOneTimePass(makePassFactory("es6ExtractClasses", new Es6ExtractClasses(compiler)));
optimizer.addOneTimePass(makePassFactory("es6RewriteClass",
new Es6RewriteClass(compiler, shouldAddInheritsPolyfill)));
if (shouldAddInheritsPolyfill) {
optimizer.addOneTimePass(
makePassFactory(
"Es6ConvertSuperConstructorCalls", new Es6ConvertSuperConstructorCalls(compiler)));
}
optimizer.addOneTimePass(makePassFactory("es6RewriteClass", new Es6RewriteClass(compiler)));
// Automatically generated constructor calls will contain a call to super() using spread.
// super(...arguments);
// We depend on that getting rewritten before we do the super constructor call rewriting.
optimizer.addOneTimePass(
makePassFactory("es6RewriteRestAndSpread", new Es6RewriteRestAndSpread(compiler)));
optimizer.addOneTimePass(
makePassFactory(
"Es6ConvertSuperConstructorCalls", new Es6ConvertSuperConstructorCalls(compiler)));
return optimizer;
}

Expand Down Expand Up @@ -622,19 +621,6 @@ public void testExtends() {
"let C = function(var_args) {};"));
}

public void testExtendsWithoutInheritsPolyfill() {
shouldAddInheritsPolyfill = false;
test("class D {} class C extends D {}",
lines(
"/** @constructor @struct */",
"let D = function() {};",
"/** @constructor @struct",
" * @extends {D}",
" * @param {...?} var_args",
" */",
"let C = function(var_args) {super.apply(this,arguments); };"));
}

public void testExtendNonNativeError() {
test(
lines(
Expand Down
7 changes: 6 additions & 1 deletion test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java
Expand Up @@ -128,6 +128,11 @@ protected CompilerPass getProcessor(final Compiler compiler) {
optimizer.addOneTimePass(makePassFactory("es6RewriteClass", new Es6RewriteClass(compiler)));
optimizer.addOneTimePass(
makePassFactory("es6InjectRuntimeLibraries", new Es6InjectRuntimeLibraries(compiler)));
// Automatically generated constructor calls will contain a call to super() using spread.
// super(...arguments);
// We depend on that getting rewritten before we do the super constructor call rewriting.
optimizer.addOneTimePass(
makePassFactory("es6RewriteRestAndSpread", new Es6RewriteRestAndSpread(compiler)));
optimizer.addOneTimePass(
makePassFactory("convertEs6Late", new LateEs6ToEs3Converter(compiler)));
optimizer.addOneTimePass(makePassFactory("es6ForOf", new Es6ForOfConverter(compiler)));
Expand All @@ -151,7 +156,7 @@ public void testObjectLiteralStringKeysWithNoValue() {
}

public void testSpreadLibInjection() {
testSame("var x = [...a];");
test("var x = [...a];", "var x=[].concat($jscomp.arrayFromIterable(a))");
assertThat(getLastCompiler().injected).containsExactly("es6/util/arrayfromiterable");
}

Expand Down

0 comments on commit cebb785

Please sign in to comment.