fix #12: PropertyName kind can conflict with value

src/main/java/com/shapesecurity/shift/ast/property/PropertyName.java

@@ -18,10 +18,10 @@
 
 import com.shapesecurity.shift.ast.Identifier;
 import com.shapesecurity.shift.ast.Node;
-import com.shapesecurity.shift.ast.expression.LiteralNumericExpression;
-import com.shapesecurity.shift.ast.expression.LiteralStringExpression;
 import com.shapesecurity.shift.ast.types.Type;
+import com.shapesecurity.shift.parser.JsError;
 import com.shapesecurity.shift.utils.D2A;
+import com.shapesecurity.shift.utils.Utils;
 import com.shapesecurity.shift.visitor.TransformerP;
 
 import org.jetbrains.annotations.NotNull;
@@ -31,36 +31,26 @@ public final class PropertyName extends Node {
   public final String value;
   public final PropertyNameKind kind;
 
-  public PropertyName(@NotNull PropertyName node) {
-    super();
-    this.value = node.value;
-    this.kind = node.kind;
-  }
-
-  public PropertyName(@NotNull Identifier ident) {
+  private PropertyName(@NotNull String value, @NotNull PropertyNameKind kind) {
     super();
-    this.value = ident.name;
-    this.kind = PropertyNameKind.Identifier;
+    this.value = value;
+    this.kind = kind;
   }
 
-  public PropertyName(@NotNull LiteralStringExpression str) {
-    this(str.value);
+  public PropertyName(@NotNull PropertyName node) {
+    this(node.value, node.kind);
   }
 
-  public PropertyName(@NotNull LiteralNumericExpression num) {
-    this(num.value);
+  public PropertyName(@NotNull Identifier ident) {
+    this(ident.name, PropertyNameKind.Identifier);
   }
 
   public PropertyName(@NotNull String str) {
-    super();
-    this.value = str;
-    this.kind = PropertyNameKind.String;
+    this(str, PropertyNameKind.String);
   }
 
   public PropertyName(double d) {
-    super();
-    this.value = D2A.d2a(d);
-    this.kind = PropertyNameKind.Number;
+    this(D2A.d2a(d), PropertyNameKind.Number);
   }
 
   @NotNull

src/main/java/com/shapesecurity/shift/parser/Parser.java

@@ -1655,17 +1655,19 @@ private PropertyName parseObjectPropertyKey() throws JsError {
     // Note: This function is called only from parseObjectProperty(), where;
     // Eof and Punctuator tokens are already filtered out.
 
+    PropertyName propertyName;
+    SourceLocation location = this.getLocation();
     if (token instanceof StringLiteralToken) {
-      return this.markLocation(this.getLocation(), new PropertyName(this.parseStringLiteral()));
-    }
-    if (token instanceof NumericLiteralToken) {
-      return this.markLocation(this.getLocation(), new PropertyName(this.parseNumericLiteral()));
-    }
-    if (token instanceof IdentifierLikeToken) {
-      return this.markLocation(this.getLocation(), new PropertyName(this.parseIdentifier()));
+      propertyName = new PropertyName(this.parseStringLiteral().value);
+    } else if (token instanceof NumericLiteralToken) {
+      propertyName = new PropertyName(this.parseNumericLiteral().value);
+    } else if (token instanceof IdentifierLikeToken) {
+      propertyName = new PropertyName(this.parseIdentifier());
+    } else {
+      throw this.createError(INVALID_PROPERTY_NAME);
     }
 
-    throw this.createError(INVALID_PROPERTY_NAME);
+    return this.markLocation(location, propertyName);
   }
 
   @NotNull

src/main/java/com/shapesecurity/shift/utils/Utils.java

@@ -250,4 +250,102 @@ public static int getHexValue(char rune) {
     }
     return -1;
   }
+
+  public static boolean isValidNumber(@NotNull String source) {
+    int index = 0;
+    int length = source.length();
+    char[] chs = source.toCharArray();
+
+    char ch = chs[index];
+    if (ch == '0') {
+      index++;
+      if (index < length) {
+        ch = chs[index];
+        if (ch == 'x' || ch == 'X') {
+          return isValidHexLiteral(chs);
+        } else if ('0' <= ch && ch <= '9') {
+          return isValidOctalLiteral(chs);
+        }
+      } else {
+        return true;
+      }
+    } else if ('1' <= ch && ch <= '9') {
+      ch = chs[index];
+      while ('0' <= ch && ch <= '9') {
+        index++;
+        if (index == length) {
+          return true;
+        }
+        ch = chs[index];
+      }
+    } else if (ch != '.') {
+      return false;
+    }
+
+    if (ch == '.') {
+      index++;
+      if (index == length) {
+        return true;
+      }
+
+      ch = chs[index];
+      while ('0' <= ch && ch <= '9') {
+        index++;
+        if (index == length) {
+          return true;
+        }
+        ch = chs[index];
+      }
+    }
+
+    // EOF not reached here
+    if (ch == 'e' || ch == 'E') {
+      index++;
+      if (index == length) {
+        return false;
+      }
+
+      ch = chs[index];
+      if (ch == '+' || ch == '-') {
+        index++;
+        if (index == length) {
+          return false;
+        }
+        ch = chs[index];
+      }
+
+      if ('0' <= ch && ch <= '9') {
+        while ('0' <= ch && ch <= '9') {
+          index++;
+          if (index == length) {
+            return true;
+          }
+          ch = chs[index];
+        }
+      } else {
+        return false;
+      }
+    }
+
+    return index != length;
+  }
+
+  private static boolean isValidOctalLiteral(char[] chs) {
+    for (int i = 1; i < chs.length; i++) {
+      if (chs[i] < '0' || chs[i] > '7') {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  private static boolean isValidHexLiteral(char[] chs) {
+    for (int i = 1; i < chs.length; i++) {
+      if (getHexValue(chs[i]) < 0) {
+        return false;
+      }
+    }
+    return true;
+  }
+
 }

src/main/java/com/shapesecurity/shift/validator/Validator.java

@@ -32,6 +32,7 @@
 import com.shapesecurity.shift.ast.expression.PrefixExpression;
 import com.shapesecurity.shift.ast.operators.PrefixOperator;
 import com.shapesecurity.shift.ast.property.ObjectProperty;
+import com.shapesecurity.shift.ast.property.PropertyName;
 import com.shapesecurity.shift.ast.property.Setter;
 import com.shapesecurity.shift.ast.statement.BreakStatement;
 import com.shapesecurity.shift.ast.statement.ContinueStatement;
@@ -447,7 +448,12 @@ public ValidationContext reduceSwitchStatementWithDefault(
       @NotNull List<ValidationContext> preDefaultCases,
       @NotNull ValidationContext defaultCase,
       @NotNull List<ValidationContext> postDefaultCases) {
-    return super.reduceSwitchStatementWithDefault(node, path, discriminant, preDefaultCases, defaultCase, postDefaultCases)
+    return super.reduceSwitchStatementWithDefault(node,
+        path,
+        discriminant,
+        preDefaultCases,
+        defaultCase,
+        postDefaultCases)
         .clearFreeBreakStatements();
   }
 
@@ -487,4 +493,24 @@ public ValidationContext reduceWithStatement(
             node,
             "WithStatement not allowed in strict mode"));
   }
+
+  @NotNull
+  @Override
+  public ValidationContext reducePropertyName(@NotNull PropertyName node, @NotNull List<Branch> path) {
+    ValidationContext v = super.reducePropertyName(node, path);
+    switch (node.kind) {
+    case Identifier:
+      if (!Utils.isValidIdentifierName(node.value)) {
+        return v.addError(
+            new ValidationError(node, "PropertyName of kind 'identifier' must be valid identifier name."));
+      }
+      break;
+    case Number:
+      if (!Utils.isValidNumber(node.value)) {
+        return v.addError(new ValidationError(node, "PropertyName of kind 'number' must be a valid number literal."));
+      }
+      break;
+    }
+    return v;
+  }
 }

src/test/java/com/shapesecurity/shift/AstHelper.java

@@ -32,6 +32,10 @@
 import com.shapesecurity.shift.ast.expression.LiteralBooleanExpression;
 import com.shapesecurity.shift.ast.expression.LiteralNullExpression;
 import com.shapesecurity.shift.ast.expression.LiteralNumericExpression;
+import com.shapesecurity.shift.ast.expression.ObjectExpression;
+import com.shapesecurity.shift.ast.property.DataProperty;
+import com.shapesecurity.shift.ast.property.ObjectProperty;
+import com.shapesecurity.shift.ast.property.PropertyName;
 import com.shapesecurity.shift.ast.statement.BlockStatement;
 import com.shapesecurity.shift.ast.statement.EmptyStatement;
 import com.shapesecurity.shift.ast.statement.ExpressionStatement;
@@ -148,4 +152,20 @@ public static void invalidExpr(int numExpectedErrs, Expression e) {
     Assert.assertTrue(!errs.isEmpty());
     Assert.assertEquals(numExpectedErrs, errs.length);
   }
+
+  protected static ObjectExpression obj(ObjectProperty... properties) {
+    return new ObjectExpression(List.from(properties));
+  }
+
+  protected static PropertyName pn(Identifier ident) {
+    return new PropertyName(ident);
+  }
+
+  protected static PropertyName pn(double value) {
+    return new PropertyName(value);
+  }
+
+  protected static DataProperty init(PropertyName propertyName, Expression value) {
+    return new DataProperty(propertyName, value);
+  }
 }

src/test/java/com/shapesecurity/shift/validator/UnitTest.java

@@ -178,6 +178,26 @@ public final void testVariableDeclarationStatementInForInVarStatementCanOnlyHasO
     invalidStmt(1, new ForInStatement(vars(VariableDeclarationKind.Var, "a", "b"), EXPR, STMT));
   }
 
