Pull out Es6NormalizeShorthandProperties as the first ES6 pass in Tra…

…nspilationPasses.

Also adds regression tests for various failures that happen when shorthand properties aren't handled correctly.

#2685

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=174746346

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

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2017 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.javascript.jscomp;
+
+import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
+import com.google.javascript.rhino.IR;
+import com.google.javascript.rhino.Node;
+
+/**
+ * Normalizes shorthand object properties. This should be one of the first things done when
+ * transpiling from ES6 down to ES5, as it allows all the following checks and transpilations to not
+ * care about shorthand and destructured assignments.
+ */
+public final class Es6NormalizeShorthandProperties extends AbstractPostOrderCallback
+    implements HotSwapCompilerPass {
+  private final AbstractCompiler compiler;
+
+  public Es6NormalizeShorthandProperties(AbstractCompiler compiler) {
+    this.compiler = compiler;
+  }
+
+  @Override
+  public void process(Node externs, Node root) {
+    TranspilationPasses.processTranspile(compiler, externs, this);
+    TranspilationPasses.processTranspile(compiler, root, this);
+  }
+
+  @Override
+  public void hotSwapScript(Node scriptRoot, Node originalRoot) {
+    TranspilationPasses.hotSwapTranspile(compiler, scriptRoot, this);
+  }
+
+  @Override
+  public void visit(NodeTraversal t, Node n, Node parent) {
+    // Transform keys that look like {foo} into {foo: foo} by adding a NAME node
+    // with the same string as the only child of any child-less STRING_KEY node.
+    if (n.isStringKey() && !n.hasChildren()) {
+      n.addChildToFront(IR.name(n.getString()).useSourceInfoFrom(n));
+      compiler.reportChangeToEnclosingScope(n);
+    }
+  }
+}

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

@@ -41,10 +41,13 @@
 import java.util.Set;
 
 /**
- * Rewrite "let"s and "const"s as "var"s.
- * Rename block-scoped declarations and their references when necessary.
+ * Rewrite "let"s and "const"s as "var"s. Rename block-scoped declarations and their references when
+ * necessary.
  *
- * TODO(moz): Try to use MakeDeclaredNamesUnique
+ * <p>Note that this must run after Es6RewriteDestructuring, since it does not process destructuring
+ * let/const declarations at all.
+ *
+ * <p>TODO(moz): Try to use MakeDeclaredNamesUnique
  *
  * @author moz@google.com (Michael Zhou)
  */

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

@@ -118,9 +118,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
       case FOR_OF:
         visitForOf(t, n, parent);
         break;
-      case STRING_KEY:
-        visitStringKey(n);
-        break;
       case TAGGED_TEMPLATELIT:
         Es6TemplateLiterals.visitTaggedTemplateLiteral(t, n, addTypes);
         break;
@@ -148,19 +145,6 @@ private void visitMemberFunctionDefInObjectLit(Node n, Node parent) {
     compiler.reportChangeToEnclosingScope(stringKey);
   }
 
-  /**
-   * Converts extended object literal {a} to {a:a}.
-   */
-  // TODO(blickly): Separate this so it can be part of the normalization early transpilation passes.
-  private void visitStringKey(Node n) {
-    if (!n.hasChildren()) {
-      Node name = withType(IR.name(n.getString()), n.getTypeI());
-      name.useSourceInfoIfMissingFrom(n);
-      n.addChildToBack(name);
-      compiler.reportChangeToEnclosingScope(name);
-    }
-  }
-
   private void visitForOf(NodeTraversal t, Node node, Node parent) {
     Node variable = node.removeFirstChild();
     Node iterable = node.removeFirstChild();
@@ -367,4 +351,3 @@ private Node withUnknownType(Node n) {
     return withType(n, unknownType);
   }
 }
-

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

@@ -2704,11 +2704,19 @@ static boolean isSwitchCase(Node n) {
   }
 
   /**
-   * @return Whether the name is a reference to a variable, function or
-   *       function parameter (not a label or a empty function expression name).
+   * @return Whether the node is a reference to a variable, function or
+   *     function parameter (not a label or a empty function expression name).
+   *     This includes both NAME nodes and shorthand property STRING_KEYs.
    */
   static boolean isReferenceName(Node n) {
-    return n.isName() && !n.getString().isEmpty();
+    return (n.isName() && !n.getString().isEmpty()) || isShorthandProperty(n);
+  }
+
+  /**
+   * @return Whether the node is a shorthand property.
+   */
+  static boolean isShorthandProperty(Node n) {
+    return n.isStringKey() && !n.hasChildren();
   }
 
   /**

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

@@ -51,6 +51,7 @@ public static void addEs2016Passes(List<PassFactory> passes) {
    * transpile them, even if the output language is also ES6.
    */
   public static void addEs6EarlyPasses(List<PassFactory> passes) {
+    passes.add(es6NormalizeShorthandProperties);
     passes.add(es6SuperCheck);
     passes.add(es6ConvertSuper);
     passes.add(es6RenameVariablesInParamLists);
@@ -109,7 +110,7 @@ protected HotSwapCompilerPass create(AbstractCompiler compiler) {
 
         @Override
         protected FeatureSet featureSet() {
-          return FeatureSet.ES8_MODULES;
+          return ES8_MODULES;
         }
       };
 
@@ -139,6 +140,19 @@ protected FeatureSet featureSet() {
         }
       };
 
