Improve representation of simple xhp class attributes

Summary:
An xhp attribute class declaration can be either complex -- a type, name, initializer and requires clause -- or just an XHP type name.

Many of these attribute declarations can be in a list.  For example:

    attribute Foo bar = whatever, :button;

We were previously parsing the "simple" form by simply sticking the token in the list. But that is weird in two ways. First, it seems "unparallel" that list items can be either a complex structure or a token. Second, it is weird that a type name appears in the parse tree without being inside a simple type node.

We fix both problems at once, by wrapping up the token first in a simple type, and then in a new "simple attribute" parse node.

Thanks to Kendall for pointing out the issue.

Reviewed By: KendallHopkins

Differential Revision: D4433214

fbshipit-source-id: 91f59c7be2180ed6b7717a520774b225837884d2

hphp/hack/src/full_fidelity/full_fidelity_declaration_parser.ml

@@ -750,7 +750,10 @@ module WithExpressionAndStatementAndTypeParser
       let (parser1, class_name) = expect_class_name parser in
       match peek_token_kind parser1 with
       | Comma
-      | Semicolon -> (parser1, class_name)
+      | Semicolon ->
+        let type_specifier = make_simple_type_specifier class_name in
+        let result = make_xhp_simple_class_attribute type_specifier in
+        (parser1, result)
       | _ -> parse_xhp_class_attribute_typed parser
     else
       parse_xhp_class_attribute_typed parser

hphp/hack/src/full_fidelity/full_fidelity_pretty_printer.ml

@@ -315,6 +315,8 @@ let rec get_doc node =
     let i = get_doc xhp_attribute_decl_initializer in
     let r = get_doc xhp_attribute_decl_required in
     group_doc (t ^| n ^| i ^| r)
+  | XHPSimpleClassAttribute { xhp_simple_class_attribute_type } ->
+    get_doc xhp_simple_class_attribute_type
   | TraitUse {
     trait_use_keyword;
     trait_use_names;

hphp/hack/src/full_fidelity/full_fidelity_syntax.ml

@@ -679,6 +679,9 @@ module WithToken(Token: TokenType) = struct
       xhp_attribute_decl_initializer: t;
       xhp_attribute_decl_required: t;
     }
+    and xhp_simple_class_attribute = {
+      xhp_simple_class_attribute_type: t;
+    }
     and xhp_attribute = {
       xhp_attribute_name: t;
       xhp_attribute_equal: t;
@@ -932,6 +935,7 @@ module WithToken(Token: TokenType) = struct
     | XHPRequired of xhp_required
     | XHPClassAttributeDeclaration of xhp_class_attribute_declaration
     | XHPClassAttribute of xhp_class_attribute
+    | XHPSimpleClassAttribute of xhp_simple_class_attribute
     | XHPAttribute of xhp_attribute
     | XHPOpen of xhp_open
     | XHPExpression of xhp_expression
@@ -1185,6 +1189,8 @@ module WithToken(Token: TokenType) = struct
         SyntaxKind.XHPClassAttributeDeclaration
       | XHPClassAttribute _ ->
         SyntaxKind.XHPClassAttribute
+      | XHPSimpleClassAttribute _ ->
+        SyntaxKind.XHPSimpleClassAttribute
       | XHPAttribute _ ->
         SyntaxKind.XHPAttribute
       | XHPOpen _ ->
@@ -1454,6 +1460,8 @@ module WithToken(Token: TokenType) = struct
       kind node = SyntaxKind.XHPClassAttributeDeclaration
     let is_xhp_class_attribute node =
       kind node = SyntaxKind.XHPClassAttribute
+    let is_xhp_simple_class_attribute node =
+      kind node = SyntaxKind.XHPSimpleClassAttribute
     let is_xhp_attribute node =
       kind node = SyntaxKind.XHPAttribute
     let is_xhp_open node =
@@ -2716,6 +2724,12 @@ module WithToken(Token: TokenType) = struct
       xhp_attribute_decl_required
     )
 
+    let get_xhp_simple_class_attribute_children {
+      xhp_simple_class_attribute_type;
+    } = (
+      xhp_simple_class_attribute_type
+    )
+
     let get_xhp_attribute_children {
       xhp_attribute_name;
       xhp_attribute_equal;
@@ -4083,6 +4097,11 @@ module WithToken(Token: TokenType) = struct
         xhp_attribute_decl_initializer;
         xhp_attribute_decl_required;
       ]
+      | XHPSimpleClassAttribute {
+        xhp_simple_class_attribute_type;
+      } -> [
+        xhp_simple_class_attribute_type;
+      ]
       | XHPAttribute {
         xhp_attribute_name;
         xhp_attribute_equal;
@@ -5423,6 +5442,11 @@ module WithToken(Token: TokenType) = struct
         "xhp_attribute_decl_initializer";
         "xhp_attribute_decl_required";
       ]
