diff --git a/src/com/google/javascript/jscomp/EarlyEs6ToEs3Converter.java b/src/com/google/javascript/jscomp/EarlyEs6ToEs3Converter.java index 105b35d0063..dd5fb8ad3c1 100644 --- a/src/com/google/javascript/jscomp/EarlyEs6ToEs3Converter.java +++ b/src/com/google/javascript/jscomp/EarlyEs6ToEs3Converter.java @@ -16,6 +16,7 @@ package com.google.javascript.jscomp; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; import com.google.javascript.jscomp.CompilerOptions.LanguageMode; import com.google.javascript.jscomp.NodeTraversal.Callback; @@ -237,6 +238,7 @@ private void visitRestParam(NodeTraversal t, Node restParam, Node paramList) { name.setJSDocInfo(builder.build()); } + // TODO(b/74074478): Use a general utility method instead of an inlined loop. Node newArr = IR.var(IR.name(REST_PARAMS), IR.arraylit()); functionBody.addChildToFront(newArr.useSourceInfoIfMissingFromForTree(restParam)); Node init = IR.var(IR.name(REST_INDEX), IR.number(restIndex)); @@ -282,64 +284,217 @@ public void visit(Node n) { * new Function.prototype.bind.apply(F, [].concat($jscomp.arrayFromIterable(args))) */ private void visitArrayLitOrCallWithSpread(Node node, Node parent) { - checkArgument(node.isCall() || node.isArrayLit() || node.isNew()); + if (node.isArrayLit()) { + visitArrayLitWithSpread(node, parent); + } else if (node.isCall()) { + visitCallWithSpread(node, parent); + } else { + checkArgument(node.isNew(), node); + visitNewWithSpread(node, parent); + } + } + + /** + * Extracts child nodes from an array literal, call or new node that may contain spread operators + * into a list of nodes that may be concatenated with Array.concat() to get an array. + * + *

Example: [a, b, ...x, c, ...arguments] returns a list containing [ [a, b], + * $jscomp.arrayFromIterable(x), [c], $jscomp.arrayFromIterable(arguments) ] + * + *

IMPORTANT: Call and New nodes must have the first, callee, child removed already. + * + *

Note that all elements of the returned list will be one of: + * + *

