Hhvm: Union types

[tslettebo]

(Apologies if this has been discussed before, I didn't find it with a search)

Our system has some functions which take or return parameters that may be one of several types, and likewise, there are built-in functions, like strpos that take or return multiple types.

Today, these have to be typed as taking or returning mixed.

Would it be a major change to the type system to allow the declaration of "union types", i.e. allowing more than a single type?

There are already such types in Hack, such as arraykey, which may be an int or string, or num, which may be an int or float.

A possible syntax could be to use "|", e.g.

function strpos(string $haystack, string | int $needle, int $offset = 0): int | bool;

There exists a proposal for this for PHP:

https://wiki.php.net/rfc/union_types

[jwatzman]

I don't know what the thinking of the current team is, but when I was working on the type system, I was super opposed to this. The union types as I saw them fell into three categories:

• unions of class types, which indicates some more interesting structure and should get an interface instead
• unions of primitives which made sense and should get their own name (int | float => num and string | int => arraykey are the only two, and even arraykey I was not super thrilled about)
• other unions of primitives which indicate a terribly broken API and shouldn't be encouraged/allowed -- this includes much of the PHP stdlib, which we were going to write wrappers for or otherwise rewrite, but that never really happened. (I wish it were easier to open-source some of FB's very nice wrappers)

[tslettebo]

Thanks for your reply, and I agree in general with what you write, in particular the first two bullet points.

For the third, there might be some valid reasons for an API to be designed this way. There's one example in particular in our system:

We have a database abstraction where you can pass in a list of fields to retrieve, and to save typing for the user, they can be passed in two ways:

1) No field aliases needed (string): "field1,field2,field3, ..."
2) Aliases needed (array): ["alias1" => "field1","alias2" => "field2","alias3" => "field3", ...]

If we were to only accept array, we'd have to write even the first kind as:

["field1","field2","field3", ...]

Admittedly, the difference from string isn't that big, but with lots of fields, it adds up.

An alternative could be to always send a string, and then parse it as alias or not.

I guess the API with most use for this kind of thing would be the standard library, as you said, but I agree: Better to clean up the API.

Feel free to close this issue.

[jwatzman]

The problem with APIs like that is that, even if you could write the proper return type, every callsite still has to do a case analysis to convince the typechecker that what its doing is safe, since the typechecker doesn't know about the convention. Splitting into two functions makes this convention explicit.

If that's unclear, let me give you an example from FB's code (glossing over a ton of details but the gist is correct). In order to load user profiles, we used to have a function Profile::load. Its parameter could either be a single ID, in which case the return value would be a single Profile; or its parameter could be an array of IDs, in which case the return value would be an array of Profiles. Before Hack came along, we'd do stuff like this:

// Single-id version:
$profile = await Profile::load($interesting_id);
$name = $profile->getName();

// Multi-id version:
$friend_profiles = await Profile::load($friend_ids);
$first_friend_name = $friend_profiles[0]->getName();

Hack can't express the return type of Profile::load as anything other than mixed -- but even if it could, if it could be properly typed as Profile | array<Profile>, _both of the above would still be type errors_. The typechecker would complain in the first case that you can't call getName on something that might be an array, and in the second case that you can't subscript something that might be a Profile. You'd have to do stuff like this:

$profile = await Profile::load($interesting_id);
invariant($profile instanceof Profile, 'This can never happen');
$name = $profile->getName();

// (Similarly asserting it's an array for the multi-id version)

At every callsite!

So instead of adding a union types feature to the typechecker, we realised it wouldn't help, and split Profile::load into Profile::loadSingle and Profile::loadMulti, which have the proper argument and return types. (And then killed Profile::loadMulti since we decided it tended to encourage efficiency anti-patterns, but that's unrelated to union types.)

Your aliases example would have the same problem.

I'm not on the team any more, so I can't say if this is going to happen or not, so I'll leave it open -- but my strong suspicion is that it won't happen :)

[tslettebo]

Thanks for your comprehensive reply and I agree with you.

[azjezz]

@jwatzman i'm sorry to bring up this old issue, i was thinking about union types today and was in my way to write a feature request for union types, wanted to check old issues about it.

i wanted to clear some things out :

unions of class types, which indicates some more interesting structure and should get an interface instead

yes, but in some cases you can't force third-party code to implement the same interface.