+      | XHPSimpleClassAttribute {
+        xhp_simple_class_attribute_type;
+      } -> [
+        "xhp_simple_class_attribute_type";
+      ]
       | XHPAttribute {
         xhp_attribute_name;
         xhp_attribute_equal;
@@ -6919,6 +6943,12 @@ module WithToken(Token: TokenType) = struct
           xhp_attribute_decl_initializer;
           xhp_attribute_decl_required;
         }
+      | (SyntaxKind.XHPSimpleClassAttribute, [
+          xhp_simple_class_attribute_type;
+        ]) ->
+        XHPSimpleClassAttribute {
+          xhp_simple_class_attribute_type;
+        }
       | (SyntaxKind.XHPAttribute, [
           xhp_attribute_name;
           xhp_attribute_equal;
@@ -8527,6 +8557,13 @@ module WithToken(Token: TokenType) = struct
         xhp_attribute_decl_required;
       ]
 
+    let make_xhp_simple_class_attribute
+      xhp_simple_class_attribute_type
+    =
+      from_children SyntaxKind.XHPSimpleClassAttribute [
+        xhp_simple_class_attribute_type;
+      ]
+
     let make_xhp_attribute
       xhp_attribute_name
       xhp_attribute_equal

hphp/hack/src/full_fidelity/full_fidelity_syntax_kind.ml

@@ -122,6 +122,7 @@ type t =
 | XHPRequired
 | XHPClassAttributeDeclaration
 | XHPClassAttribute
+| XHPSimpleClassAttribute
 | XHPAttribute
 | XHPOpen
 | XHPExpression
@@ -258,6 +259,7 @@ let to_string kind =
   | XHPRequired -> "xhp_required"
   | XHPClassAttributeDeclaration -> "xhp_class_attribute_declaration"
   | XHPClassAttribute -> "xhp_class_attribute"
+  | XHPSimpleClassAttribute -> "xhp_simple_class_attribute"
   | XHPAttribute -> "xhp_attribute"
   | XHPOpen -> "xhp_open"
   | XHPExpression -> "xhp_expression"

hphp/hack/src/full_fidelity/js/full_fidelity_editable.js

@@ -284,6 +284,8 @@ class EditableSyntax
       return XHPClassAttributeDeclaration.from_json(json, position, source);
     case 'xhp_class_attribute':
       return XHPClassAttribute.from_json(json, position, source);
+    case 'xhp_simple_class_attribute':
+      return XHPSimpleClassAttribute.from_json(json, position, source);
     case 'xhp_attribute':
       return XHPAttribute.from_json(json, position, source);
     case 'xhp_open':
@@ -13065,6 +13067,53 @@ class XHPClassAttribute extends EditableSyntax
     return XHPClassAttribute._children_keys;
   }
 }
