From a4c501ca2b436b1d5c273d0be9c00d4c5d80e20f Mon Sep 17 00:00:00 2001 From: yitingwang Date: Fri, 21 Jul 2017 17:16:54 -0700 Subject: [PATCH] Support computed props in ClosureOptimizePrimitives pass. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=162803643 --- .../jscomp/ClosureOptimizePrimitives.java | 65 ++++++++++++------- .../javascript/jscomp/DefaultPassConfig.java | 3 +- .../jscomp/ClosureOptimizePrimitivesTest.java | 63 +++++++++++++++--- 3 files changed, 98 insertions(+), 33 deletions(-) diff --git a/src/com/google/javascript/jscomp/ClosureOptimizePrimitives.java b/src/com/google/javascript/jscomp/ClosureOptimizePrimitives.java index 96088c40955..15b941461d3 100644 --- a/src/com/google/javascript/jscomp/ClosureOptimizePrimitives.java +++ b/src/com/google/javascript/jscomp/ClosureOptimizePrimitives.java @@ -48,6 +48,9 @@ final class ClosureOptimizePrimitives implements CompilerPass { /** Whether property renaming is enabled */ private final boolean propertyRenamingEnabled; + /** Whether we can use Es6 syntax */ + private final boolean canUseEs6Syntax; + /** * Identifies all calls to closure primitive functions */ @@ -74,9 +77,11 @@ public void visit(NodeTraversal t, Node n, Node parent) { } /** @param compiler The AbstractCompiler */ - ClosureOptimizePrimitives(AbstractCompiler compiler, boolean propertyRenamingEnabled) { + ClosureOptimizePrimitives( + AbstractCompiler compiler, boolean propertyRenamingEnabled, boolean canUseEs6Syntax) { this.compiler = compiler; this.propertyRenamingEnabled = propertyRenamingEnabled; + this.canUseEs6Syntax = canUseEs6Syntax; } @Override @@ -101,13 +106,7 @@ private void processObjectCreateCall(Node callNode) { callNode.removeChild(keyNode); callNode.removeChild(valueNode); - if (!keyNode.isString()) { - keyNode = IR.string(NodeUtil.getStringValue(keyNode)) - .srcref(keyNode); - } - keyNode.setToken(Token.STRING_KEY); - keyNode.setQuotedString(); - objNode.addChildToBack(IR.propdef(keyNode, valueNode)); + addKeyValueToObjLit(objNode, keyNode, valueNode); } callNode.replaceWith(objNode); compiler.reportChangeToEnclosingScope(objNode); @@ -148,11 +147,10 @@ private void processRenamePropertyCall(Node callNode) { * Returns whether the given call to goog.object.create can be converted to an * object literal. */ - private static boolean canOptimizeObjectCreate(Node firstParam) { + private boolean canOptimizeObjectCreate(Node firstParam) { Node curParam = firstParam; while (curParam != null) { - // All keys must be strings or numbers. - if (!curParam.isString() && !curParam.isNumber()) { + if (!isOptimizableKey(curParam)) { return false; } curParam = curParam.getNext(); @@ -181,13 +179,7 @@ private void processObjectCreateSetCall(Node callNode) { curParam = curParam.getNext(); callNode.removeChild(keyNode); - if (!keyNode.isString()) { - keyNode = IR.string(NodeUtil.getStringValue(keyNode)) - .srcref(keyNode); - } - keyNode.setToken(Token.STRING_KEY); - keyNode.setQuotedString(); - objNode.addChildToBack(IR.propdef(keyNode, valueNode)); + addKeyValueToObjLit(objNode, keyNode, valueNode); } callNode.replaceWith(objNode); compiler.reportChangeToEnclosingScope(objNode); @@ -202,20 +194,45 @@ private boolean canOptimizeObjectCreateSet(Node firstParam) { Set keys = new HashSet<>(); while (curParam != null) { // All keys must be strings or numbers, otherwise we can't optimize the call. - if (!curParam.isString() && !curParam.isNumber()) { + if (!isOptimizableKey(curParam)) { return false; } - String key = - curParam.isString() ? curParam.getString() : numberToString(curParam.getDouble()); - if (!keys.add(key)) { - compiler.report(JSError.make(firstParam.getPrevious(), DUPLICATE_SET_MEMBER, key)); - return false; + if (curParam.isString() || curParam.isNumber()) { + String key = + curParam.isString() ? curParam.getString() : numberToString(curParam.getDouble()); + if (!keys.add(key)) { + compiler.report(JSError.make(firstParam.getPrevious(), DUPLICATE_SET_MEMBER, key)); + return false; + } } curParam = curParam.getNext(); } return true; } + private void addKeyValueToObjLit(Node objNode, Node keyNode, Node valueNode) { + if (keyNode.isNumber() || keyNode.isString()) { + if (keyNode.isNumber()) { + keyNode = IR.string(numberToString(keyNode.getDouble())) + .srcref(keyNode); + } + keyNode.setToken(Token.STRING_KEY); + keyNode.setQuotedString(); + objNode.addChildToBack(IR.propdef(keyNode, valueNode)); + } else { + objNode.addChildToBack(IR.computedProp(keyNode, valueNode).srcref(keyNode)); + } + } + + private boolean isOptimizableKey(Node curParam) { + if (this.canUseEs6Syntax) { + return !NodeUtil.isStatement(curParam); + } else { + // Not ES6, all keys must be strings or numbers. + return curParam.isString() || curParam.isNumber(); + } + } + /** * Converts the given node to string if it is safe to do so. */ diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index 1ce99c12804..a9f3dd55539 100644 --- a/src/com/google/javascript/jscomp/DefaultPassConfig.java +++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java @@ -2293,7 +2293,8 @@ protected CompilerPass create(final AbstractCompiler compiler) { protected CompilerPass create(final AbstractCompiler compiler) { return new ClosureOptimizePrimitives( compiler, - compiler.getOptions().propertyRenaming == PropertyRenamingPolicy.ALL_UNQUOTED); + compiler.getOptions().propertyRenaming == PropertyRenamingPolicy.ALL_UNQUOTED, + compiler.getOptions().getLanguageOut().toFeatureSet().contains(FeatureSet.ES6)); } }; diff --git a/test/com/google/javascript/jscomp/ClosureOptimizePrimitivesTest.java b/test/com/google/javascript/jscomp/ClosureOptimizePrimitivesTest.java index 0ec9bcc54b5..7efd12f678a 100644 --- a/test/com/google/javascript/jscomp/ClosureOptimizePrimitivesTest.java +++ b/test/com/google/javascript/jscomp/ClosureOptimizePrimitivesTest.java @@ -18,6 +18,8 @@ import static com.google.javascript.jscomp.ClosureOptimizePrimitives.DUPLICATE_SET_MEMBER; +import com.google.javascript.jscomp.CompilerOptions.LanguageMode; + /** * Tests for {@link ClosureOptimizePrimitives}. * @@ -26,13 +28,16 @@ public final class ClosureOptimizePrimitivesTest extends CompilerTestCase { private boolean propertyRenamingEnabled = true; + private boolean canUseEs6Syntax = true; @Override protected CompilerPass getProcessor(final Compiler compiler) { - return new ClosureOptimizePrimitives(compiler, propertyRenamingEnabled); + return new ClosureOptimizePrimitives(compiler, propertyRenamingEnabled, canUseEs6Syntax); } - public void testObjectCreateNonConstKey() { - testSame("goog.object.create('a',1,2,3,foo,bar);"); + @Override + protected void setUp() throws Exception { + super.setUp(); + setAcceptedLanguage(LanguageMode.ECMASCRIPT_2017); } public void testObjectCreateOddParams() { @@ -61,8 +66,33 @@ public void testObjectCreate5() { test("goog.object.create('a',2).toString()", "({'a':2}).toString()"); } - public void testObjectCreateSetNonConstKey() { - testSame("goog.object.createSet('a',1,2,3,foo,bar);"); + public void testObjectCreateNonConstKey1() { + test("var a = goog.object.create('a', 1, 2, 3, foo, bar);", + "var a = {'a': 1, 2: 3, [foo]: bar};"); + } + + public void testObjectCreateNonConstKey2() { + test("var a = goog.object.create('a' + 'b', 0);", "var a = {['a' + 'b']: 0};"); + } + + public void testObjectCreateNonConstKey3() { + test("var a = goog$object$create(i++,goog$object$create(foo(), 'd'))", + "var a = {[i++]: {[foo()]: 'd'}};"); + } + + public void testObjectCreateNonConstKey4() { + test("alert(goog.object.create(a = 1, 2).toString())", + "alert({[a = 1]: 2}.toString())"); + } + + public void testObjectCreateNonConstKey5() { + test("goog.object.create(function foo() {}, 2).toString()", + "({[function foo() {}]: 2}).toString()"); + } + + public void testObjectCreateNonConstKeyNotEs6() { + canUseEs6Syntax = false; + testSame("var a = goog.object.create(foo, bar)"); } public void testObjectCreateSet1() { @@ -88,6 +118,26 @@ public void testObjectCreateSet_duplicate() { testWarning("goog.object.createSet(4, '4')", DUPLICATE_SET_MEMBER); } + public void testObjectCreateSetNonConstKey1() { + test("var a = goog.object.createSet(foo, bar);", + "var a = {[foo]: true, [bar]: true};"); + } + + public void testObjectCreateSetNonConstKey2() { + test("alert(goog$object$createSet(a = 1, 2).toString())", + "alert({[a = 1]: true, 2: true}.toString())"); + } + + public void testObjectCreateSetNonConstKey3() { + test("goog.object.createSet(() => {}).toString()", + "({[() => {}]: true}).toString()"); + } + + public void testObjectCreateSetNonConstKeyNotEs6() { + canUseEs6Syntax = false; + testSame("var a = goog.object.createSet(foo, bar);"); + } + public void testDomTagName() { testSame("goog.dom.TagName.A = 'A';"); testSame("goog.dom.TagName.prototype.toString = function() { return 'a'; };"); @@ -179,8 +229,5 @@ public void testEs6Compatibility() { "async function foo() {", " return await {'a': 2};", "}")); - - // TODO(yitingwang): handle optimizations of computed props. - testSame("var obj = goog.object.create(a, b);"); } }