From 4f69e93d08c9d60b9f3dcefab755fbf2fb079fd1 Mon Sep 17 00:00:00 2001 From: Lexidor Digital <31805625+lexidor@users.noreply.github.com> Date: Wed, 4 Dec 2019 19:16:56 +0100 Subject: [PATCH] Some general cleanup: (#46) * Some general cleanup: - AutoloadMap.hack This type should be in sync with HH\autoload_set_paths(), but this function has an incorrect hhi. I'll make a PR to hhvm, trying to get this fixed. - ConfigurationLoader.hack Remove an HH_FIXME by making self::fromData accept a wider type. Use the null constructor on ImmVector instead of a varray. - HHClientFallbackHandler.hack Require is allowed from inside a function. This might have been a parse error in the past. disallow_top_level_requires .hhconfig option availability suggests that this is now preferred. - Merger.hack Use a darray typehint where an array typehint was used. - Writer.hack Prefer inst_meth over calling a varray as a callable. - ComposerImporter.hack Use darray/varray typehints where appropriate. Stop relying on falsey array promotion. - FactParseScanner.hack Remove an HH_FIXME, since HH\facts_parse does have an HHI these days. Use ImmVector::toValuesArray() to prefer varray use. * Prefer darray(x) over x->toArray() --- src/AutoloadMap.hack | 13 +++++++++---- src/ConfigurationLoader.hack | 16 +++++----------- src/HHClientFallbackHandler.hack | 7 +++---- src/Merger.hack | 4 ++-- src/Writer.hack | 6 +++--- src/builders/ComposerImporter.hack | 6 ++++-- src/builders/FactParseScanner.hack | 4 +--- src/filters/BasePSRFilter.hack | 2 +- 8 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/AutoloadMap.hack b/src/AutoloadMap.hack index 11da8ad..3640083 100644 --- a/src/AutoloadMap.hack +++ b/src/AutoloadMap.hack @@ -12,10 +12,15 @@ namespace Facebook\AutoloadMap; /** The main shape of an autoload map. * * Must match `\HH\autoload_set_paths()` + * However, this is nolonger accurate. + * The parameter of autoload_set_paths is a + * `KeyedContainer>`. + * This does sadly not allow for the fallback key, + * which is a (function(string, string): bool). */ type AutoloadMap = shape( - 'class' => array, - 'function' => array, - 'type' => array, - 'constant' => array, + 'class' => darray, + 'function' => darray, + 'type' => darray, + 'constant' => darray, ); diff --git a/src/ConfigurationLoader.hack b/src/ConfigurationLoader.hack index 4411787..8f1e9d9 100644 --- a/src/ConfigurationLoader.hack +++ b/src/ConfigurationLoader.hack @@ -30,15 +30,11 @@ abstract final class ConfigurationLoader { public static function fromJSON(string $json, string $path): Config { $decoded = \json_decode($json, /* as array = */ true); invariant( - \is_array($decoded), + $decoded is KeyedContainer<_, _>, 'Expected configuration file to contain a JSON object, got %s', \gettype($decoded), ); - //hackfmt-ignore (else the comment applies to both arguments) - return self::fromData( - /* HH_IGNORE_ERROR[4110] */ $decoded, - $path - ); + return self::fromData($decoded, $path); } /** Load configuration from decoded data. @@ -46,7 +42,7 @@ abstract final class ConfigurationLoader { * @param $path arbitrary string - used to create clearer error messages */ public static function fromData( - array $data, + KeyedContainer $data, string $path, ): Config { $failure_handler = TypeAssert\is_nullable_string( @@ -62,8 +58,7 @@ abstract final class ConfigurationLoader { TypeAssert\is_nullable_array_of_strings( $data['devRoots'] ?? null, 'devRoots', - ) ?? - varray[], + ), ), 'autoloadFilesBehavior' => TypeAssert\is_nullable_enum( AutoloadFilesBehavior::class, @@ -85,8 +80,7 @@ abstract final class ConfigurationLoader { TypeAssert\is_nullable_array_of_strings( $data['extraFiles'] ?? null, 'extraFiles', - ) ?? - varray[], + ), ), 'parser' => TypeAssert\is_nullable_enum( Parser::class, diff --git a/src/HHClientFallbackHandler.hack b/src/HHClientFallbackHandler.hack index b1ad2b0..995cc81 100644 --- a/src/HHClientFallbackHandler.hack +++ b/src/HHClientFallbackHandler.hack @@ -95,7 +95,7 @@ class HHClientFallbackHandler extends FailureHandler { $this->map = $map; $map['failure'] = inst_meth($this, 'handleFailure'); \HH\autoload_set_paths( - /* HH_IGNORE_ERROR[4110] shape as array */ $map, + /* HH_IGNORE_ERROR[4110] incorrect hhi */ $map, Generated\root(), ); @@ -230,7 +230,7 @@ class HHClientFallbackHandler extends FailureHandler { return null; } - $data = \json_decode($last, /* arrays = */ true); + $data = \json_decode($last, /* assoc = */ true); if (!\is_array($data)) { return null; } @@ -247,7 +247,6 @@ class HHClientFallbackHandler extends FailureHandler { } private function requireFile(string $path): void { - /* HH_IGNORE_ERROR[1002] */ - require ($path); + require $path; } } diff --git a/src/Merger.hack b/src/Merger.hack index b94eb3b..377ed03 100644 --- a/src/Merger.hack +++ b/src/Merger.hack @@ -31,8 +31,8 @@ abstract final class Merger { } private static function mergeImpl( - Iterable> $maps, - ): array { + Traversable> $maps, + ): darray { $out = darray[]; foreach ($maps as $map) { foreach ($map as $def => $file) { diff --git a/src/Writer.hack b/src/Writer.hack index 60425d3..d962431 100644 --- a/src/Writer.hack +++ b/src/Writer.hack @@ -152,8 +152,8 @@ final class Writer { $add_failure_handler = \sprintf( "if (%s::isEnabled()) {\n". " \$handler = new %s();\n". - " \$map['failure'] = varray[\$handler, 'handleFailure'];\n". - " \HH\autoload_set_paths(/* HH_FIXME[4110] */ \$map, Generated\\root());\n". + " \$map['failure'] = inst_meth(\$handler, 'handleFailure');\n". + " \HH\autoload_set_paths(/* HH_FIXME[4110] incorrect hhi */ \$map, Generated\\root());\n". " \$handler->initialize();\n". "}", $failure_handler, @@ -236,7 +236,7 @@ function initialize(): void { _Private\bootstrap(); \$map = Generated\\map(); - \HH\autoload_set_paths(/* HH_FIXME[4110] */ \$map, Generated\\root()); + \HH\autoload_set_paths(/* HH_FIXME[4110] incorrect hhi */ \$map, Generated\\root()); foreach (\spl_autoload_functions() ?: varray[] as \$autoloader) { \spl_autoload_unregister(\$autoloader); } diff --git a/src/builders/ComposerImporter.hack b/src/builders/ComposerImporter.hack index 28a2644..b90b267 100644 --- a/src/builders/ComposerImporter.hack +++ b/src/builders/ComposerImporter.hack @@ -26,7 +26,7 @@ final class ComposerImporter implements Builder { } $this->root = \dirname($path); $composer_json = \file_get_contents($path); - $composer_config = \json_decode($composer_json, /* as array = */ true); + $composer_config = \json_decode($composer_json, /* assoc = */ true); $composer_autoload = idx($composer_config, 'autoload'); if ($composer_autoload === null) { return; @@ -145,13 +145,15 @@ final class ComposerImporter implements Builder { private static function normalizePSRRoots( array $roots, - ): array> { + ): darray> { $out = darray[]; foreach ($roots as $k => $v) { if ($v is string) { + $out[$k] ??= varray[]; $out[$k][] = $v; } else if (\is_array($v)) { foreach ($v as $w) { + $out[$k] ??= varray[]; $out[$k][] = $w; } } diff --git a/src/builders/FactParseScanner.hack b/src/builders/FactParseScanner.hack index 32a4945..5bcf918 100644 --- a/src/builders/FactParseScanner.hack +++ b/src/builders/FactParseScanner.hack @@ -105,11 +105,9 @@ final class FactParseScanner implements Builder { } public function getAutoloadMap(): AutoloadMap { - /* HH_FIXME[2049] no HHI for \HH\facts_parse */ - /* HH_FIXME[4107] no HHI for \HH|facts_parse */ $facts = \HH\facts_parse( $this->root, - $this->paths->toArray(), + $this->paths->toValuesArray(), /* force_hh = */ false, /* multithreaded = */ true, ); diff --git a/src/filters/BasePSRFilter.hack b/src/filters/BasePSRFilter.hack index 9830677..983de8d 100644 --- a/src/filters/BasePSRFilter.hack +++ b/src/filters/BasePSRFilter.hack @@ -69,7 +69,7 @@ abstract class BasePSRFilter implements Builder { ); return shape( - 'class' => $classes->toArray(), + 'class' => darray($classes), 'function' => darray[], 'type' => darray[], 'constant' => darray[],