Support multiple forms of inline suppression

hhast-lint.json

@@ -22,5 +22,8 @@
         "Facebook\\HHAST\\DataProviderTypesLinter"
       ]
     }
-  ]
+  ],
+  "suppressionAliases": {
+    "Facebook\\HHAST\\DontAwaitInALoopLinter": [ "This is a polling loop" ]
+  }
 }

src/Linters/BaseLinter.hack

@@ -11,7 +11,7 @@ namespace Facebook\HHAST;
 
 use namespace Facebook\TypeAssert;
 use type Facebook\HHAST\File;
-use namespace HH\Lib\{C, Str};
+use namespace HH\Lib\{C, Keyset, Str};
 
 <<__ConsistentConstruct>>
 abstract class BaseLinter {
@@ -27,6 +27,7 @@ abstract class BaseLinter {
   public function __construct(
     private File $file,
     private ?this::TConfig $config,
+    private keyset<string> $suppressionAliases,
   ) {
   }
 
@@ -57,7 +58,15 @@ abstract class BaseLinter {
     string $path,
     ?this::TConfig $config,
   ): this {
-    return new static(File::fromPath($path), $config);
+    return new static(File::fromPath($path), $config, keyset[]);
+  }
+
+  final public static function fromPathWithConfigAndSuppressionAliasses(
+    string $path,
+    ?this::TConfig $config,
+    keyset<string> $suppression_aliases,
+  ): this {
+    return new static(File::fromPath($path), $config, $suppression_aliases);
   }
 
   final public function getFile(): File {
@@ -81,14 +90,52 @@ abstract class BaseLinter {
     return 'HHAST_IGNORE_ALL['.$this->getLinterName().']';
   }
 
+  /**
+   * Provided for consistency with other (internal) linting frameworks.
+   * Identical to HHAST_IGNORE_ALL.
+   */
+  final public function getFrameworkAgnosticIgnoreAllMarker(): string {
+    return '@lint-ignore-all '.$this->getLinterName();
+  }
+
+  final public function getAllSuppressors(): keyset<string> {
+    return Keyset\union($this->defaultSuppressors(), $this->suppressionAliases);
+  }
+
+  final public function defaultSuppressors(): keyset<string> {
+    return keyset[
+      $this->getIgnoreSingleErrorMarker(),
+      $this->getFixmeMarker(),
+      $this->getFrameworkAgnosticFixmeMarker(),
+      $this->getShortIgnoreSingleMarker(),
+      $this->getFrameworkAgnosticIgnoreSingleMarker(),
+    ];
+  }
+
   /**
    * A user can choose to ignore a specific error reported by this linter
-   * using this string as a marker
+   * using this string as a marker. HHAST_IGNORE or @lint-ignore are recommended instead.
    */
   public function getIgnoreSingleErrorMarker(): string {
     return 'HHAST_IGNORE_ERROR['.$this->getLinterName().']';
   }
 
+  /**
+   * Identical to HHAST_IGNORE_ERROR, but doesn't suggest that
+   * the suppressed code is erroneous. Prefer this over HHAST_IGNORE_ERROR.
+   */
+  final public function getShortIgnoreSingleMarker(): string {
+    return 'HHAST_IGNORE['.$this->getLinterName().']';
+  }
+
+  /**
+   * Provided for consistency with other (internal) linting frameworks.
+   * Identical to HHAST_IGNORE.
+   */
+  final public function getFrameworkAgnosticIgnoreSingleMarker(): string {
+    return '@lint-ignore '.$this->getLinterName();
+  }
+
   /**
    * A user can choose to ignore a specific error reported by this linter
    * using this string as a marker.
@@ -100,16 +147,23 @@ abstract class BaseLinter {
     return 'HHAST_FIXME['.$this->getLinterName().']';
   }
 
+  /**
+   * Provided for consistency with other (internal) linting frameworks.
+   * Identical to HHAST_FIXME.
+   */
+  final public function getFrameworkAgnosticFixmeMarker(): string {
+    return '@lint-fixme '.$this->getLinterName();
+  }
+
   /**
    * Is this linter error disabled for the entire file?
    * Memoized since this should not change per run.
    */
   <<__Memoize>>
   public function isLinterSuppressedForFile(): bool {
-    return Str\contains(
-      $this->getFile()->getContents(),
-      $this->getIgnoreAllMarker(),
-    );
+    $contents = $this->getFile()->getContents();
+    return Str\contains($contents, $this->getIgnoreAllMarker()) ||
+      Str\contains($contents, $this->getFrameworkAgnosticIgnoreAllMarker());
   }
 
 }

src/Linters/LineLinter.hack

@@ -50,8 +50,10 @@ abstract class LineLinter<+Terror as LineLintError> extends BaseLinter {
   /** Check if this linter has been disabled by a comment on the previous line.
    */
   protected function isLinterSuppressed(string $previous_line): bool {
-    return Str\contains($previous_line, $this->getFixmeMarker()) ||
-      Str\contains($previous_line, $this->getIgnoreSingleErrorMarker());
+    return C\any(
+      $this->getAllSuppressors(),
+      $supp ==> Str\contains($previous_line, $supp),
+    );
   }
 
   /** Parse a single line of code and attempts to find lint errors */

src/Linters/suppress_ast_linter_error.hack

@@ -21,53 +21,48 @@ function is_linter_error_suppressed(
   Node $node,
   Script $root,
 ): bool {
-  $fixme = $linter->getFixmeMarker();
-  $ignore = $linter->getIgnoreSingleErrorMarker();
-
-  if (is_linter_suppressed_in_current_node($node, $fixme, $ignore)) {
+  $suppressors = $linter->getAllSuppressors();
+  if (is_linter_suppressed_in_current_node($node, $suppressors)) {
     return true;
   }
 
-  return is_linter_suppressed_in_sibling_node($node, $root, $fixme, $ignore) ||
-    is_linter_suppressed_up_to_statement($node, $root, $fixme, $ignore);
+  return is_linter_suppressed_in_sibling_node($node, $root, $suppressors) ||
+    is_linter_suppressed_up_to_statement($node, $root, $suppressors);
 }
 
 // Check the current token's leading trivia. For example a comment on the line before
 function is_linter_suppressed_in_current_node(
   Node $node,
-  string $fixme,
-  string $ignore,
+  keyset<string> $suppressors,
 ): bool {
   $token = $node->getFirstToken();
   if ($token === null) {
     return false;
   }
 
   $leading = $token->getLeading()->getCode();
-  return Str\contains($leading, $fixme) || Str\contains($leading, $ignore);
+  return C\any($suppressors, $supp ==> Str\contains($leading, $supp));
 }
 
 // Check sibling node as the comment might be attached there instead of on the current node
 function is_linter_suppressed_in_sibling_node(
   Node $node,
   Script $root,
-  string $fixme,
-  string $ignore,
+  keyset<string> $suppressors,
 ): bool {
   $token = $root->getPreviousToken($node->getFirstTokenx());
   if ($token === null) {
     return false;
   }
   $trailing = $token->getTrailing()->getCode();
-  return Str\contains($trailing, $fixme) || Str\contains($trailing, $ignore);
+  return C\any($suppressors, $supp ==> Str\contains($trailing, $supp));
 }
 
 // Walk up the parents and check the leading trivia until we hit a Statement type node.
 function is_linter_suppressed_up_to_statement(
   Node $node,
   Script $root,
-  string $fixme,
-  string $ignore,
+  keyset<string> $suppressors,
 ): bool {
   if ($node is IStatement) {
     return false;
@@ -91,6 +86,6 @@ function is_linter_suppressed_up_to_statement(
   if ($statement === null) {
     return false;
   }
-  return is_linter_suppressed_in_current_node($statement, $fixme, $ignore) ||
-    is_linter_suppressed_in_sibling_node($statement, $root, $fixme, $ignore);
+  return is_linter_suppressed_in_current_node($statement, $suppressors) ||
+    is_linter_suppressed_in_sibling_node($statement, $root, $suppressors);
 }

src/__Private/LSPLib/ServerState.hack

@@ -46,7 +46,7 @@ class ServerState {
       ServerStatus::INITIALIZING,
     ];
     while (C\contains_key($pre_init, $this->getStatus())) {
-      /* HHAST_IGNORE_ERROR[DontAwaitInALoop] */
+      // This is a polling loop
       await \HH\Asio\usleep(100 * 1000);
     }
   }

src/__Private/LintRun.hack

@@ -78,6 +78,7 @@ final class LintRun {
           $class,
           $file,
           $config->getLinterConfigForLinter($class),
+          $config->getSuppressionAliasesForLinter($class),
         );
         $result->v = self::worstResult($result->v, $this_result);
       } catch (LinterException $e) {
@@ -104,12 +105,13 @@ final class LintRun {
     classname<TLinter> $linter,
     File $file,
     ?TLinterConfig $linter_config,
+    keyset<string> $suppression_aliases,
   ): Awaitable<LintRunResult> where TLinterConfig = TLinter::TConfig {
     if (!$file->isHackFile() || !$linter::shouldLintFile($file)) {
       return LintRunResult::NO_ERRORS;
     }
 
-    $linter = new $linter($file, $linter_config);
+    $linter = new $linter($file, $linter_config, $suppression_aliases);
 
     if ($linter->isLinterSuppressedForFile()) {
       return LintRunResult::NO_ERRORS;

src/__Private/LintRunConfig.hack

@@ -54,6 +54,12 @@ final class LintRunConfig {
     // Each linter may specify a type for itself.
     // The type of this key is effectively `dict<classname<T1 ... Tn>, Tx>`
     ?'linterConfigs' => dict<string, BaseLinter::TConfig>,
+    // Maps FQN linter names to suppression aliases.
+    // "suppressionAliases": {
+    //   "Facebook\\HHAST\\DontAwaitInALoopLinter": [ "This is a polling loop" ]
+    // }
+    // Comments with "This is a polling loop" now act like "HHAST_FIXME[DontAwaitInALoop]" would.
+    ?'suppressionAliases' => dict<string, vec<string>>,
   );
 
   const type TFileConfig = shape(
@@ -266,6 +272,12 @@ final class LintRunConfig {
     }
   }
 
+  public function getSuppressionAliasesForLinter(
+    classname<BaseLinter> $classname,
+  ): keyset<string> {
+    return keyset($this->configFile['suppressionAliases'][$classname] ?? vec[]);
+  }
+
   private function getFullyQualifiedLinterName(string $name): string {
     if (Str\starts_with($name, 'Facebook\\HHAST\\Linters')) {
       $name = 'Facebook\\HHAST'.

tests/LinterTestTrait.hack

@@ -85,6 +85,14 @@ trait LinterTestTrait {
 
     $linter = $this->getLinter(__DIR__.'/examples/'.$example.'.in');
 
+    if ($linter->isLinterSuppressedForFile()) {
+      // @see tests/examples/NewLintErrorSuppressionMechanism/old_style_global.php.in
+      // We want to be able to explicitly suppress the linter under test.
+      $global_suppression = 'Linter was suppressed globally';
+      expect($global_suppression)->toMatchExpectFile($example.'.expect');
+      return;
+    }
+
     /*HHAST_FIXME[DontUseAsioJoin]*/
     $out = \HH\Asio\join($linter->getLintErrorsAsync())
       |> Vec\map($$, $error ==> self::getErrorAsShape($error))

tests/NewLintErrorSuppressionMechanismTest.hack

@@ -0,0 +1,76 @@
+/*
+ *  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;
+
+final class NewLintErrorSuppressionMechanismTest extends TestCase {
+  use LinterTestTrait;
+
+  protected function getLinter(string $file): BaseLinter {
+    return DontAwaitInALoopLinter::fromPathWithConfigAndSuppressionAliasses(
+      $file,
+      null,
+      keyset['This is a polling loop'],
+    );
+  }
+
+  public function getCleanExamples(): vec<(string)> {
+    return vec[
+      tuple(<<<'HACK'
+<?hh
+
+async function old_styles(): Awaitable<void> {
+  while(false) {
+    /* HHAST_IGNORE_ERROR[DontAwaitInALoop] */
+    await in_a_loop();
+    // HHAST_IGNORE_ERROR[DontAwaitInALoop]
+    await in_a_loop();
+    /* HHAST_FIXME[DontAwaitInALoop] */
+    await in_a_loop();
+    // HHAST_FIXME[DontAwaitInALoop]
+    await in_a_loop();
+  }
+}
+HACK
+      ),
+      tuple(<<<'HACK'
+<?hh
+
+async function new_styles(): Awaitable<void> {
+  while(false) {
+    /* HHAST_IGNORE[DontAwaitInALoop] */
+    await in_a_loop();
+    // HHAST_IGNORE[DontAwaitInALoop]
+    await in_a_loop();
+    /* @lint-ignore DontAwaitInALoop */
+    await in_a_loop();
+    // @lint-ignore DontAwaitInALoop
+    await in_a_loop();
+    /* @lint-fixme DontAwaitInALoop */
+    await in_a_loop();
+    // @lint-fixme DontAwaitInALoop
+    await in_a_loop();
+  }
+}
+HACK
+      ),
+      tuple(<<<'HACK'
+<?hh
+
+async function aliased(): Awaitable<void> {
+  while(false) {
+    // This is a polling loop
+    await in_a_loop();
+  }
+}
+HACK
+      ),
+    ];
+  }
+}

tests/examples/NewLintErrorSuppressionMechanism/new_global_style.php.expect

@@ -0,0 +1 @@
+Linter was suppressed globally

tests/examples/NewLintErrorSuppressionMechanism/new_global_style.php.in

@@ -0,0 +1,9 @@
+<?hh
+
+// @lint-ignore-all DontAwaitInALoop
+
+async function old_style(): Awaitable<void> {
+  while(false) {
+    await in_a_loop();
+  }
+}

tests/examples/NewLintErrorSuppressionMechanism/old_style_global.php.expect

@@ -0,0 +1 @@
+Linter was suppressed globally

tests/examples/NewLintErrorSuppressionMechanism/old_style_global.php.in

@@ -0,0 +1,9 @@
+<?hh
+
+// HHAST_IGNORE_ALL[DontAwaitInALoop]
+
+async function old_style(): Awaitable<void> {
+  while(false) {
+    await in_a_loop();
+  }
+}