From caf5acd06f0ea3ee7840b6146ea1b89334b44b30 Mon Sep 17 00:00:00 2001 From: "Yang, Bo" Date: Thu, 9 Dec 2021 22:30:32 +0000 Subject: [PATCH 1/5] Allow HHClientLinterTestTrait to be used without LinterTestTrait --- tests/HHClientLinterIgnoreExceptTest.hack | 1 + tests/HHClientLinterIgnoreTest.hack | 1 + tests/HHClientLinterTest.hack | 1 + tests/HHClientLinterTestTrait.hack | 2 -- 4 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/HHClientLinterIgnoreExceptTest.hack b/tests/HHClientLinterIgnoreExceptTest.hack index d9bbfd60a..39c531730 100644 --- a/tests/HHClientLinterIgnoreExceptTest.hack +++ b/tests/HHClientLinterIgnoreExceptTest.hack @@ -10,6 +10,7 @@ namespace Facebook\HHAST; final class HHClientLinterIgnoreExceptTest extends TestCase { + use LinterTestTrait; use HHClientLinterTestTrait; <<__Override>> diff --git a/tests/HHClientLinterIgnoreTest.hack b/tests/HHClientLinterIgnoreTest.hack index 9968c1f08..db34f4a1c 100644 --- a/tests/HHClientLinterIgnoreTest.hack +++ b/tests/HHClientLinterIgnoreTest.hack @@ -10,6 +10,7 @@ namespace Facebook\HHAST; final class HHClientLinterIgnoreTest extends TestCase { + use LinterTestTrait; use HHClientLinterTestTrait; <<__Override>> diff --git a/tests/HHClientLinterTest.hack b/tests/HHClientLinterTest.hack index 71083086b..570165dbf 100644 --- a/tests/HHClientLinterTest.hack +++ b/tests/HHClientLinterTest.hack @@ -10,6 +10,7 @@ namespace Facebook\HHAST; final class HHClientLinterTest extends TestCase { + use LinterTestTrait; use HHClientLinterTestTrait; <<__Override>> diff --git a/tests/HHClientLinterTestTrait.hack b/tests/HHClientLinterTestTrait.hack index 94062548e..2a4b4ae13 100644 --- a/tests/HHClientLinterTestTrait.hack +++ b/tests/HHClientLinterTestTrait.hack @@ -12,7 +12,6 @@ namespace Facebook\HHAST; use namespace HH\Lib\{C, Str}; trait HHClientLinterTestTrait { - use LinterTestTrait; abstract const HHClientLinter::TConfig CONFIG; @@ -28,7 +27,6 @@ trait HHClientLinterTestTrait { __DIR__.'/../.var/tmp/hhast/'.C\lastx(Str\split(static::class, '\\')); } - <<__Override>> protected function getLinter(string $file): HHClientLinter { $ext = Str\strip_suffix($file, '.in') |> Str\ends_with($$, '.php') From bf19feeb80a5854fc5e97760f08b03ba7c6e71cb Mon Sep 17 00:00:00 2001 From: "Yang, Bo" Date: Thu, 9 Dec 2021 23:04:36 +0000 Subject: [PATCH 2/5] Add a test to detect duplicated lint errors in both HHClientLinter and other linters --- tests/HHClientDuplicatedLintErrorTest.hack | 46 ++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/HHClientDuplicatedLintErrorTest.hack diff --git a/tests/HHClientDuplicatedLintErrorTest.hack b/tests/HHClientDuplicatedLintErrorTest.hack new file mode 100644 index 000000000..3e4f6937a --- /dev/null +++ b/tests/HHClientDuplicatedLintErrorTest.hack @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2017-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +namespace Facebook\HHAST; + +use type Facebook\HackTest\DataProvider; +use namespace HH\Lib\{C, Str, Vec}; + +final class HHClientDuplicatedLintErrorTest extends TestCase { + use HHClientLinterTestTrait; + const HHClientLinter::TConfig CONFIG = shape(); + + public function getDirtyFixturesFromOtherLinters(): vec<(string)> { + return \glob(__DIR__.'/examples/*/*.in') + |> Vec\filter($$, $path ==> !Str\contains($path, 'HHClientLinter')) + |> Vec\map($$, $path ==> tuple($path)); + } + + <> + public async function testDuplicatedLintError(string $path): Awaitable { + $linter = $this->getLinter($path); + $errors = await $linter->getLintErrorsAsync(); + if (C\is_empty($errors)) { + return; + } + Vec\map( + $errors, + $error ==> Str\format( + "- %s: %s at line %s:%d:\n%s", + $error->getLintRule()->getName(), + $error->getDescription(), + $path, + $error->getRange()[0][1] ?? -1, + $error->getPrettyBlame() ?? '', + ), + ) + |> Str\join($$, "\n") + |> self::fail("Expected no errors, got:\n".$$); + } +} From 8851216bea254f026ad7e1aef9806c37b7cdcee5 Mon Sep 17 00:00:00 2001 From: "Yang, Bo" Date: Thu, 9 Dec 2021 23:18:17 +0000 Subject: [PATCH 3/5] Ignore any lint error detected by HHClientLinter that is not a duplicate --- .../aa_test_typechecks.hack.autofix.expect | 1 + .../examples/DataProviderTypesLinter/aa_test_typechecks.hack.in | 1 + .../namespace_private_type_annotation.php.in | 2 ++ .../type_error_thrown_on_autofix.php.autofix.expect | 1 + .../NoEmptyStatementsLinter/type_error_thrown_on_autofix.php.in | 1 + .../UnusedVariableLinter/unused_foreach.php.autofix.expect | 1 + tests/examples/UnusedVariableLinter/unused_foreach.php.in | 1 + 7 files changed, 8 insertions(+) diff --git a/tests/examples/DataProviderTypesLinter/aa_test_typechecks.hack.autofix.expect b/tests/examples/DataProviderTypesLinter/aa_test_typechecks.hack.autofix.expect index 3276a499d..5eff1af21 100644 --- a/tests/examples/DataProviderTypesLinter/aa_test_typechecks.hack.autofix.expect +++ b/tests/examples/DataProviderTypesLinter/aa_test_typechecks.hack.autofix.expect @@ -1,6 +1,7 @@ namespace Herp\Derp; /*HHAST_IGNORE_ALL[CamelCasedMethodsUnderscoredFunctions]*/ +// HHAST_IGNORE_ALL[5623] HHAST_IGNORE_ALL[5624] because the error code is not the interested in this example use type Facebook\HackTest\{DataProvider, HackTest}; use function Facebook\FBExpect\expect; diff --git a/tests/examples/DataProviderTypesLinter/aa_test_typechecks.hack.in b/tests/examples/DataProviderTypesLinter/aa_test_typechecks.hack.in index 3276a499d..5eff1af21 100644 --- a/tests/examples/DataProviderTypesLinter/aa_test_typechecks.hack.in +++ b/tests/examples/DataProviderTypesLinter/aa_test_typechecks.hack.in @@ -1,6 +1,7 @@ namespace Herp\Derp; /*HHAST_IGNORE_ALL[CamelCasedMethodsUnderscoredFunctions]*/ +// HHAST_IGNORE_ALL[5623] HHAST_IGNORE_ALL[5624] because the error code is not the interested in this example use type Facebook\HackTest\{DataProvider, HackTest}; use function Facebook\FBExpect\expect; diff --git a/tests/examples/NamespacePrivateLinter/namespace_private_type_annotation.php.in b/tests/examples/NamespacePrivateLinter/namespace_private_type_annotation.php.in index 7a3103c86..4782ed47c 100644 --- a/tests/examples/NamespacePrivateLinter/namespace_private_type_annotation.php.in +++ b/tests/examples/NamespacePrivateLinter/namespace_private_type_annotation.php.in @@ -8,6 +8,8 @@ */ namespace BlahNew; +// HHAST_IGNORE_ALL[5624] because the error code is not the interested in this example + final class Test { // using private type as type annotations function anotherTest( diff --git a/tests/examples/NoEmptyStatementsLinter/type_error_thrown_on_autofix.php.autofix.expect b/tests/examples/NoEmptyStatementsLinter/type_error_thrown_on_autofix.php.autofix.expect index ee8df831e..b02687d70 100644 --- a/tests/examples/NoEmptyStatementsLinter/type_error_thrown_on_autofix.php.autofix.expect +++ b/tests/examples/NoEmptyStatementsLinter/type_error_thrown_on_autofix.php.autofix.expect @@ -1,4 +1,5 @@ Date: Fri, 10 Dec 2021 21:09:20 +0000 Subject: [PATCH 4/5] Always ignore error code 0 because it's not a real lint error --- src/Linters/HHClientLinter.hack | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Linters/HHClientLinter.hack b/src/Linters/HHClientLinter.hack index bbff1669a..4fe097b7a 100644 --- a/src/Linters/HHClientLinter.hack +++ b/src/Linters/HHClientLinter.hack @@ -27,6 +27,7 @@ final class HHClientLinter implements Linter { * The error code that are always ignored */ const keyset ALWAYS_IGNORE_ERRORS = keyset[ + 0 /* InternalError, indicating a bug in the OCaml linter, not a real lint error */, 5583 /* DontAwaitInALoop, which should have been covered by DontAwaitInALoopLinter */, ]; From b94f654d0a91426eea5d46bf6ca203d7c91a4c58 Mon Sep 17 00:00:00 2001 From: "Yang, Bo" Date: Fri, 10 Dec 2021 21:16:11 +0000 Subject: [PATCH 5/5] Add some comment about the purpose of this test --- tests/HHClientDuplicatedLintErrorTest.hack | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/HHClientDuplicatedLintErrorTest.hack b/tests/HHClientDuplicatedLintErrorTest.hack index 3e4f6937a..1b68125e5 100644 --- a/tests/HHClientDuplicatedLintErrorTest.hack +++ b/tests/HHClientDuplicatedLintErrorTest.hack @@ -12,6 +12,18 @@ namespace Facebook\HHAST; use type Facebook\HackTest\DataProvider; use namespace HH\Lib\{C, Str, Vec}; +/** + * The test suite ensures that no lint error is found by `HHClientLinter` in + * the examples for `SingleRuleLinter`s. + * + * If an example triggers hh_client lint errors that indicate problems other + * than the `SingleRuleLinter` supposes to detect, add HHAST_IGNORE_ALL markers + * to suppress them. If an example triggers hh_client lint errors that + * indicate problems that are essentially the same as the `SingleRuleLinter` + * supposes to detect, it means the `SingleRuleLinter` is a duplicate of the + * hh_client lint error, and we should either disable the hh_client lint error + * code or remove the `SingleRuleLinter`. + */ final class HHClientDuplicatedLintErrorTest extends TestCase { use HHClientLinterTestTrait; const HHClientLinter::TConfig CONFIG = shape();