Skip to content
This repository was archived by the owner on Sep 1, 2023. It is now read-only.

Commit 5a3f382

Browse files
authored
fix EnumSpec upper-bound-enforcement issues (#55)
`\HH\BuiltinEnum` is not a real type (honestly I'm not sure what it is), so this started failing once HHVM started enforcing upper bounds on generic arguments/return values. I'm not sure if this is 100% correct now (or if it was before), but I added some tests to verify that the return value from `$enumspec->assert(...)` still passes enum typehints, which is probably the important part.
1 parent f92d517 commit 5a3f382

File tree

5 files changed

+73
-14
lines changed

5 files changed

+73
-14
lines changed

src/TypeSpec.hack

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,7 @@ function dict<Tk as arraykey, Tv>(
7373
return new __Private\DictSpec($tsk, $tsv);
7474
}
7575

76-
function enum<
77-
Tinner as arraykey,
78-
T as /* HH_IGNORE_ERROR[2053] */ \HH\BuiltinEnum<Tinner>,
79-
>(classname<T> $what): TypeSpec<T> {
76+
function enum<T as arraykey>(\HH\enumname<T> $what): TypeSpec<T> {
8077
return new __Private\EnumSpec($what);
8178
}
8279

src/TypeSpec/__Private/EnumSpec.hack

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,15 @@ namespace Facebook\TypeSpec\__Private;
1313
use type Facebook\TypeAssert\{IncorrectTypeException, TypeCoercionException};
1414
use type Facebook\TypeSpec\TypeSpec;
1515

16-
final class
17-
EnumSpec<
18-
Tinner as arraykey,
19-
T as /* HH_IGNORE_ERROR[2053] */ \HH\BuiltinEnum<Tinner>,
20-
> extends TypeSpec<T> {
21-
public function __construct(private classname<T> $what) {
16+
final class EnumSpec<T as arraykey> extends TypeSpec<T> {
17+
18+
public function __construct(private \HH\enumname<T> $what) {
2219
}
2320

2421
<<__Override>>
2522
public function coerceType(mixed $value): T {
2623
$what = $this->what;
2724
try {
28-
/* HH_IGNORE_ERROR[4110] */
2925
return $what::assert($value);
3026
} catch (\UnexpectedValueException $_e) {
3127
throw TypeCoercionException::withValue($this->getTrace(), $what, $value);
@@ -36,7 +32,6 @@ final class
3632
public function assertType(mixed $value): T {
3733
$what = $this->what;
3834
try {
39-
/* HH_IGNORE_ERROR[4110] */
4035
return $what::assert($value);
4136
} catch (\UnexpectedValueException $_e) {
4237
throw IncorrectTypeException::withValue($this->getTrace(), $what, $value);

src/TypeSpec/__Private/from_type_structure.hack

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ function from_type_structure<T>(TypeStructure<T> $ts): TypeSpec<T> {
195195
throw new UnsupportedTypeException('OF_TRAIT');
196196
case TypeStructureKind::OF_ENUM:
197197
$enum = TypeAssert\not_null($ts['classname']);
198-
/* HH_IGNORE_ERROR[4323] */
198+
/* HH_IGNORE_ERROR[4110] */
199199
return new EnumSpec($enum);
200200
case TypeStructureKind::OF_NULL:
201201
/* HH_IGNORE_ERROR[4110] unsafe generics */

tests/EnumSpecTest.hack

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Copyright (c) 2016, Fred Emmott
3+
* Copyright (c) 2017-present, Facebook, Inc.
4+
* All rights reserved.
5+
*
6+
* This source code is licensed under the MIT license found in the
7+
* LICENSE file in the root directory of this source tree.
8+
*
9+
*/
10+
11+
namespace Facebook\TypeAssert;
12+
13+
use namespace Facebook\{TypeAssert, TypeSpec};
14+
use type Facebook\TypeAssert\TestFixtures\{ExampleEnum, TypeConstants};
15+
use type Facebook\TypeSpec\TypeSpec;
16+
use function Facebook\FBExpect\expect;
17+
18+
final class EnumSpecTest extends TypeSpecTest<ExampleEnum> {
19+
<<__Override>>
20+
public function getTypeSpec(): TypeSpec<ExampleEnum> {
21+
return TypeSpec\enum(ExampleEnum::class);
22+
}
23+
24+
<<__Override>>
25+
public function getValidCoercions(): vec<(mixed, ExampleEnum)> {
26+
return vec[
27+
tuple('herp', ExampleEnum::HERP),
28+
tuple(ExampleEnum::DERP, ExampleEnum::DERP),
29+
];
30+
}
31+
32+
<<__Override>>
33+
public function getInvalidCoercions(): vec<(mixed)> {
34+
return vec[
35+
tuple('analbumcover'),
36+
tuple(42),
37+
];
38+
}
39+
40+
<<__Override>>
41+
public function getToStringExamples(): vec<(TypeSpec<ExampleEnum>, string)> {
42+
$spec = TypeSpec\enum(ExampleEnum::class);
43+
return vec[
44+
tuple($spec, ExampleEnum::class),
45+
tuple($spec, 'Facebook\\TypeAssert\\TestFixtures\\ExampleEnum'),
46+
];
47+
}
48+
49+
/**
50+
* In this test we mostly care that all these possible ways of coercing an
51+
* enum value are accepted by the typechecker. They should also not cause
52+
* runtime errors, but that's already covered by other tests.
53+
*/
54+
public function testTypechecks(): void {
55+
self::takesEnum(TypeSpec\of<ExampleEnum>()->assertType('herp'));
56+
self::takesEnum(TypeSpec\enum(ExampleEnum::class)->assertType('herp'));
57+
self::takesEnum(TypeAssert\matches<ExampleEnum>('herp'));
58+
self::takesEnum(TypeAssert\matches_type_structure(
59+
type_structure(TypeConstants::class, 'TEnum'),
60+
'herp',
61+
));
62+
}
63+
64+
private static function takesEnum(ExampleEnum $value): void {
65+
expect($value is ExampleEnum)->toBeTrue();
66+
}
67+
}

tests/TypeStructureTest.hack

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ final class TypeStructureTest extends \Facebook\HackTest\HackTest {
426426
);
427427
}
428428

429-
const type TUnsupported = array<string>;
429+
const type TUnsupported = (function(): void);
430430
public function testUnsupportedType(): void {
431431
$ts = type_structure(self::class, 'TUnsupported');
432432

0 commit comments

Comments
 (0)