[ RFC ] Add a typesafe infrastructure for DataProvider tests which use generics.

[lexidor]

Context

• https://github.com/hhvm/hsl/pull/132#pullrequestreview-437798802
• https://github.com/hhvm/hsl/pull/132#issuecomment-649805948

There is currently no typesafe way to write this kind of test (hhvm/hsl/tests/vec/VecTranformTest.php):

  public static function provideTestMap(): vec<mixed> {
    return vec[
      //...
      tuple(Vec\range(10, 15), $x ==> $x * 2, vec[20, 22, 24, 26, 28, 30]),
      tuple(varray['a'], $x ==> $x. ' buzz', vec['a buzz'],)
      //....
    ];
  }

  <<DataProvider('provideTestMap')>>
  public function testMap<Tv1, Tv2>(
    Traversable<Tv1> $traversable,
    (function(Tv1): Tv2) $value_func,
    vec<Tv2> $expected,
  ): void {
    expect(Vec\map($traversable, $value_func))->toEqual($expected);
  }

The real return type of provideTestMap is vec<(Traversable<T1>, (function(T): T2), T2)>, which Hack should infer as vec<(Traversable<arraykey>, (function(arraykey): arraykey), arraykey)>. However, the first method operates on ints, not strings, so putting the real return type in the method signature will cause a hh_client error. Vecs can't have a different (rebound) generic for each element, so we must provide a better way of providing the results, which can rebind the generics for each element.

I am not at all bound to this suggestion, but this could be something that works.

final class MyTestClass extends HackTest {
  <<TestBatchProvider>>
  public function provideTestMap(): vec<(function(): void)> {
    return vec[
      () ==> $this->testMap(Vec\range(10, 15), $x ==> $x * 2, vec[20, 22, 24, 26, 28, 30]),
      () ==> $this->testMap(varray['a'], $x ==> $x. ' buzz', vec['a buzz'])
    ];
  }

  public function testMap<Tv1, Tv2>(
    Traversable<Tv1> $traversable,
    (function(Tv1): Tv2) $value_func,
    vec<Tv2> $expected,
  ): void {
    expect(Vec\map($traversable, $value_func))->toEqual($expected);
  }
}

This is more clunky than returning tuples, but it pushes the typesafety into the Hack typesystem, instead of into a linter.

I'd be willing to work on this myself. One thing however, how do we prevent HackTest from still trying to execute testMap()?

[jjergus]

Intriguing proposal, thanks for writing it up!

I'll think about it some more, but some initial impressions:

• I'm not sure this can be described as 100% type safe. The problem I see is that nothing enforces that the body of each returned lambda is as expected (a call to $this->testMap(...)), so someone could accidentally put something else there (call to a different test function, or something completely different), resulting in an invalid test, confusing output, etc.
• This seems roughly equivalent to:

final class MyTestClass extends HackTest {
  public function testMap1(): void {
    $this->testMap(Vec\range(10, 15), $x ==> $x * 2, vec[20, 22, 24, 26, 28, 30]);
  }

  public function testMap2(): void {
    $this->testMap(varray['a'], $x ==> $x. ' buzz', vec['a buzz']);
  }

  public function testMap<Tv1, Tv2>(
    Traversable<Tv1> $traversable,
    (function(Tv1): Tv2) $value_func,
    vec<Tv2> $expected,
  ): void {
    expect(Vec\map($traversable, $value_func))->toEqual($expected);
  }
}

Is there a good case for using the proposed version instead of just doing something like this?

[lexidor]

That would work nicely too. How would this prevent you from calling $this->someOtherTestFunction() in testMap2(). Especially when there are multiple testcases in testMap2().

You could prevent people from making that mistake using __FUNCTION_CREDENTIAL__. HackTest would just have to assert that they are all the same.

final class MyTestClass extends HackTest {
  <<TestBatchProvider>>
  public function provideTestMap(): vec<(function(): FunctionCredential)> {
    return vec[
      () ==> $this->testMap(/*...*/),
      () ==> $this->testMap(/*...*/)
    ];
  }

  public function testMap<Tv1, Tv2>(/*...*/): FunctionCredential {
    expect(Vec\map($traversable, $value_func))->toEqual($expected);
    return __FUNCTION_CREDENTIAL__;
  }
}

[lexidor]

There is nothing meaningful that HackTest could do that a simple Vec\map_async() over the tests in provideTestMap() couldn't do. Closing...