replace `children` declaration language feature with a trait without any special syntax

[fredemmott]

My perspective

• we're unlikely to add first-class typechecker support, as it has its’ own type system that is inconsistent with the rest of hack.
• we're unlikely to remove it as it does provide some safety at runtime that users have been depending on for over a decade

It feels like it's likely to stay in limbo.

With the introduction of reified generics, I think it can actually be implemented better as a standard Hack trait, without any special syntax.

Why is it weird?

• it has its' own type system - e.g. the magic pcdata, any, and empty types
• child declarations are basically regular expressions
• it is a type-based language feature with no typechecker validation
  • appendChild() would need custom support or to be banned - huge impact on usage
  • would be extremely special case given the regex-like validation
  • while we are aiming to make the typechecker verify that the declarations do not refer to classes that do not exist, there will still be no typechecker validation that usage matches the declaration
• it does not get emitted in a standard way - e.g. string class names with no type structure. This /could/ be fixed, e.g. by adding type structures here, but we'd need to expose the type structure to the library code - I don't see how this could be reified.

Proposal

1. prototype the rough idea below in open source XHP-Lib, HHAST, and migrate docs.hhvm.com
2. log when trait-based and existing validation differs.
3. FB ports to FB www
4. delete the syntax from the parser

Rough idea:

class :foo extends :x:element {
  use XHPChildrenValidation;

  // old:
  //  children (:bar*, :baz?, pcdata);

  // new:
  protected static function getChildValidationPattern(): XHPChildPattern {
    return XHPChild::tuple(
      XHPChild::anyNumberOf(XHPChild::ofType<:bar>()),
      XHPChild::optional(XHPChild::ofType<:baz>())
      XHPChild::pcdata(),
    );
  }

  protected async function genRender() {
    $this->validateChildren(); // or, automatic:
    // - maybe check for presence of `IHaveChildValidation`
    //   or similar in framework?
    // - maybe make trait override getChildren() and friends
  }
}

XHPChild::anyNumberOf(XHPChild::ofType<:bar>()) is a bit verbose; the problem with XHPChild::anyNumberOf<:bar>() or similar is patterns like children ((:bar | :baz)*) - with the verbose syntax, they're something like XHPChild::anyNumberOf(XHPChild::ofUnionType(XHPChild::ofType<:bar>(), XHPChild::ofType<:baz>()) , but I don't see a clean way to fit them in to something like XHPChild::anyNumberOf<T>()

Migrations

This can be done automatically:

• via HackAST/HHAST
• by using the existing parser - add codegen methods to ReflectionXHPChildrenDeclaration

Verifying correctness

• verify the automated migration: add XHPChild::toLegacySerialization(), verify it matches __xhpChildrenDeclaration()
• if verification is reimplemented, can verify children via both approaches, and log/fatal if result differs

Why do reified generics help?

They allow XHPChild::of<T>() consistently, rather than XHPChild::string(), XHPChild::instanceOfClass(classname<T>) etc

Additionally, classname<T> is getting harder to use, as the typechecker gets stricter about unsafe usage.

Why is it better?

• type-based language features without full typechecker support are extremely surprising to Hack users; while implementing in a trait does not improve safety, users have more accurate expectations
• easy to extend to support new features, such as new types, generics, etc - no need to synchronize changes between the parser, typechecker, runtime, and xhp-lib
• easier to simplify to remove unwanted features: makes the possibility of a stricter, fully-supported, but less flexible language feature much more practical for migration; e.g.
  1. split out XHPElementWithExactlyOneChildOfType<T>() and XHPElementWithAnyNumberOfChildrenOfType<T>()
  2. add new language feature that can represent those only (possibly similar to React's child validation)
  3. migrate those types, but keep using the trait for the more powerful regexp-style constraints
• simplifies the language - less syntax, fewer type systems
  • new types will be automatically supported, as long as reified generics support them
• makes replacing XHP categories with standard Hack interfaces much more practical

How to start?

• constructive feedback is very useful
• the next step is to prototype; while this isn't currently a priority for us, this is a problem that is very suited to pull requests

[Wilfred]

I think this is a very worthwhile change.

(1) Hack is a big language and removing this would make learning the language easier. The syntax here is really weird and not used elsewhere (e.g. %my-category, :my-class+).

(2) There's no type checker support today, and we don't have any plans to implement the full attribute/children patterns in the type checker.

(3) Since the checking of children happens in userland code at runtime anyway, it would be more consistent to use normal code to define the constraints.

(Since XHP is very old and reified generics are very new, I'm sure there will be a few bugs when we initially exercise it FWIW.)

[fredemmott]

Today's hack nightlies add disable_xhp_children_declarations hhconfig option

[fredemmott]

This is mostly done - though xhp-lib should be modified to throw an exception if an old-style children declaration is present.