+  private static final HotSwapPassFactory es6NormalizeShorthandProperties =
+      new HotSwapPassFactory("es6NormalizeShorthandProperties") {
+        @Override
+        protected HotSwapCompilerPass create(AbstractCompiler compiler) {
+          return new Es6NormalizeShorthandProperties(compiler);
+        }
+
+        @Override
+        protected FeatureSet featureSet() {
+          return ES8;
+        }
+      };
+
   private static final PassFactory es6SuperCheck =
       new PassFactory("es6SuperCheck", true) {
         @Override

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

@@ -123,8 +123,11 @@ protected FeatureSet featureSet() {
   protected CompilerPass getProcessor(final Compiler compiler) {
     PhaseOptimizer optimizer = new PhaseOptimizer(compiler, null);
     optimizer.addOneTimePass(
-        makePassFactory("Es6RenameVariablesInParamLists",
-            new Es6RenameVariablesInParamLists(compiler)));
+        makePassFactory(
+            "Es6NormalizeShorthandProperties", new Es6NormalizeShorthandProperties(compiler)));
+    optimizer.addOneTimePass(
+        makePassFactory(
+            "Es6RenameVariablesInParamLists", new Es6RenameVariablesInParamLists(compiler)));
     optimizer.addOneTimePass(
         makePassFactory("es6ConvertSuper", new Es6ConvertSuper(compiler)));
     optimizer.addOneTimePass(makePassFactory("es6ExtractClasses", new Es6ExtractClasses(compiler)));
@@ -2776,6 +2779,53 @@ public void testUnicodeEscapes() {
     test("var str = `begin\\u{2026}end`", "var str = 'begin\\u2026end'");
   }
 
+  public void testObjectLiteralShorthand() {
+    test(
+        lines(
+            "function f() {",
+            "  var x = 1;",
+            "  if (a) {",
+            "    let x = 2;",
+            "    return {x};",
+            "  }",
+            "  return x;",
+            "}"),
+        lines(
+            "function f() {",
+            "  var x = 1;",
+            "  if (a) {",
+            "    var x$0 = 2;",
+            "    return {x: x$0};",
+            "  }",
+            "  return x;",
+            "}"));
+
+    test(
+        lines(
+            "function f(a) {",
+            "  var {x} = a;",
+            "  if (a) {",
+            "    let x = 2;",
+            "    return x;",
+            "  }",
+            "  return x;",
+            "}"),
+        lines(
+            "function f(a) {",
+            "  var {x: x} = a;",
+            "  if (a) {",
+            "    var x$0 = 2;",
+            "    return x$0;",
+            "  }",
+            "  return x;",
+            "}"));
+
+    // Note: if the inner `let` declaration is defined as a destructuring assignment
+    // then the test would fail because Es6RewriteBlockScopeDeclaration does not even
+    // look at destructuring declarations, expecting them to already have been
+    // rewritten, and this test does not include that pass.
+  }
+
   @Override
   protected Compiler createCompiler() {
     return new NoninjectingCompiler();

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

@@ -1089,7 +1089,9 @@ public void testDefaultParam() {
     assertEarlyReferenceError("function f(x=a) { var a; }");
     assertEarlyReferenceError("function f(x=a()) { function a() {} }");
     assertEarlyReferenceError("function f(x=[a]) { var a; }");
+    assertEarlyReferenceError("function f(x={a}) { let a; }");
     assertEarlyReferenceError("function f(x=y, y=2) {}");
+    assertEarlyReferenceError("function f(x={y}, y=2) {}");
     assertEarlyReferenceError("function f(x=x) {}");
     assertEarlyReferenceError("function f([x]=x) {}");
     // x within a function isn't referenced at the time the default value for x is evaluated.

test/com/google/javascript/jscomp/runtime_tests/let_const_rename_test.js

@@ -312,3 +312,12 @@ function testLabeledLoop() {
   }
   assertArrayEquals([0, 1, 2], [arr[0](), arr[1](), arr[2]()]);
 }
+
+function testRenamingDoesNotBreakObjectShorthand() {
+  let x = 2;
+  {
+    let x = 4;
+    assertObjectEquals({x: 4}, {x});
+  }
+  assertObjectEquals({x: 2}, {x});
+}