example, Sweet and Nuxed Container. both have ->get() and ->has() methods in their containers.
the difference is :
Sweet ( good ) :
->get<T>(typename<T> $service): T
->has<T>(typename<T> $service): bool
Nuxed ( bad ) :
->get(string $service): mixed
->has(string $service): bool

this means a union would take a string ( since typename is a supertype of string ) and return mixed.

if you want your library to be compatible with both, you can do :

use type Nuxed\Contract\Container\ContainerInterface as Nuxed;
use type Sweet\ServiceContainerInterface as Sweet;

type ServiceContainer = Nuxed | Sweet;

function foo(ServiceContainer $container): void {
  // type safe in both cases.
  // sweet->get(Bar::class) returns Bar.
  // nuxed->get(Bar::class) returns Bar but type hinted as `mixed`.
  // the `as` operator ensures that its a Bar instance in this case.
  $bar = $container->get(Bar::class) as Bar;
  // do what you want with $bar
}

unions of primitives which made sense and should get their own name (int | float => num and string | int => arraykey are the only two, and even arraykey I was not super thrilled about)

num and arraykey are more like general purpose union types, users would need unions in some areas, not just to make it "work", but to make things easier for other developers :

use namespace HH\Lib\Filesystem;
use namespace Nuxed\Io;

type Path = string | Io\Path | Filesystem\Path;
type File = Io\File | Filesystem\FileHandle | Path;

async function read(File $file, ?int $bytes = null): Awaitable<string> {
  if (!$file is Io\File) {
    if ($file is Filesystem\FileHandle) {
      // $path is Filesystem\Path
      $path = $file->getPath();
    } else {
      // $path is string | Io\Path | Filesystem\Path
      $path = $file;
    }

    // Io\Path::create(Stringish $path): Io\Path
    // both Io\Path and Filesystem\Path as stringish
    $file = new Io\File(Io\Path::create($path));
  }

  return await $file->read($bytes);
}

usage :

$content = await read('path/to/file.txt');
$file = new Io\File('path/to/file.txt');
$content = await read($file);
$path = new Io\Path('path/to/file.txt');
$content = await read($path);
$path = new Filesystem\Path('path/to/file');
$content = await read($path);

other unions of primitives which indicate a terribly broken API and shouldn't be encouraged/allowed -- this includes much of the PHP stdlib, which we were going to write wrappers for or otherwise rewrite, but that never really happened. (I wish it were easier to open-source some of FB's very nice wrappers)

i think what is worst than union type is the mixed type, which is basically a union of every existing type.
using union doesn't mean the API is broken, specially when type declaration is a thing.
as you can see in the example above, its clear that the read() function takes a file or its path as the first argument ( whether is a filesystem handle, a nuxed io file, path as a string, filesystem path, or nuxed io path ) the result is still the same at the end.

$profile = await Profile::load($interesting_id);
invariant($profile instanceof Profile, 'This can never happen');
$name = $profile->getName();

as can be used here instead of invariant() :

class Profile {
  public static async function load(
    int|vec<int> $ids
  ): Awaitable<Profile>|Awaitable<vec<Profile>> { ... }
}
...

$profile = await Profile::load(4) as Profile;
// since generics are not permitted with `as` operation
// type checker should be smart enough to tell that
// friends is `vec<Profile>` as its the only `vec<_>` possible return type.
$friends = await Profile::load($profile->getFriendsIds()) as vec<_>;

but this would require a special case in the typechecker + won't be able to check if return type is union of multiple <vec> with different value types.

Method constraints :

class Foo<T> {
  // calling bar() is invalid unless T is an int
  public function bar(): string where T as int { ... }
}

we can use a new syntax similar to method constraints with union types ( replacing as with is ) which will solve the Profile::load() issue :

type Ids = int | vec<int>;

class Profile {
  public static async function load<T as Ids>(
    T $ids
                             // v-- `is` instead of `as` 
  ): Awaitable<Profile> where T is int | Awaitable<vec<Profile>> {
    if ($ids is int) {
      return new Profile(getData($ids));
    }
    $awaitables = vec[];
    foreach($ids as $id) {
      $awaitables[] = static::load($id);
    }

    return await HH\Asio\v($awaitables);
  }
}

this tells the type checker that if T is int, Profile::load() will return Awaitable<Profile> and Awaitable<vec<Profile>> otherwise.

but this colser to function overloading or C++ template specialization.

usage :

$profile = await Profile::load(4); // $profile is Profile instance
$friends = await Profile::load($profile->getFriendsIds()); // $friends is vec<Profile>