From 67308738a338978778b1585e4a40b2c60475f045 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 19:31:02 +0200 Subject: [PATCH 1/6] Lint clean --- src/http-exceptions/InternalServerErrorException.php | 4 ++-- src/http-exceptions/MethodNotAllowedException.php | 1 + src/http-exceptions/NotFoundException.php | 4 ++-- src/request-parameters/EnumRequestParameter.php | 3 ++- src/router/UnknownRouterException.php | 1 + src/uri-patterns/UriPattern.php | 8 ++++---- 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/http-exceptions/InternalServerErrorException.php b/src/http-exceptions/InternalServerErrorException.php index 96a1c16..5fb2870 100644 --- a/src/http-exceptions/InternalServerErrorException.php +++ b/src/http-exceptions/InternalServerErrorException.php @@ -10,5 +10,5 @@ namespace Facebook\HackRouter; -class InternalServerErrorException extends HttpException { -} +// HHAST_IGNORE_ERROR[FinalOrAbstractClass] @see UnknownRouterException. +class InternalServerErrorException extends HttpException {} diff --git a/src/http-exceptions/MethodNotAllowedException.php b/src/http-exceptions/MethodNotAllowedException.php index 45ce801..4873228 100644 --- a/src/http-exceptions/MethodNotAllowedException.php +++ b/src/http-exceptions/MethodNotAllowedException.php @@ -10,6 +10,7 @@ namespace Facebook\HackRouter; +// HHAST_IGNORE_ERROR[FinalOrAbstractClass] maybe extended outside this library. class MethodNotAllowedException extends HttpException { public function __construct( protected keyset $allowed, diff --git a/src/http-exceptions/NotFoundException.php b/src/http-exceptions/NotFoundException.php index 5292a68..3d3904c 100644 --- a/src/http-exceptions/NotFoundException.php +++ b/src/http-exceptions/NotFoundException.php @@ -10,5 +10,5 @@ namespace Facebook\HackRouter; -class NotFoundException extends HttpException { -} +// HHAST_IGNORE_ERROR[FinalOrAbstractClass] maybe extended outside this library. +class NotFoundException extends HttpException {} diff --git a/src/request-parameters/EnumRequestParameter.php b/src/request-parameters/EnumRequestParameter.php index 57f2da1..a2db346 100644 --- a/src/request-parameters/EnumRequestParameter.php +++ b/src/request-parameters/EnumRequestParameter.php @@ -12,6 +12,7 @@ use namespace HH\Lib\{Str, Vec}; +// HHAST_IGNORE_ERROR[FinalOrAbstractClass] maybe extended outside this library. class EnumRequestParameter extends TypedUriParameter { public function __construct( /* HH_FIXME[2053] */ @@ -42,7 +43,7 @@ public function assert(string $input): T { public function getRegExpFragment(): ?string { $class = $this->enumClass; $values = $class::getValues(); - $sub_fragments = Vec\map($values, $value ==> \preg_quote((string) $value)); + $sub_fragments = Vec\map($values, $value ==> \preg_quote((string)$value)); return '(?:'.Str\join($sub_fragments, '|').')'; } } diff --git a/src/router/UnknownRouterException.php b/src/router/UnknownRouterException.php index c950b90..427737c 100644 --- a/src/router/UnknownRouterException.php +++ b/src/router/UnknownRouterException.php @@ -10,6 +10,7 @@ namespace Facebook\HackRouter; +// HHAST_IGNORE_ERROR[FinalOrAbstractClass] maybe extended outside this library. class UnknownRouterException extends InternalServerErrorException { public function __construct(private vec $fastRouteData) { parent::__construct( diff --git a/src/uri-patterns/UriPattern.php b/src/uri-patterns/UriPattern.php index bb31416..e672b22 100644 --- a/src/uri-patterns/UriPattern.php +++ b/src/uri-patterns/UriPattern.php @@ -10,8 +10,8 @@ namespace Facebook\HackRouter; -// Non-final so you can extend it with additional convenience -// methods. +// Non-final so you can extend it with additional convenience methods. +// HHAST_IGNORE_ERROR[FinalOrAbstractClass] message above. class UriPattern implements HasFastRouteFragment { private Vector $parts = Vector {}; @@ -31,8 +31,8 @@ final public function getParts(): ImmVector { final public function getParameters(): ImmVector { $out = Vector {}; - foreach($this->parts as $part) { - if($part is UriParameter) { + foreach ($this->parts as $part) { + if ($part is UriParameter) { $out->add($part); } } From 86ebb8ac90ac109f0c5d443da435db954bb94c30 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 19:42:01 +0200 Subject: [PATCH 2/6] Remove HH_(FIXME|IGNORE_ERROR) directives --- src/request-parameters/EnumRequestParameter.php | 6 ++---- src/request-parameters/RequestParameterGetters.php | 9 +++------ src/request-parameters/RequestParametersBase.php | 8 +++----- src/uri-patterns/UriBuilderBase.php | 5 ++++- src/uri-patterns/UriBuilderSetters.php | 2 +- src/uri-patterns/UriPattern.php | 3 +-- 6 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/request-parameters/EnumRequestParameter.php b/src/request-parameters/EnumRequestParameter.php index a2db346..674c49a 100644 --- a/src/request-parameters/EnumRequestParameter.php +++ b/src/request-parameters/EnumRequestParameter.php @@ -15,15 +15,13 @@ // HHAST_IGNORE_ERROR[FinalOrAbstractClass] maybe extended outside this library. class EnumRequestParameter extends TypedUriParameter { public function __construct( - /* HH_FIXME[2053] */ - private classname<\HH\BuiltinEnum> $enumClass, + private \HH\enumname $enumClass, string $name, ) { parent::__construct($name); } - /* HH_FIXME[2053] */ - final public function getEnumName(): classname<\HH\BuiltinEnum> { + final public function getEnumName(): \HH\enumname { return $this->enumClass; } diff --git a/src/request-parameters/RequestParameterGetters.php b/src/request-parameters/RequestParameterGetters.php index 3eaaf1b..9462d13 100644 --- a/src/request-parameters/RequestParameterGetters.php +++ b/src/request-parameters/RequestParameterGetters.php @@ -32,8 +32,7 @@ final public function getOptionalInt(string $name): ?int { } final public function getEnum( - /* HH_FIXME[2053] */ - classname<\HH\BuiltinEnum> $class, + \HH\enumname $class, string $name, ): TValue { $value = $this->getEnumImpl( @@ -45,8 +44,7 @@ classname<\HH\BuiltinEnum> $class, } final public function getOptionalEnum( - /* HH_FIXME[2053] */ - classname<\HH\BuiltinEnum> $class, + \HH\enumname $class, string $name, ): ?TValue { return $this->getEnumImpl( @@ -58,8 +56,7 @@ classname<\HH\BuiltinEnum> $class, final private function getEnumImpl( EnumRequestParameter $spec, - /* HH_FIXME[2053] */ - classname<\HH\BuiltinEnum> $class, + \HH\enumname $class, string $name, ): ?TValue { invariant( diff --git a/src/request-parameters/RequestParametersBase.php b/src/request-parameters/RequestParametersBase.php index e4c7e5c..4bdfcb5 100644 --- a/src/request-parameters/RequestParametersBase.php +++ b/src/request-parameters/RequestParametersBase.php @@ -24,9 +24,8 @@ public function __construct( KeyedTraversable $values, ) { $this->values = new ImmMap($values); - $spec_vector_to_map = ( - Traversable $specs - ) ==> Dict\pull($specs, $it ==> $it, $it ==> $it->getName()); + $spec_vector_to_map = (Traversable $specs) ==> + Dict\pull($specs, $it ==> $it, $it ==> $it->getName()); $this->requiredSpecs = $spec_vector_to_map($required_specs); $this->optionalSpecs = $spec_vector_to_map($optional_specs); @@ -63,14 +62,13 @@ classname $class, ): T { $spec = $specs[$name]; invariant( - /* HH_FIXME[4162] need reified generics */ \is_a($spec, $class), 'Expected %s to be a %s, got %s', $name, $class, \get_class($spec), ); - return /* HH_FIXME[4110] */ $spec; + return \HH\FIXME\UNSAFE_CAST($spec, 'is_a($spec, $class) ~= $spec is T'); } final protected function getSimpleTyped( diff --git a/src/uri-patterns/UriBuilderBase.php b/src/uri-patterns/UriBuilderBase.php index 928863c..a69f8b7 100644 --- a/src/uri-patterns/UriBuilderBase.php +++ b/src/uri-patterns/UriBuilderBase.php @@ -80,12 +80,15 @@ classname> $parameter_type, $parameter_type, \get_class($part), ); + $part = \HH\FIXME\UNSAFE_CAST>( + $part, + 'is_a($part, $parameter_type) ~= $part is TypedUriParameter', + ); invariant( !$this->values->containsKey($name), 'trying to set %s twice', $name, ); - /* HH_FIXME[4053] need reified generics */ $this->values[$name] = $part->getUriFragment($value); return $this; } diff --git a/src/uri-patterns/UriBuilderSetters.php b/src/uri-patterns/UriBuilderSetters.php index 24790f7..072e582 100644 --- a/src/uri-patterns/UriBuilderSetters.php +++ b/src/uri-patterns/UriBuilderSetters.php @@ -22,7 +22,7 @@ final public function setInt(string $name, int $value): this { } final public function setEnum( - /* HH_FIXME[2053] */ classname<\HH\BuiltinEnum> $class, + \HH\enumname $class, string $name, T $value, ): this { diff --git a/src/uri-patterns/UriPattern.php b/src/uri-patterns/UriPattern.php index e672b22..72832da 100644 --- a/src/uri-patterns/UriPattern.php +++ b/src/uri-patterns/UriPattern.php @@ -68,8 +68,7 @@ final public function int(string $name): this { } final public function enum( - /* HH_FIXME[2053] \HH\BuiltinEnum is an implementation detail */ - classname<\HH\BuiltinEnum> $enum_class, + \HH\enumname $enum_class, string $name, ): this { return $this->appendPart(new EnumRequestParameter($enum_class, $name)); From cb0b6b8f0df24f79e150fb87e9d8a1b445ee5488 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 20:30:31 +0200 Subject: [PATCH 3/6] Make an explicit enum request parameter The type inference engine rebinds the T when it pleases. The <<__Explicit>> annotation should prevent this programming mistake. --- src/request-parameters/EnumRequestParameter.php | 7 ++++++- .../EnumRequestParameterExplicit.php | 14 ++++++++++++++ tests/RequestParametersTest.php | 17 ++++++++++++++++- 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 src/request-parameters/EnumRequestParameterExplicit.php diff --git a/src/request-parameters/EnumRequestParameter.php b/src/request-parameters/EnumRequestParameter.php index 674c49a..9ade536 100644 --- a/src/request-parameters/EnumRequestParameter.php +++ b/src/request-parameters/EnumRequestParameter.php @@ -12,7 +12,12 @@ use namespace HH\Lib\{Str, Vec}; -// HHAST_IGNORE_ERROR[FinalOrAbstractClass] maybe extended outside this library. +/** + * WARNING: hh_client will not infer a correct enum type when given `SomeEnum::class`. + * @see RequestParametersTest::testInvalidEnumParamToUri() (grep for @ref) + * `EnumRequestParameterExplicit` is typesafe, so you can use that instead. + */ +// HHAST_IGNORE_ERROR[FinalOrAbstractClass] comment above. class EnumRequestParameter extends TypedUriParameter { public function __construct( private \HH\enumname $enumClass, diff --git a/src/request-parameters/EnumRequestParameterExplicit.php b/src/request-parameters/EnumRequestParameterExplicit.php new file mode 100644 index 0000000..c22b7b9 --- /dev/null +++ b/src/request-parameters/EnumRequestParameterExplicit.php @@ -0,0 +1,14 @@ +> T> + extends EnumRequestParameter {} diff --git a/tests/RequestParametersTest.php b/tests/RequestParametersTest.php index f2b668c..4e33fa2 100644 --- a/tests/RequestParametersTest.php +++ b/tests/RequestParametersTest.php @@ -62,14 +62,29 @@ public function testEnumParamToUri(): void { ); } + // @ref EnumRequestParameter public function testInvalidEnumParamToUri(): void { expect(() ==> { $part = (new EnumRequestParameter(TestIntEnum::class, 'foo')); - /* HH_IGNORE_ERROR[4110] intentionally bad type */ + // hh_client expands the inferred generic here to include TestStringEnum. + // This used to be a type error in a previous version of hhvm! $_throws = $part->getUriFragment(TestStringEnum::BAR); })->toThrow(\UnexpectedValueException::class); } + public function testInvalidEnumExplicitParamToUri(): void { + expect(() ==> { + $part = ( + new EnumRequestParameterExplicit(TestIntEnum::class, 'foo') + ); + $value = \HH\FIXME\UNSAFE_CAST( + TestStringEnum::BAR, + 'Telling lies for the purpose of testing.', + ); + $_throws = $part->getUriFragment($value); + })->toThrow(\UnexpectedValueException::class); + } + public function testFromPattern(): void { $parts = (new UriPattern()) ->literal('/') From 1577d3eae6b03be95c2841f489f25680c4c0c326 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 20:36:56 +0200 Subject: [PATCH 4/6] Make the examples runnable without top-level code --- README.md | 4 +-- examples/RunBaseRouterExample.php | 30 +++++++++++++++++++++ examples/RunUriPatternsExample.php | 30 +++++++++++++++++++++ examples/{ => lib}/BaseRouterExample.php | 27 +++---------------- examples/{ => lib}/UriPatternsExample.php | 33 ++++------------------- 5 files changed, 70 insertions(+), 54 deletions(-) create mode 100644 examples/RunBaseRouterExample.php create mode 100644 examples/RunUriPatternsExample.php rename examples/{ => lib}/BaseRouterExample.php (65%) rename examples/{ => lib}/UriPatternsExample.php (78%) diff --git a/README.md b/README.md index ff06e4a..2e0ac00 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ final class BaseRouterExample extends BaseRouter { ``` Simplified for conciseness - see -[`examples/BaseRouterExample.php`](examples/BaseRouterExample.php) for +[`examples/RunBaseRouterExample.php`](examples/RunBaseRouterExample.php) for full executable example. UriPatterns @@ -87,7 +87,7 @@ $link = UserPageController::getUriBuilder() ``` These examples are simplified for conciseness - see -[`examples/UriPatternsExample.php`](examples/UriPatternsExample.php) for +[`examples/RunUriPatternsExample.php`](examples/RunUriPatternsExample.php) for full executable example. Codegen diff --git a/examples/RunBaseRouterExample.php b/examples/RunBaseRouterExample.php new file mode 100644 index 0000000..2be6b60 --- /dev/null +++ b/examples/RunBaseRouterExample.php @@ -0,0 +1,30 @@ +> +function main(): noreturn { + require_once __DIR__.'/../vendor/autoload.hack'; + \Facebook\AutoloadMap\initialize(); + + $router = new BaseRouterExample(); + foreach (get_example_inputs() as $input) { + list($method, $path) = $input; + + list($responder, $params) = $router->routeMethodAndPath($method, $path); + \printf("%s %s\n\t%s\n", $method, $path, $responder(dict($params))); + } + exit(0); +} diff --git a/examples/RunUriPatternsExample.php b/examples/RunUriPatternsExample.php new file mode 100644 index 0000000..5904e12 --- /dev/null +++ b/examples/RunUriPatternsExample.php @@ -0,0 +1,30 @@ +> +function main(): void { + require_once __DIR__.'/../vendor/autoload.hack'; + \Facebook\AutoloadMap\initialize(); + + $router = new UriPatternsExample(); + foreach (get_example_paths() as $path) { + list($controller, $params) = + $router->routeMethodAndPath(HttpMethod::GET, $path); + \printf("GET %s\n\t%s\n", $path, (new $controller($params))->getResponse()); + } +} diff --git a/examples/BaseRouterExample.php b/examples/lib/BaseRouterExample.php similarity index 65% rename from examples/BaseRouterExample.php rename to examples/lib/BaseRouterExample.php index 2fd7aa4..f8fb7e1 100644 --- a/examples/BaseRouterExample.php +++ b/examples/lib/BaseRouterExample.php @@ -14,15 +14,13 @@ namespace Facebook\HackRouter\Examples\BaseRouterExample; -require_once(__DIR__.'/../vendor/hh_autoload.php'); - use type Facebook\HackRouter\{BaseRouter, HttpMethod}; /** This can be whatever you want; in this case, it's a * callable, but classname is also a * common choice. */ -type TResponder = (function(dict):string); +type TResponder = (function(dict): string); final class BaseRouterExample extends BaseRouter { <<__Override>> @@ -30,10 +28,8 @@ protected function getRoutes( ): ImmMap> { return ImmMap { HttpMethod::GET => ImmMap { - '/' => - ($_params) ==> 'Hello, world', - '/user/{user_name}' => - ($params) ==> 'Hello, '.$params['user_name'], + '/' => ($_params) ==> 'Hello, world', + '/user/{user_name}' => ($params) ==> 'Hello, '.$params['user_name'], }, HttpMethod::POST => ImmMap { '/' => ($_params) ==> 'Hello, POST world', @@ -50,20 +46,3 @@ function get_example_inputs(): ImmVector<(HttpMethod, string)> { tuple(HttpMethod::POST, '/'), }; } - -<<__EntryPoint>> -function main(): noreturn { - $router = new BaseRouterExample(); - foreach (get_example_inputs() as $input) { - list($method, $path) = $input; - - list($responder, $params) = $router->routeMethodAndPath($method, $path); - \printf( - "%s %s\n\t%s\n", - $method, - $path, - $responder(dict($params)), - ); - } - exit(0); -} diff --git a/examples/UriPatternsExample.php b/examples/lib/UriPatternsExample.php similarity index 78% rename from examples/UriPatternsExample.php rename to examples/lib/UriPatternsExample.php index cb38985..772ba57 100644 --- a/examples/UriPatternsExample.php +++ b/examples/lib/UriPatternsExample.php @@ -14,16 +14,14 @@ namespace Facebook\HackRouter\Examples\UrlPatternsExample; -require_once(__DIR__.'/../vendor/hh_autoload.php'); - -use type Facebook\HackRouter\{ +use type Facebook\HackRouter\{ BaseRouter, GetFastRoutePatternFromUriPattern, GetUriBuilderFromUriPattern, HasUriPattern, HttpMethod, RequestParameters, - UriPattern + UriPattern, }; <<__ConsistentConstruct>> @@ -38,12 +36,10 @@ final protected function getRequestParameters(): RequestParameters { return $this->uriParameters; } - public function __construct( - ImmMap $uri_parameter_values, - ) { + public function __construct(ImmMap $uri_parameter_values) { $this->uriParameters = new RequestParameters( static::getUriPattern()->getParameters(), - ImmVector { }, + ImmVector {}, $uri_parameter_values, ); } @@ -86,8 +82,7 @@ public static function getControllers(): ImmVector { } <<__Override>> - public function getRoutes( - ): ImmMap> { + public function getRoutes(): ImmMap> { $urls_to_controllers = dict[]; foreach (self::getControllers() as $controller) { $pattern = $controller::getFastRoutePattern(); @@ -107,21 +102,3 @@ function get_example_paths(): ImmVector { ->getPath(), }; } - -function main(): void { - $router = new UriPatternsExample(); - foreach (get_example_paths() as $path) { - list($controller, $params) = $router->routeMethodAndPath( - HttpMethod::GET, - $path, - ); - \printf( - "GET %s\n\t%s\n", - $path, - (new $controller($params))->getResponse(), - ); - } -} - -/* HH_IGNORE_ERROR[1002] top-level statement in strict file */ -main(); From 2c438058f680195ec177c19dad99c6d83d598767 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 20:40:00 +0200 Subject: [PATCH 5/6] Run CI on lts versions and ubuntu 20.04 --- .github/workflows/build-and-test.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 1d2c578..4baa377 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -11,12 +11,12 @@ jobs: # Run tests on all OS's and HHVM versions, even if one fails fail-fast: false matrix: - os: [ ubuntu ] + os: [ ubuntu-20.04 ] hhvm: - '4.128' - - latest - - nightly - runs-on: ${{matrix.os}}-latest + - '4.153' + - '4.168' + runs-on: ${{matrix.os}} steps: - uses: actions/checkout@v2 - name: Create branch for version alias From 06843a49c7abae786f86349e3c3b63df4b8e2cf8 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Thu, 25 May 2023 20:41:30 +0200 Subject: [PATCH 6/6] Bump minimum hhvm version to 4.153 for unsafe casts --- .github/workflows/build-and-test.yml | 1 - composer.json | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 4baa377..6c8dc42 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -13,7 +13,6 @@ jobs: matrix: os: [ ubuntu-20.04 ] hhvm: - - '4.128' - '4.153' - '4.168' runs-on: ${{matrix.os}} diff --git a/composer.json b/composer.json index 22e0966..2c1ce1e 100644 --- a/composer.json +++ b/composer.json @@ -5,7 +5,7 @@ "keywords": ["hack", "router", "routing", "hhvm"], "homepage": "https://github.com/hhvm/hack-router", "require": { - "hhvm": "^4.128", + "hhvm": "^4.153", "facebook/hack-http-request-response-interfaces": "^0.2|^0.3" }, "extra": {