Roll forward 179255046: Normalize shorthand object properties.

NEW: updated transpile.js to be in sync with the final state of the CL, rather than an intermediate state.

Automated g4 rollback of changelist 179430808.

*** Reason for rollback ***

Roll forward

*** Original change description ***

Automated g4 rollback of changelist 179255046.

*** Reason for rollback ***

Breaks transpilation.

*** Original change description ***

Normalize shorthand object properties.

After this change, the AST no longer has special forms for shorthand object properties (i.e. {x}), neither in object literals nor in object destructuring patterns.  Import specs are unchanged.

All handling of shorthand properties has been pushed to the extremities of parsing (which returns the same AST as {x:x}, except the STRING_KEY node has an IS_SHORTHAND_PROPERTY annotation) and code generation (which will re-collapse the property if the annotation is present and the names are the same).

This greatly simplifies a number of checks, transpilations, and optimizations, which now no longer need to deal with a different form of tree.

***

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

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

@@ -557,10 +557,16 @@ private void validateEnum(Node n) {
   private void validateEnumMembers(Node n) {
     validateNodeType(Token.ENUM_MEMBERS, n);
     for (Node c = n.getFirstChild(); c != null; c = c.getNext()) {
-      validateObjectLitStringKey(c);
+      validateEnumStringKey(c);
     }
   }
 
+  private void validateEnumStringKey(Node n) {
+    validateNodeType(Token.STRING_KEY, n);
+    validateObjectLiteralKeyName(n);
+    validateChildCount(n, 0);
+  }
+
   /**
    * In a class declaration, unlike a class expression,
    * the class name is required.
@@ -955,9 +961,6 @@ private void validateObjectPattern(Token type, Node n) {
         case OBJECT_PATTERN:
           validateObjectPattern(type, c);
           break;
-        case DEFAULT_VALUE:
-          validateDefaultValue(type, c);
-          break;
         case REST:
           validateRest(type, c);
           break;
@@ -1318,27 +1321,22 @@ private void validateObjectLitStringKey(Node n) {
     validateNodeType(Token.STRING_KEY, n);
     validateObjectLiteralKeyName(n);
 
-    validateChildCountIn(n, 0, 1);
-
-    if (n.hasOneChild()) {
-      validateExpression(n.getFirstChild());
-    }
+    validateChildCount(n, 1);
+    validateExpression(n.getFirstChild());
   }
 
   private void validateObjectPatternStringKey(Token type, Node n) {
     validateNodeType(Token.STRING_KEY, n);
     validateObjectLiteralKeyName(n);
-    validateChildCountIn(n, 0, 1);
+    validateChildCount(n, 1);
 
-    if (n.hasOneChild()) {
-      Node c = n.getFirstChild();
-      switch (c.getToken()) {
-        case DEFAULT_VALUE:
-          validateDefaultValue(type, c);
-          break;
-        default:
-          validateLHS(type, c);
-      }
+    Node c = n.getFirstChild();
+    switch (c.getToken()) {
+      case DEFAULT_VALUE:
+        validateDefaultValue(type, c);
+        break;
+      default:
+        validateLHS(type, c);
     }
   }
 

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

@@ -79,11 +79,6 @@ class ClosureRewriteClass extends AbstractPostOrderCallback
           "JSC_GOOG_CLASS_ES6_COMPUTED_PROP_NAMES_NOT_SUPPORTED",
           "Computed property names not supported in goog.defineClass.");
 
-  static final DiagnosticType GOOG_CLASS_ES6_SHORTHAND_ASSIGNMENT_NOT_SUPPORTED =
-      DiagnosticType.error(
-          "JSC_GOOG_CLASS_ES6_SHORTHAND_ASSIGNMENT_NOT_SUPPORTED",
-          "Shorthand assignments not supported in goog.defineClass.");
-
   static final DiagnosticType GOOG_CLASS_ES6_ARROW_FUNCTION_NOT_SUPPORTED =
       DiagnosticType.error(
           "JSC_GOOG_CLASS_ES6_ARROW_FUNCTION_NOT_SUPPORTED",
@@ -351,12 +346,6 @@ private boolean validateObjLit(Node objlit, Node parent) {
             GOOG_CLASS_ES6_COMPUTED_PROP_NAMES_NOT_SUPPORTED));
         return false;
       }
-      if (key.isStringKey() && !key.hasChildren()) {
-        // report using shorthand assignment
-        compiler.report(JSError.make(objlit,
-            GOOG_CLASS_ES6_SHORTHAND_ASSIGNMENT_NOT_SUPPORTED));
-        return false;
-      }
       if (key.isStringKey()
           && key.hasChildren()
           && key.getFirstChild().isArrowFunction()){

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

@@ -1625,14 +1625,23 @@ void addList(Node firstInList, boolean isArrayOrFunctionArgument,
 
   void addStringKey(Node n) {
     String key = n.getString();
-    // Object literal property names don't have to be quoted if they
-    // are not JavaScript keywords
-    if (!n.isQuotedString()
-        && !(quoteKeywordProperties && TokenStream.isKeyword(key))
-        && TokenStream.isJSIdentifier(key)
-        // do not encode literally any non-literal characters that
-        // were Unicode escaped.
-        && NodeUtil.isLatin(key)) {
+    // Object literal property names don't have to be quoted if they are not JavaScript keywords.
+    boolean mustBeQuoted =
+        n.isQuotedString()
+        || (quoteKeywordProperties && TokenStream.isKeyword(key))
+        || !TokenStream.isJSIdentifier(key)
+        // do not encode literally any non-literal characters that were Unicode escaped.
+        || !NodeUtil.isLatin(key);
+    if (!mustBeQuoted) {
+      // Check if the property is eligible to be printed as shorthand.
+      if (n.isShorthandProperty()) {
+        Node child = n.getFirstChild();
+        if (child.matchesQualifiedName(key)
+            || (child.isDefaultValue() && child.getFirstChild().matchesQualifiedName(key))) {
+          add(child);
+          return;
+        }
+      }
       add(key);
     } else {
       // Determine if the string is a simple number.
@@ -1644,6 +1653,9 @@ void addStringKey(Node n) {
       }
     }
     if (n.hasChildren()) {
+      // NOTE: the only time a STRING_KEY node does *not* have children is when it's
+      // inside a TypeScript enum.  We should change these to their own ENUM_KEY token
+      // so that the bifurcating logic can be removed from STRING_KEY.
       add(":");
       addExpr(n.getFirstChild(), 1, Context.OTHER);
     }

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

@@ -18,7 +18,6 @@
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
 import com.google.javascript.jscomp.parsing.parser.FeatureSet;
 import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
-import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
 
 /**
@@ -49,11 +48,10 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
 
   @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);
+    // Remove all IS_SHORTHAND_PROPERTY annotations on all nodes so that the code printer
+    // does not print it as shorthand.
+    if (n.isStringKey()) {
+      n.setShorthandProperty(false);
     }
   }
 }

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

@@ -3471,11 +3471,10 @@ private static boolean isLhsByDestructuringHelper(Node n) {
     }
 
     if (parent.isObjectPattern()) {
-      // STRING_KEY children of object patterns are only LHS nodes if they have no children,
-      // meaning that they represent the object pattern shorthand (e.g. "var {a} = ...").
-      // If n is not a STRING_KEY, it is an OBJECT_PATTERN, DEFAULT_VALUE, or
-      // COMPUTED_PROP and contains a LHS node.
-      return !(n.isStringKey() && n.hasChildren());
+      // STRING_KEY children of object patterns are not LHS nodes, since shorthand (e.g.
+      // "var {a} = ...") is normalized at parse-time. If n is not a STRING_KEY, it is
+      // an OBJECT_PATTERN or a COMPUTED_PROP and contains a LHS node.
+      return !n.isStringKey();
     }
 
     if (parent.isComputedProp() && n == parent.getSecondChild()) {

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

@@ -1053,27 +1053,6 @@ public boolean apply(Node node) {
             break;
           }
 
-          // ES6 object literal shorthand notation can refer to renamed variables
-        case STRING_KEY:
-          {
-            if (n.hasChildren()
-                || n.isQuotedString()
-                || n.getParent().getParent().isDestructuringLhs()) {
-              break;
-            }
-            Var nameDeclaration = t.getScope().getVar(n.getString());
-            if (nameDeclaration == null) {
-              break;
-            }
-            String importedName = getModuleImportName(t, nameDeclaration.getNode());
-            if (nameDeclaration.isGlobal() || importedName != null) {
-              Node value = IR.name(n.getString()).useSourceInfoFrom(n);
-              n.addChildToBack(value);
-              maybeUpdateName(t, value, nameDeclaration);
-            }
-            break;
-          }
-
         case GETPROP:
           if (n.matchesQualifiedName(MODULE + ".id")) {
             Var v = t.getScope().getVar(MODULE);
@@ -1241,7 +1220,7 @@ private void visitExport(NodeTraversal t, ExportInfo export) {
 
     /**
      * Since CommonJS modules may have only a single export, it's common to see the export be an
-     * object literal. We want to expand this to individual property assignments. If any individual
+     * object pattern. We want to expand this to individual property assignments. If any individual
      * property assignment has been renamed, it will be removed.
      *
      * <p>We need to keep assignments which aren't names
@@ -1725,12 +1704,15 @@ private Node getExportedNameNode(ExportInfo info) {
     private String getModuleImportName(NodeTraversal t, Node n) {
       Node rValue = null;
       String propSuffix = "";
-      if (n.isStringKey()
-          && n.getParent().isObjectPattern()
-          && n.getParent().getParent().isDestructuringLhs()) {
-        rValue = n.getParent().getNext();
-        propSuffix = "." + n.getString();
-      } else if (n.getParent() != null) {
+      Node parent = n.getParent();
+      if (parent != null && parent.isStringKey()) {
+        Node grandparent = parent.getParent();
+        if (grandparent.isObjectPattern() && grandparent.getParent().isDestructuringLhs()) {
+          rValue = grandparent.getNext();
+          propSuffix = "." + parent.getString();
+        }
+      }
+      if (propSuffix.isEmpty() && parent != null) {
         rValue = NodeUtil.getRValueOfLValue(n);
       }
 

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

@@ -74,12 +74,6 @@ class ReplaceIdGenerators implements CompilerPass {
           "Object literal shorthand functions is not allowed in the "
           + "arguments of an id generator");
 
-  static final DiagnosticType SHORTHAND_ASSIGNMENT_NOT_SUPPORTED_IN_ID_GEN =
-      DiagnosticType.error(
-          "JSC_SHORTHAND_ASSIGNMENT_NOT_SUPPORTED_IN_ID_GEN",
-          "Object literal shorthand assignment is not allowed in the "
-          + "arguments of an id generator");
-
   static final DiagnosticType COMPUTED_PROP_NOT_SUPPORTED_IN_ID_GEN =
       DiagnosticType.error(
           "JSC_COMPUTED_PROP_NOT_SUPPORTED_IN_ID_GEN",
@@ -381,10 +375,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
             compiler.report(t.makeError(n, SHORTHAND_FUNCTION_NOT_SUPPORTED_IN_ID_GEN));
             return;
           }
-          if (key.isStringKey() && !key.hasChildren()) {
-            compiler.report(t.makeError(n, SHORTHAND_ASSIGNMENT_NOT_SUPPORTED_IN_ID_GEN));
-            return;
-          }
           if (key.isComputedProp()) {
             compiler.report(t.makeError(n, COMPUTED_PROP_NOT_SUPPORTED_IN_ID_GEN));
             return;

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

@@ -53,11 +53,8 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         }
         break;
       case STRING_KEY:
-        if (n.hasChildren()
-            && n.getFirstChild().isName()
-            && n.getFirstChild().getString().equals(n.getString())) {
-          n.removeFirstChild();
-          compiler.reportChangeToEnclosingScope(n);
+        if (n.getFirstChild().isName() && n.getFirstChild().getString().equals(n.getString())) {
+          n.setShorthandProperty(true);
         }
         break;
       default:

src/com/google/javascript/jscomp/lint/CheckEnums.java

@@ -78,7 +78,7 @@ private void checkName(NodeTraversal t, Node node) {
       return;
     }
 
-    if (node.isStringKey() && !node.hasChildren()) {
+    if (node.isStringKey() && node.isShorthandProperty()) {
       t.report(node, SHORTHAND_ASSIGNMENT_IN_ENUM);
     }
 
@@ -108,4 +108,3 @@ private void checkDuplicateEnumValues(NodeTraversal t, Node n) {
     }
   }
 }
-

src/com/google/javascript/jscomp/parsing/IRFactory.java

@@ -1047,12 +1047,21 @@ Node processObjectPattern(ObjectPatternTree tree) {
       maybeWarnForFeature(tree, Feature.DESTRUCTURING);
 
       Node node = newNode(Token.OBJECT_PATTERN);
-      for (ParseTree c : tree.fields) {
-        Node child = transformNodeWithInlineJsDoc(c);
-        if (child.isRest()) {
-          maybeWarnForFeature(c, Feature.OBJECT_PATTERN_REST);
+      for (ParseTree child : tree.fields) {
+        Node childNode = transformNodeWithInlineJsDoc(child);
+        if (childNode.isDefaultValue()) {
+          // Children of the form {a = b} are parsed as DEFAULT_VALUE{NAME, initializer}.
+          // In this case, insert an extra STRING_KEY node.
+          Node name = childNode.getFirstChild();
+          Node stringKey = newStringNode(Token.STRING_KEY, name.getString());
+          setSourceInfo(stringKey, name);
+          stringKey.setShorthandProperty(true);
+          stringKey.addChildToBack(childNode);
+          childNode = stringKey;
+        } else if (childNode.isRest()) {
+          maybeWarnForFeature(child, Feature.OBJECT_PATTERN_REST);
         }
-        node.addChildToBack(child);
+        node.addChildToBack(childNode);
       }
       return node;
     }
@@ -1636,7 +1645,7 @@ Node processObjectLiteral(ObjectLiteralExpressionTree objTree) {
         if (key.isSpread()) {
           maybeWarnForFeature(el, Feature.OBJECT_LITERALS_WITH_SPREAD);
         }
-        if (!key.hasChildren()) {
+        if (key.isShorthandProperty()) {
           maybeWarn = true;
         }
 
@@ -1748,6 +1757,11 @@ Node processPropertyNameAssignment(PropertyNameAssignmentTree tree) {
       key.setToken(Token.STRING_KEY);
       if (tree.value != null) {
         key.addChildToFront(transform(tree.value));
+      } else {
+        Node value = key.cloneNode();
+        key.setShorthandProperty(true);
+        value.setToken(Token.NAME);
+        key.addChildToFront(value);
       }
       return key;
     }
@@ -2216,7 +2230,14 @@ Node processEnumDeclaration(EnumDeclarationTree tree) {
       Node body = newNode(Token.ENUM_MEMBERS);
       setSourceInfo(body, tree);
       for (ParseTree child : tree.members) {
-        body.addChildToBack(transform(child));
+        Node childNode = transform(child);
+        // NOTE: we may need to "undo" the shorthand property normalization,
+        // since this syntax has a different meaning in enums.
+        if (childNode.isShorthandProperty()) {
+          childNode.removeChild(childNode.getLastChild());
+          childNode.setShorthandProperty(false);
+        }
+        body.addChildToBack(childNode);
       }
 
       return newNode(Token.ENUM, name, body);

src/com/google/javascript/jscomp/parsing/TypeTransformationParser.java

@@ -414,7 +414,7 @@ private boolean validRecordParam(Node expr) {
     if (expr.isObjectLit()) {
       // Each value of a property must be a valid expression
       for (Node prop : expr.children()) {
-        if (!prop.hasChildren()) {
+        if (prop.isShorthandProperty()) {
           warnInvalid("property, missing type", prop);
           return false;
         } else if (!validTypeTransformationExpression(prop.getFirstChild())) {

src/com/google/javascript/rhino/Node.java

@@ -160,8 +160,9 @@ public class Node implements Serializable {
       // need to preserve them for building index.
       IS_UNUSED_PARAMETER = 96, // Mark a parameter as unused. Used to defer work from
       // RemovedUnusedVars to OptimizeParameters.
-      MODULE_EXPORT = 97; // Mark a property as a module export so that collase properties
-  // can act on it.
+      MODULE_EXPORT = 97, // Mark a property as a module export so that collase properties
+      // can act on it.
+      IS_SHORTHAND_PROPERTY = 98; // Indicates that a property {x:x} was originally parsed as {x}.
 
   private static final String propToString(byte propType) {
       switch (propType) {
@@ -224,8 +225,10 @@ private static final String propToString(byte propType) {
         case DELETED:            return "DELETED";
         case GOOG_MODULE_ALIAS:  return "goog_module_alias";
         case IS_UNUSED_PARAMETER: return "is_unused_parameter";
-      case MODULE_EXPORT:
-        return "module_export";
+        case MODULE_EXPORT:
+          return "module_export";
+        case IS_SHORTHAND_PROPERTY:
+          return "is_shorthand_property";
         default:
           throw new IllegalStateException("unexpected prop id " + propType);
       }
@@ -2422,6 +2425,16 @@ public final boolean isUnusedParameter() {
     return getBooleanProp(IS_UNUSED_PARAMETER);
   }
 
+  /** Sets the isShorthandProperty annotation. */
+  public final void setShorthandProperty(boolean shorthand) {
+    putBooleanProp(IS_SHORTHAND_PROPERTY, shorthand);
+  }
+
+  /** Whether this {x:x} property was originally parsed as {x}. */
+  public final boolean isShorthandProperty() {
+    return getBooleanProp(IS_SHORTHAND_PROPERTY);
+  }
+
   /**
    * Sets whether this node is a variable length argument node. This
    * method is meaningful only on {@link Token#NAME} nodes