+ * + * TODO(bradfordcsmith): When this pass moves after type checking, we can use type information to + * avoid unnecessary calls to $jscomp.arrayFromIterable(). + */ + private List extractSpreadGroups(Node parentNode) { + checkArgument(parentNode.isCall() || parentNode.isArrayLit() || parentNode.isNew()); List groups = new ArrayList<>(); Node currGroup = null; - Node callee = node.isArrayLit() ? null : node.removeFirstChild(); - Node currElement = node.removeFirstChild(); + Node currElement = parentNode.removeFirstChild(); while (currElement != null) { if (currElement.isSpread()) { - if (currGroup != null) { - groups.add(currGroup); - currGroup = null; + Node spreadExpression = currElement.removeFirstChild(); + if (spreadExpression.isArrayLit()) { + // We can expand an array literal spread in place. + if (currGroup == null) { + // [...[spread, contents], a, b] + // we can use this array lit itself as a group and append following elements to it + currGroup = spreadExpression; + } else { + // [ a, b, ...[spread, contents], c] + // Just add contents of this array lit to the group we were already collecting. + currGroup.addChildrenToBack(spreadExpression.removeChildren()); + } + } else { + // We need to treat the spread expression as a separate group + if (currGroup != null) { + // finish off and add the group we were collecting before + groups.add(currGroup); + currGroup = null; + } + + groups.add(Es6ToEs3Util.arrayFromIterable(compiler, spreadExpression)); } - groups.add(Es6ToEs3Util.arrayFromIterable(compiler, currElement.removeFirstChild())); } else { if (currGroup == null) { currGroup = IR.arraylit(); } currGroup.addChildToBack(currElement); } - currElement = node.removeFirstChild(); + currElement = parentNode.removeFirstChild(); } if (currGroup != null) { groups.add(currGroup); } - Node result = null; - Node firstGroup = node.isNew() ? IR.arraylit(IR.nullNode()) : IR.arraylit(); + return groups; + } + + /** + * Processes array literals containing spreads. + * + *

Example: + * + * 


+   * [1, 2, ...x, 4, 5] => [1, 2].concat($jscomp.arrayFromIterable(x), [4, 5])
+   *
+ */ + private void visitArrayLitWithSpread(Node node, Node parent) { + checkArgument(node.isArrayLit()); + List groups = extractSpreadGroups(node); + Node baseArrayLit; + if (groups.get(0).isArrayLit()) { + baseArrayLit = groups.remove(0); + } else { + baseArrayLit = IR.arraylit(); + // [].concat(g0, g1, g2, ..., gn) + } Node joinedGroups = - IR.call(IR.getprop(firstGroup, IR.string("concat")), groups.toArray(new Node[0])); - if (node.isArrayLit()) { - result = joinedGroups; - } else if (node.isCall()) { - if (NodeUtil.mayHaveSideEffects(callee) && callee.isGetProp()) { - Node statement = node; - while (!NodeUtil.isStatement(statement)) { - statement = statement.getParent(); - } - Node freshVar = IR.name(FRESH_SPREAD_VAR + compiler.getUniqueNameIdSupplier().get()); - Node n = IR.var(freshVar.cloneTree()); - n.useSourceInfoIfMissingFromForTree(statement); - statement.getParent().addChildBefore(n, statement); - callee.addChildToFront(IR.assign(freshVar.cloneTree(), callee.removeFirstChild())); - result = IR.call( - IR.getprop(callee, IR.string("apply")), - freshVar, - joinedGroups); + groups.isEmpty() + ? baseArrayLit + : IR.call(IR.getprop(baseArrayLit, IR.string("concat")), groups.toArray(new Node[0])); + joinedGroups.useSourceInfoIfMissingFromForTree(node); + parent.replaceChild(node, joinedGroups); + compiler.reportChangeToEnclosingScope(joinedGroups); + } + + /** + * Processes calls containing spreads. + * + *

Examples: + * + * 


+   * f(...arr) => f.apply(null, $jscomp.arrayFromIterable(arr))
+   * f(a, ...arr) => f.apply(null, [a].concat($jscomp.arrayFromIterable(arr)))
+   * f(...arr, b) => f.apply(null, [].concat($jscomp.arrayFromIterable(arr), [b]))
+   *
+ */ + private void visitCallWithSpread(Node node, Node parent) { + checkArgument(node.isCall()); + // must remove callee before extracting argument groups + Node callee = node.removeFirstChild(); + Node joinedGroups; + if (node.hasOneChild() && isSpreadOfArguments(node.getOnlyChild())) { + // Check for special case of + // `foo(...arguments)` and pass `arguments` directly to `foo.apply(null, arguments)`. + // We want to avoid calling $jscomp.arrayFromIterable(arguments) for this case, + // because it can have side effects, which prevents code removal. + // TODO(b/74074478): generalize this to avoid ever calling $jscomp.arrayFromIterable() for + // `arguments`. + joinedGroups = node.removeFirstChild().removeFirstChild(); + } else { + List groups = extractSpreadGroups(node); + checkState(!groups.isEmpty()); + if (groups.size() == 1) { + // single group can just be passed to apply() as-is + // It could be `arguments`, an array literal, or $jscomp.arrayFromIterable(someExpression). + joinedGroups = groups.remove(0); } else { - Node context = callee.isGetProp() ? callee.getFirstChild().cloneTree() : IR.nullNode(); - result = IR.call(IR.getprop(callee, IR.string("apply")), context, joinedGroups); + // If the first group is an array literal, we can just use that for concatenation, + // otherwise use an empty array literal. + Node baseArrayLit = groups.get(0).isArrayLit() ? groups.remove(0) : IR.arraylit(); + joinedGroups = + groups.isEmpty() + ? baseArrayLit + : IR.call( + IR.getprop(baseArrayLit, IR.string("concat")), groups.toArray(new Node[0])); } - } else { - if (compiler.getOptions().getLanguageOut() == LanguageMode.ECMASCRIPT3) { - // TODO(tbreisacher): Support this in ES3 too by not relying on Function.bind. - Es6ToEs3Util.cannotConvert( - compiler, node, "\"...\" passed to a constructor (consider using --language_out=ES5)"); + } + + Node result = null; + if (NodeUtil.mayHaveSideEffects(callee) && callee.isGetProp()) { + // foo().method(...[a, b, c]) + // must convert to + // var freshVar; + // (freshVar = foo()).method.apply(freshVar, [a, b, c]) + Node statement = node; + while (!NodeUtil.isStatement(statement)) { + statement = statement.getParent(); } - Node bindApply = NodeUtil.newQName(compiler, - "Function.prototype.bind.apply"); - result = IR.newNode(IR.call(bindApply, callee, joinedGroups)); + Node freshVar = IR.name(FRESH_SPREAD_VAR + compiler.getUniqueNameIdSupplier().get()); + Node n = IR.var(freshVar.cloneTree()); + n.useSourceInfoIfMissingFromForTree(statement); + statement.getParent().addChildBefore(n, statement); + callee.addChildToFront(IR.assign(freshVar.cloneTree(), callee.removeFirstChild())); + result = IR.call(IR.getprop(callee, IR.string("apply")), freshVar, joinedGroups); + } else { + // foo.method(...[a, b, c]) -> foo.method.apply(foo, [a, b, c] + // or + // foo(...[a, b, c]) -> foo.apply(null, [a, b, c]) + Node context = callee.isGetProp() ? callee.getFirstChild().cloneTree() : IR.nullNode(); + result = IR.call(IR.getprop(callee, IR.string("apply")), context, joinedGroups); + } + result.useSourceInfoIfMissingFromForTree(node); + parent.replaceChild(node, result); + compiler.reportChangeToEnclosingScope(result); + } + + private boolean isSpreadOfArguments(Node n) { + return n.isSpread() && n.getOnlyChild().matchesQualifiedName("arguments"); + } + + /** + * Processes new calls containing spreads. + * + *

Example: + * + * 


+   * new F(...args) =>
+   *     new Function.prototype.bind.apply(F, [].concat($jscomp.arrayFromIterable(args)))
+   *
+ */ + private void visitNewWithSpread(Node node, Node parent) { + checkArgument(node.isNew()); + // must remove callee before extracting argument groups + Node callee = node.removeFirstChild(); + List groups = extractSpreadGroups(node); + // We need to generate + // new (Function.prototype.bind.apply(callee, [null].concat(other, args))(); + // null stands in for the 'this' arg to bind + Node baseArrayLit; + if (groups.get(0).isArrayLit()) { + baseArrayLit = groups.remove(0); + } else { + baseArrayLit = IR.arraylit(); + } + baseArrayLit.addChildToFront(IR.nullNode()); + Node joinedGroups = + groups.isEmpty() + ? baseArrayLit + : IR.call(IR.getprop(baseArrayLit, IR.string("concat")), groups.toArray(new Node[0])); + if (compiler.getOptions().getLanguageOut() == LanguageMode.ECMASCRIPT3) { + // TODO(tbreisacher): Support this in ES3 too by not relying on Function.bind. + Es6ToEs3Util.cannotConvert( + compiler, node, "\"...\" passed to a constructor (consider using --language_out=ES5)"); } + Node bindApply = NodeUtil.newQName(compiler, "Function.prototype.bind.apply"); + Node result = IR.newNode(IR.call(bindApply, callee, joinedGroups)); result.useSourceInfoIfMissingFromForTree(node); parent.replaceChild(node, result); compiler.reportChangeToEnclosingScope(result); diff --git a/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java b/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java index 75a5d9a1040..2264ece2e89 100644 --- a/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java +++ b/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java @@ -150,6 +150,46 @@ protected int getNumRepetitions() { return 1; } + public void testSpreadOfArrayLiteral() { + test("[...[1, 2], 3, ...[4, 5], 6, ...[7, 8]]", "[1, 2, 3, 4, 5, 6, 7, 8]"); + test( + "function use() {} use(...[1, 2], 3, ...[4, 5], 6, ...[7, 8]);", + "function use() {} use.apply(null, [1, 2, 3, 4, 5, 6, 7, 8])"); + } + + public void testExplicitSuperSpread() { + test( + lines( + "class A {", + " constructor(a) {", + " this.p = a;", + " }", + "}", + "", + "class B extends A {", + " constructor(a) {", + " super(...arguments);", + " }", + "}", + ""), + lines( + "/** @struct @constructor */", + "var A = function(a) {", + " this.p=a", + "};", + "", + "/**", + " * @struct", + " * @constructor", + " * @extends {A}", + " */", + "var B = function(a){", + " A.apply(this, arguments)", + "};", + "$jscomp.inherits(B,A)", + "")); + } + public void testObjectLiteralStringKeysWithNoValue() { test("var x = {a, b};", "var x = {a: a, b: b};"); assertThat(getLastCompiler().injected).isEmpty(); @@ -1471,7 +1511,7 @@ public void testSuperSpread() { "var D = function(){};", "/** @constructor @struct @extends {D} */", "var C=function(args) {", - " D.apply(this, [].concat($jscomp.arrayFromIterable(args)));", + " D.apply(this, $jscomp.arrayFromIterable(args));", "};", "$jscomp.inherits(C,D);")); assertThat(getLastCompiler().injected) @@ -2397,19 +2437,21 @@ public void testForOfJSDoc() { } public void testSpreadArray() { - test("var arr = [1, 2, ...mid, 4, 5];", - "var arr = [].concat([1, 2], $jscomp.arrayFromIterable(mid), [4, 5]);"); + test( + "var arr = [1, 2, ...mid, 4, 5];", + "var arr = [1, 2].concat($jscomp.arrayFromIterable(mid), [4, 5]);"); assertThat(getLastCompiler().injected).containsExactly("es6/util/arrayfromiterable"); - test("var arr = [1, 2, ...mid(), 4, 5];", - "var arr = [].concat([1, 2], $jscomp.arrayFromIterable(mid()), [4, 5]);"); - test("var arr = [1, 2, ...mid, ...mid2(), 4, 5];", - "var arr = [].concat([1, 2], $jscomp.arrayFromIterable(mid)," - + " $jscomp.arrayFromIterable(mid2()), [4, 5]);"); - test("var arr = [...mid()];", - "var arr = [].concat($jscomp.arrayFromIterable(mid()));"); - test("f(1, [2, ...mid, 4], 5);", - "f(1, [].concat([2], $jscomp.arrayFromIterable(mid), [4]), 5);"); + test( + "var arr = [1, 2, ...mid(), 4, 5];", + "var arr = [1, 2].concat($jscomp.arrayFromIterable(mid()), [4, 5]);"); + test( + "var arr = [1, 2, ...mid, ...mid2(), 4, 5];", + lines( + "var arr = [1,2].concat(", + " $jscomp.arrayFromIterable(mid), $jscomp.arrayFromIterable(mid2()), [4, 5]);")); + test("var arr = [...mid()];", "var arr = [].concat($jscomp.arrayFromIterable(mid()));"); + test("f(1, [2, ...mid, 4], 5);", "f(1, [2].concat($jscomp.arrayFromIterable(mid), [4]), 5);"); test( "function f() { return [...arguments]; };", lines( @@ -2458,39 +2500,47 @@ public void testForOfOnNonIterable() { } public void testSpreadCall() { - test("f(...arr);", "f.apply(null, [].concat($jscomp.arrayFromIterable(arr)));"); - test("f(0, ...g());", "f.apply(null, [].concat([0], $jscomp.arrayFromIterable(g())));"); + test("f(...arr);", "f.apply(null, $jscomp.arrayFromIterable(arr));"); + test("f(0, ...g());", "f.apply(null, [0].concat($jscomp.arrayFromIterable(g())));"); test("f(...arr, 1);", "f.apply(null, [].concat($jscomp.arrayFromIterable(arr), [1]));"); - test("f(0, ...g(), 2);", "f.apply(null, [].concat([0], $jscomp.arrayFromIterable(g()), [2]));"); - test("obj.m(...arr);", "obj.m.apply(obj, [].concat($jscomp.arrayFromIterable(arr)));"); - test("x.y.z.m(...arr);", "x.y.z.m.apply(x.y.z, [].concat($jscomp.arrayFromIterable(arr)));"); - test("f(a, ...b, c, ...d, e);", - "f.apply(null, [].concat([a], $jscomp.arrayFromIterable(b)," - + " [c], $jscomp.arrayFromIterable(d), [e]));"); - - test("Factory.create().m(...arr);", - lines( - "var $jscomp$spread$args0;", - "($jscomp$spread$args0 = Factory.create()).m.apply(" - + "$jscomp$spread$args0, [].concat($jscomp.arrayFromIterable(arr)));" - )); + test("f(0, ...g(), 2);", "f.apply(null, [0].concat($jscomp.arrayFromIterable(g()), [2]));"); + test("obj.m(...arr);", "obj.m.apply(obj, $jscomp.arrayFromIterable(arr));"); + test("x.y.z.m(...arr);", "x.y.z.m.apply(x.y.z, $jscomp.arrayFromIterable(arr));"); + test( + "f(a, ...b, c, ...d, e);", + lines( + "f.apply(", + " null,", + " [a].concat(", + " $jscomp.arrayFromIterable(b),", + " [c],", + " $jscomp.arrayFromIterable(d),", + " [e]));")); + + test( + "Factory.create().m(...arr);", + lines( + "var $jscomp$spread$args0;", + "($jscomp$spread$args0 = Factory.create()).m.apply(", + " $jscomp$spread$args0, $jscomp.arrayFromIterable(arr));")); - test("var x = b ? Factory.create().m(...arr) : null;", + test( + "var x = b ? Factory.create().m(...arr) : null;", lines( "var $jscomp$spread$args0;", "var x = b ? ($jscomp$spread$args0 = Factory.create()).m.apply($jscomp$spread$args0, ", - " [].concat($jscomp.arrayFromIterable(arr))) : null;")); + " $jscomp.arrayFromIterable(arr)) : null;")); - test("getF()(...args);", "getF().apply(null, [].concat($jscomp.arrayFromIterable(args)));"); + test("getF()(...args);", "getF().apply(null, $jscomp.arrayFromIterable(args));"); test( "F.c().m(...a); G.d().n(...b);", lines( "var $jscomp$spread$args0;", "($jscomp$spread$args0 = F.c()).m.apply($jscomp$spread$args0,", - " [].concat($jscomp.arrayFromIterable(a)));", + " $jscomp.arrayFromIterable(a));", "var $jscomp$spread$args1;", "($jscomp$spread$args1 = G.d()).n.apply($jscomp$spread$args1,", - " [].concat($jscomp.arrayFromIterable(b)));")); + " $jscomp.arrayFromIterable(b));")); this.mode = TypeInferenceMode.BOTH; @@ -2526,7 +2576,7 @@ public void testSpreadCall() { "var arr = [1,2]", "var $jscomp$spread$args0;", "($jscomp$spread$args0 = Factory.create()).m.apply(", - " $jscomp$spread$args0, [].concat($jscomp.arrayFromIterable(arr)));")); + " $jscomp$spread$args0, $jscomp.arrayFromIterable(arr));")); } public void testSpreadNew() { diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java index be43ea9fbc8..20bf5d67c5f 100644 --- a/test/com/google/javascript/jscomp/IntegrationTest.java +++ b/test/com/google/javascript/jscomp/IntegrationTest.java @@ -3312,7 +3312,7 @@ public void testIssue724() { " getType.toString.apply(functionToCheck) === '[object Function]';", "};"); String result = - "isFunction = function(a){ var b={}; return a && '[object Function]' === b.b.a(a); }"; + "isFunction = function(a){ var b={}; return a && '[object Function]' === b.a.apply(a); }"; test(options, code, result); } @@ -4875,6 +4875,40 @@ public void testPureFunctionIdentifierWorksWithMultipleNames() { "window._customData.foo='bar';"); } + public void testSpreadArgumentsPassedToSuperDoesNotPreventRemoval() { + CompilerOptions options = createCompilerOptions(); + options.setLanguageIn(LanguageMode.ECMASCRIPT_2017); + options.setLanguageOut(LanguageMode.ECMASCRIPT5); + CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options); + + // Create a noninjecting compiler avoid comparing all the polyfill code. + useNoninjectingCompiler = true; + + // include externs definitions for the stuff that would have been injected + ImmutableList.Builder externsList = ImmutableList.builder(); + externsList.addAll(externs); + externsList.add(SourceFile.fromCode("extraExterns", "var $jscomp = {};")); + externs = externsList.build(); + + test( + options, + lines( + "", // preserve newlines + "class A {", + " constructor(a, b) {", + " this.a = a;", + " this.b = b;", + " }", + "}", + "class B extends A {", + " constructor() {", + " super(...arguments);", + " }", + "}", + "var b = new B();"), + ""); + } + public void testUnnecessaryBackslashInStringLiteral() { CompilerOptions options = createCompilerOptions(); test(options, diff --git a/test/com/google/javascript/jscomp/IntegrationTestCase.java b/test/com/google/javascript/jscomp/IntegrationTestCase.java index eba76e9a6e3..307f2b38d72 100644 --- a/test/com/google/javascript/jscomp/IntegrationTestCase.java +++ b/test/com/google/javascript/jscomp/IntegrationTestCase.java @@ -201,6 +201,11 @@ abstract class IntegrationTestCase extends TestCase { " * @constructor", " */", "function Function(var_args) {}", + "/**", + " * @param {*} context", + " * @param {!IArrayLike} args", + " */", + "Function.prototype.apply = function (context, args) {};", "/** @param {...*} var_args */", "Function.prototype.call = function (var_args) {};", "",