+  @Test
+  public final void testPropertyNameOfKindIdentifierMustBeValidIdentifier() {
+    validExpr(obj(init(pn(ID), EXPR)));
+    validExpr(obj(init(pn(new Identifier("function")), EXPR)));
+    invalidExpr(1, obj(init(pn(new Identifier("x x")), EXPR)));
+    invalidExpr(1, obj(init(pn(new Identifier(" ")), EXPR)));
+  }
+
+  @Test
+  public final void testPropertyNameOfKindNumberMustBeValidNumber() {
+    validExpr(obj(init(pn(0), EXPR)));
+    validExpr(obj(init(pn(3), EXPR)));
+    validExpr(obj(init(new PropertyName(""), EXPR)));
+    validExpr(obj(init(new PropertyName("not an ident"), EXPR)));
+    invalidExpr(1, obj(init(pn(-1), EXPR)));
+    invalidExpr(1, obj(init(pn(Double.NaN), EXPR)));
+    invalidExpr(1, obj(init(pn(Double.POSITIVE_INFINITY), EXPR)));
+    invalidExpr(1, obj(init(pn(Double.NEGATIVE_INFINITY), EXPR)));
+  }
+
   private void testLibrary(String fileName) throws IOException, JsError {
     String source = readLibrary(fileName);
     Script script = Parser.parse(source);