Automatically call transferAttributes as needed.

- split out XHPBaseHTMLHelpers to just implement the ID and class helpers
  without transferAttributes
- added tests
- XHPHelpers should probably be renamed to XHPHTMLHelpers in XHP-Lib 3, and
  the bulk of the transfer/copyattribtues functiontionality split to
  XHPTransferAttributes or something like that. Keeping like this for now to
  reduce BC breakage
- Allows implementation of non-HTML helpers; until the above modification happens
  this will involve copypasta, but better than the current situation.
- I /think/ this gets rid of the render()/compose() pattern.

fixes #130
closes #133

Author: fredemmott

autoload-map.php

@@ -11,6 +11,7 @@
 
 function autoload($class): bool {
   $classmap = Map {
+    'HasXHPBaseHTMLHelpers' => '/src/html/HasXHPBaseHTMLHelpers.php',
     'HasXHPHelpers' => '/src/html/XHPHelpers.php',
     'ReflectionXHPAttribute' => '/src/core/ReflectionXHPAttribute.php',
     'ReflectionXHPChildrenDeclaration' => '/src/core/ReflectionXHPChildrenDeclaration.php',
@@ -24,12 +25,14 @@ function autoload($class): bool {
     'XHPAttributeRequiredException' => '/src/exceptions/AttributeRequiredException.php',
     'XHPAttributeType' => '/src/core/ReflectionXHPAttribute.php',
     'XHPAwaitable' => '/src/core/XHPAwaitable.php',
+    'XHPBaseHTMLHelpers' => '/src/html/XHPBaseHTMLHelpers.php',
     'XHPChildrenConstraintType' => '/src/core/ReflectionXHPChildrenDeclaration.php',
     'XHPChildrenDeclarationType' => '/src/core/ReflectionXHPChildrenDeclaration.php',
     'XHPChildrenExpressionType' => '/src/core/ReflectionXHPChildrenDeclaration.php',
     'XHPClassException' => '/src/exceptions/ClassException.php',
     'XHPCoreRenderException' => '/src/exceptions/CoreRenderException.php',
     'XHPException' => '/src/exceptions/Exception.php',
+    'XHPHasTransferAttributes' => '/src/core/XHPHasTransferAttributes.php',
     'XHPHelpers' => '/src/html/XHPHelpers.php',
     'XHPInvalidArrayAttributeException' => '/src/exceptions/InvalidArrayAttributeException.php',
     'XHPInvalidArrayKeyAttributeException' => '/src/exceptions/InvalidArrayKeyAttributeException.php',

src/core/Element.php

@@ -47,6 +47,9 @@ final public function toString(): string {
       }
       assert($composed instanceof :x:element);
       $composed->__transferContext($that->getAllContexts());
+      if ($that instanceof XHPHasTransferAttributes) {
+        $that->transferAttributesToRenderedRoot($composed);
+      }
       $that = $composed;
     } while ($composed instanceof :x:element);
 

src/core/XHPHasTransferAttributes.php

@@ -0,0 +1,22 @@
+<?hh // strict
+/*
+ *  Copyright (c) 2015, Facebook, Inc.
+ *  All rights reserved.
+ *
+ *  This source code is licensed under the BSD-style license found in the
+ *  LICENSE file in the root directory of this source tree. An additional grant
+ *  of patent rights can be found in the PATENTS file in the same directory.
+ *
+ */
+
+/**
+ * Indicates that any attributes set on an element should be transferred to the
+ * element returned from ::render() or ::asyncRender(). This is automatically
+ * invoked by :x:element.
+ */
+interface XHPHasTransferAttributes {
+  require extends :x:element;
+  public function transferAttributesToRenderedRoot(
+    :x:composable-element $root,
+  ): void;
+}

src/html/Element.php

@@ -16,8 +16,7 @@
  * own elements.
  */
 abstract class :xhp:html-element extends :x:primitive {
-
-  use XHPHelpers;
+  use XHPBaseHTMLHelpers;
 
   attribute
     // Global HTML attributes

src/html/HasXHPBaseHTMLHelpers.php

@@ -0,0 +1,19 @@
+<?hh // strict
+/*
+ *  Copyright (c) 2015, Facebook, Inc.
+ *  All rights reserved.
+ *
+ *  This source code is licensed under the BSD-style license found in the
+ *  LICENSE file in the root directory of this source tree. An additional grant
+ *  of patent rights can be found in the PATENTS file in the same directory.
+ *
+ */
+
+interface HasXHPBaseHTMLHelpers {
+  require extends :x:composable-element;
+
+  public function addClass(string $class): this;
+  public function conditionClass(bool $cond, string $class): this;
+  public function requireUniqueID(): string;
+  public function getID(): string;
+}

src/html/XHPBaseHTMLHelpers.php

@@ -0,0 +1,68 @@
+<?hh // strict
+
+/*
+ *  Copyright (c) 2015, Facebook, Inc.
+ *  All rights reserved.
+ *
+ *  This source code is licensed under the BSD-style license found in the
+ *  LICENSE file in the root directory of this source tree. An additional grant
+ *  of patent rights can be found in the PATENTS file in the same directory.
+ *
+ */
+
+trait XHPBaseHTMLHelpers implements HasXHPBaseHTMLHelpers {
+  require extends :x:composable-element;
+
+  /*
+   * Appends a string to the "class" attribute (space separated).
+   */
+  public function addClass(string $class): this {
+    try {
+      $current_class = /* UNSAFE_EXPR */ $this->:class;
+      return $this->setAttribute('class', trim($current_class.' '.$class));
+    } catch (XHPInvalidAttributeException $error) {
+      throw new XHPException(
+        'You are trying to add an HTML class to a(n) '.
+        :xhp::class2element(static::class).' element, but it does not support '.
+        'the "class" attribute. The best way to do this is to inherit '.
+        'the HTML attributes from the element your component will render into.',
+      );
+    }
+  }
+
+  /*
+   * Conditionally adds a class to the "class" attribute.
+   */
+  public function conditionClass(bool $cond, string $class): this {
+    return $cond ? $this->addClass($class) : $this;
+  }
+
+  /*
+   * Generates a unique ID (and sets it) on the "id" attribute. A unique ID
+   * will only be generated if one has not already been set.
+   */
+  public function requireUniqueID(): string {
+    $id = /* UNSAFE_EXPR */ $this->:id;
+    if ($id === null || $id === '') {
+      try {
+        $this->setAttribute('id', $id = substr(md5(mt_rand(0, 100000)), 0, 10));
+      } catch (XHPInvalidAttributeException $error) {
+        throw new XHPException(
+          'You are trying to add an HTML id to a(n) '.
+          :xhp::class2element(static::class).' element, but it does not '.
+          'support the "id" attribute. The best way to do this is to inherit '.
+          'the HTML attributes from the element your component will render '.
+          'into.',
+        );
+      }
+    }
+    return (string)$id;
+  }
+
+  /*
+   * Fetches the "id" attribute, will generate a unique value if not set.
+   */
+  final public function getID(): string {
+    return $this->requireUniqueID();
+  }
+}

src/html/XHPHelpers.php

@@ -9,8 +9,7 @@
  *
  */
 
-interface HasXHPHelpers {
-  require extends :x:composable-element;
+interface HasXHPHelpers extends HasXHPBaseHTMLHelpers, XHPHasTransferAttributes {
 };
 
 /*
@@ -20,61 +19,9 @@ interface HasXHPHelpers {
  * attribute :xhp:html-element;
  */
 trait XHPHelpers implements HasXHPHelpers {
-
   require extends :x:composable-element;
 
-  /*
-   * Appends a string to the "class" attribute (space separated).
-   */
-  public function addClass(string $class): this {
-    try {
-      $current_class = /* UNSAFE_EXPR */ $this->:class;
-      return $this->setAttribute('class', trim($current_class.' '.$class));
-    } catch (XHPInvalidAttributeException $error) {
-      throw new XHPException(
-        'You are trying to add an HTML class to a(n) '.
-        :xhp::class2element(static::class).' element, but it does not support '.
-        'the "class" attribute. The best way to do this is to inherit '.
-        'the HTML attributes from the element your component will render into.',
-      );
-    }
-  }
-
-  /*
-   * Conditionally adds a class to the "class" attribute.
-   */
-  public function conditionClass(bool $cond, string $class): this {
-    return $cond ? $this->addClass($class) : $this;
-  }
-
-  /*
-   * Generates a unique ID (and sets it) on the "id" attribute. A unique ID
-   * will only be generated if one has not already been set.
-   */
-  public function requireUniqueID(): string {
-    $id = /* UNSAFE_EXPR */ $this->:id;
-    if ($id === null || $id === '') {
-      try {
-        $this->setAttribute('id', $id = substr(md5(mt_rand(0, 100000)), 0, 10));
-      } catch (XHPInvalidAttributeException $error) {
-        throw new XHPException(
-          'You are trying to add an HTML id to a(n) '.
-          :xhp::class2element(static::class).' element, but it does not '.
-          'support the "id" attribute. The best way to do this is to inherit '.
-          'the HTML attributes from the element your component will render '.
-          'into.',
-        );
-      }
-    }
-    return (string)$id;
-  }
-
-  /*
-   * Fetches the "id" attribute, will generate a unique value if not set.
-   */
-  final public function getID(): string {
-    return $this->requireUniqueID();
-  }
+  use XHPBaseHTMLHelpers;
 
   /*
    * Copies all attributes that are set on $this and valid on $target to
@@ -187,4 +134,59 @@ final private function transferAttributesImpl(
     }
   }
 
+  protected function getAttributeNamesThatAppendValuesOnTransfer(): ImmSet<string> {
+    return ImmSet { 'class' };
+  }
+
+  final public function transferAttributesToRenderedRoot(
+    :x:composable-element $root,
+  ): void {
+    if (:xhp::$ENABLE_VALIDATION && $root instanceof :x:element) {
+      if (!($root instanceof HasXHPHelpers)) {
+        throw new XHPClassException(
+          $this,
+          'render() must return an object using the XHPHelpers trait.'
+        );
+      }
+
+      $rootID = $root->getAttribute('id') ?: null;
+      $thisID = $this->getAttribute('id') ?: null;
+
+      if ($rootID && $thisID && $rootID != $thisID) {
+        throw new XHPException(
+          'ID Collision. '.(:xhp::class2element(self::class)).' has an ID '.
+          'of "'.$thisID.'" but it renders into a(n) '.
+          (:xhp::class2element(get_class($root))).
+          ' which has an ID of "'.$rootID.'". The latter will get '.
+          'overwritten (most often unexpectedly). If you are intending for '.
+          'this behavior consider calling $this->removeAttribute(\'id\') '.
+          'before returning your node from compose().'
+        );
+      }
+    }
+    assert($root instanceof HasXHPHelpers);
+
+    $attributes = $this->getAttributes();
+
+    // We want to append classes to the root node, instead of replace them,
+    // so do this attribute manually and then remove it.
+    foreach ($this->getAttributeNamesThatAppendValuesOnTransfer() as $attr) {
+      if (array_key_exists($attr, $attributes)) {
+        $rootAttributes = $root->getAttributes();
+        if (
+          array_key_exists($attr, $rootAttributes)
+          && ($rootValue = (string) $rootAttributes[$attr]) !== ''
+        ) {
+          $thisValue = (string) $attributes[$attr];
+          if ($thisValue !== '') {
+            $root->setAttribute($attr, $rootValue.' '.$thisValue);
+          }
+          $this->removeAttribute($attr);
+        }
+      }
+    }
+
+    // Transfer all valid attributes to the returned node.
+    $this->transferAllAttributes($root);
+  }
 }