[ RFC ] Do we want to open up using custom TypeSpec<T>'s for the reified API's?

[lexidor]

With the current capabilities of TypeAssert, we can derive from Facebook\TypeSpec\TypeSpec<T> to implement validation for custom newtypes. These specs compose just like the built-in specs.

__Private\from_type_structure<T>() (and its friends like TypeSpec\of<T>()) will however not know how to find our custom TypeSpecs. This means that you need to manually build the complex TypeSpec object via TypeSpec\xxx() calls in practice.

Do we want to offer the capability to supply your own TypeSpecs for your own newtypes and have them be recognized by TypeSpec\of<T>() and friends?

This has the major downside of revealing how TypeSpec\of<T>() works and will prevent us from changing our implementation away from TypeStrucuture<T>. The fact that we use TypeStrucuture<T> is an implementation detail of this library.

I have created a PR which demonstrates how something like this may work.
I am not suggesting that this is the PR that adds this feature.
I just need to be able to point to some example code.

[lexidor]

The definition of UserID can be found here.

The all the code of the PoC can be found here #50.

Why the current behavior is not well-typed

The current behavior of __Private\from_type_structure<T>() ignores opaque type aliases and returns the TypeSpec for the runtime type. The typechecker is not aware of this and it will create a TypeSpec with the newtype in there.

This creates a situation where the invariants of a newtype can be trampled upon, without a visible HH_FIXME[].

https://github.com/lexidor/type-assert/blob/4f9cb175bb403b74eca1f33285357fdeb63e2cc6/src/example/old_behavior.hack#L22-L30

  // @type TypeSpec<dict<string, UserID>>
  $with_manual_typespec = TypeSpec\dict(TypeSpec\string(), MyTypeSpec\UserID());
  echo '$with_manual_typespec: '.$with_manual_typespec->toString().\PHP_EOL;


  // @typechecker-type TypeSpec<dict<string, UserID>>
  // @runtime-type     TypeSpec<dict<string, int>>
  $with_typespec_of = TypeSpec\of<dict<string, UserID>>();
  echo '$with_typespec_of:     '.$with_typespec_of->toString().\PHP_EOL;

https://github.com/lexidor/type-assert/blob/4f9cb175bb403b74eca1f33285357fdeb63e2cc6/src/example/old_behavior.hack#L33-L39

  // @typechecker-type UserID
  // @runtime-type     int (not validated)
  $user_id = $with_typespec_of->assertType(dict['string' => 1])['string'];

  // TypeAssert collapsed UserID to int and my invariance on UserID is silently violated.
  echo "Is my \$user_id a UserID?\n";
  \var_dump(\is_user_id($user_id as int));

What I would like to add

I know that this blows the encapsulation of us using TypeStructure<T> under the hood wide open.
This decision should not be taken lightly.

I suggest adding a $resolver parameter to every method that transforms a TypeStructure<T> into a TypeSpec<T>.
This allows the programmer to resolve newtypes for their own code.
This is a blessing when using huge typespecs with a newtype at one of the leafs.
This reduces the need for manual TypeSpec\dict(...)ing these structures.

Migration path

We could also make the default behavior of __Private\from_type_structure<T>() a little stricter.
Instead of silently downcasting to the runtime type, we could issue a warning, letting you know that your TypeSpec is not well-typed.

I'd personally like for an exception to be thrown, but this will be impossible to migrate to for larger consumers of this library.
I suggest adding a TypeSpec\throwing_resolver() which always throws to achieve this behavior on an opt-in, callsite-by-callsite basis.

Composition of resolvers

In my PoC, I assume you have one giant resolver, which resolves all the newtypes your project uses.
This will quickly become unmaintainable for larger projects.
There should be a way of composing resolvers to make this feature usable for large projects.
This could be achieved by returning null when the TypeStructure<T> could not be resolved and glueing resolvers together with the ?? operator, or via some other mechanism.

I left this out of my PoC, since it is not important to this RFC and we may just decide to not reveal TypeStructure<T> to the users of the library.

[jjergus]

This looks like a valid usecase to me, I agree we should support it somehow.

I'm not sure we can commit to a specific API right now -- from what I remember, TypeStructure was meant to be a "temporary" feature of the Hack language and might be replaced by something completely different eventually (although it's been there for a few years now so it's probably not happening any time soon). But if you're OK with a temporary API that might be changed or removed completely in the future, I wouldn't mind adding something like this.

Composition of resolvers (like you mentioned) also seems like an important feature, it should probably be supported from the get-go. In fact, the current implementation of from_type_structure() -- a giant switch statement -- could benefit from that architecture. If we replaced it with a loop that iterates over a list of resolvers stored somewhere, it would probably be an improvement, and it would be very easy to support adding custom resolvers to the list.

Going even further, we might not even need a special "resolver" abstraction, we could just add a static method to TypeSpec with the same purpose:

function from_type_structure<T>(TypeStructure<T> $ts): TypeSpec<T> {
  foreach (Config::TYPE_SPEC_CLASSES as $class) {
    $spec = $class::fromTypeStructure($ts);
    if ($spec is nonnull) {
      return $spec;
    }
  }
  throw new UnsupportedTypeException(...);
}

[lexidor]

I thought the big switch was made to make sure you were exhausting all the types, but then I saw that it has a default...
How would you add more specs to the Config::TYPE_SPEC_CLASSES constant?

[jjergus]

How would you add more specs to the Config::TYPE_SPEC_CLASSES constant?

Sorry that was a bad example, of course it can't be a constant, but probably some function that gets all the relevant lists of classes (built-in, custom, ...) and returns them all in the correct order.