+class XHPSimpleClassAttribute extends EditableSyntax
+{
+  constructor(
+    type)
+  {
+    super('xhp_simple_class_attribute', {
+      type: type });
+  }
+  get type() { return this.children.type; }
+  with_type(type){
+    return new XHPSimpleClassAttribute(
+      type);
+  }
+  rewrite(rewriter, parents)
+  {
+    if (parents == undefined)
+      parents = [];
+    let new_parents = parents.slice();
+    new_parents.push(this);
+    var type = this.type.rewrite(rewriter, new_parents);
+    if (
+      type === this.type)
+    {
+      return rewriter(this, parents);
+    }
+    else
+    {
+      return rewriter(new XHPSimpleClassAttribute(
+        type), parents);
+    }
+  }
+  static from_json(json, position, source)
+  {
+    let type = EditableSyntax.from_json(
+      json.xhp_simple_class_attribute_type, position, source);
+    position += type.width;
+    return new XHPSimpleClassAttribute(
+        type);
+  }
+  get children_keys()
+  {
+    if (XHPSimpleClassAttribute._children_keys == null)
+      XHPSimpleClassAttribute._children_keys = [
+        'type'];
+    return XHPSimpleClassAttribute._children_keys;
+  }
+}
 class XHPAttribute extends EditableSyntax
 {
   constructor(
@@ -15894,6 +15943,7 @@ exports.XHPEnumType = XHPEnumType;
 exports.XHPRequired = XHPRequired;
 exports.XHPClassAttributeDeclaration = XHPClassAttributeDeclaration;
 exports.XHPClassAttribute = XHPClassAttribute;
+exports.XHPSimpleClassAttribute = XHPSimpleClassAttribute;
 exports.XHPAttribute = XHPAttribute;
 exports.XHPOpen = XHPOpen;
 exports.XHPExpression = XHPExpression;

hphp/hack/src/full_fidelity/js/full_fidelity_schema.json

@@ -1,6 +1,6 @@
 { "description" :
   "Auto-generated JSON schema of the Hack Full Fidelity Parser AST",
-  "version" : "2017-01-18-0001",
+  "version" : "2017-01-18-0002",
   "trivia" : [
     { "trivia_kind_name" : "WhiteSpace",
       "trivia_type_name" : "whitespace" },
@@ -1381,6 +1381,13 @@
         { "field_name" : "initializer" },
         { "field_name" : "required" }
       ] },
+    { "kind_name" : "XHPSimpleClassAttribute",
+      "type_name" : "xhp_simple_class_attribute",
+      "description" : "xhp_simple_class_attribute",
+      "prefix" : "xhp_simple_class_attribute",
+      "fields" : [
+        { "field_name" : "type" }
+      ] },
     { "kind_name" : "XHPAttribute",
       "type_name" : "xhp_attribute",
       "description" : "xhp_attribute",

hphp/hack/src/full_fidelity/php/full_fidelity_editable.php

@@ -347,6 +347,8 @@ public static function from_json(mixed $json, int $position, string $source) {
       return XHPClassAttributeDeclaration::from_json($json, $position, $source);
     case 'xhp_class_attribute':
       return XHPClassAttribute::from_json($json, $position, $source);
+    case 'xhp_simple_class_attribute':
+      return XHPSimpleClassAttribute::from_json($json, $position, $source);
     case 'xhp_attribute':
       return XHPAttribute::from_json($json, $position, $source);
     case 'xhp_open':
@@ -15098,6 +15100,49 @@ public function children(): Generator<string, EditableSyntax, void> {
     yield break;
   }
 }
+final class XHPSimpleClassAttribute extends EditableSyntax {
+  private EditableSyntax $_type;
+  public function __construct(
+    EditableSyntax $type) {
+    parent::__construct('xhp_simple_class_attribute');
+    $this->_type = $type;
+  }
+  public function type(): EditableSyntax {
+    return $this->_type;
+  }
+  public function with_type(EditableSyntax $type): XHPSimpleClassAttribute {
+    return new XHPSimpleClassAttribute(
+      $type);
+  }
+
+  public function rewrite(
+    ( function
+      (EditableSyntax, ?array<EditableSyntax>): ?EditableSyntax ) $rewriter,
+    ?array<EditableSyntax> $parents = null): ?EditableSyntax {
+    $new_parents = $parents ?? [];
+    array_push($new_parents, $this);
+    $type = $this->type()->rewrite($rewriter, $new_parents);
+    if (
+      $type === $this->type()) {
+      return $rewriter($this, $parents ?? []);
+    } else {
+      return $rewriter(new XHPSimpleClassAttribute(
+        $type), $parents ?? []);
+    }
+  }
+
+  public static function from_json(mixed $json, int $position, string $source) {
+    $type = EditableSyntax::from_json(
+      $json->xhp_simple_class_attribute_type, $position, $source);
+    $position += $type->width();
+    return new XHPSimpleClassAttribute(
+        $type);
+  }
+  public function children(): Generator<string, EditableSyntax, void> {
+    yield $this->_type;
+    yield break;
+  }
+}
 final class XHPAttribute extends EditableSyntax {
   private EditableSyntax $_name;
   private EditableSyntax $_equal;

hphp/hack/src/full_fidelity/schema/full_fidelity_schema.ml

@@ -10,7 +10,7 @@
 
 (* If you make changes to the schema that cause it to serialize / deserialize
 differently, please update this version number *)
-let full_fidelity_schema_version_number = "2017-01-18-0001"
+let full_fidelity_schema_version_number = "2017-01-18-0002"
 (* TODO: Consider basing the version number on an auto-generated
 hash of a file rather than relying on people remembering to update it. *)
 (* TODO: It may be worthwhile to investigate how Thrift describes data types
@@ -872,6 +872,11 @@ let schema = List.map from_list [
     "name";
     "initializer";
     "required" ];
+  [ "XHPSimpleClassAttribute";
+    "xhp_simple_class_attribute";
+    "xhp_simple_class_attribute";
+    "xhp_simple_class_attribute";
+    "type" ];
   [ "XHPAttribute";
     "xhp_attribute";
     "xhp_attribute";

hphp/hack/src/hackfmt/hack_format.ml

@@ -1175,6 +1175,9 @@ offending text is '%s'." (text node)));
     t init;
     if not (is_missing req) then pending_space ();
     t req;
+  | XHPSimpleClassAttribute x ->
+    let attr_type = get_xhp_simple_class_attribute_children x in
+    t attr_type;
   | XHPAttribute x ->
     let (name, eq, expr) = get_xhp_attribute_children x in
     tl_with ~span ~f:(fun () ->