Fix some shortcomings of UnusedParameterLinter

src/Linters/UnusedLambdaParameterLinter.hack

@@ -0,0 +1,69 @@
+/*
+ *  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 namespace HH\Lib\{C, Str};
+
+final class UnusedLambdaParameterLinter extends AutoFixingASTLinter {
+  const type TConfig = shape();
+  const type TContext = LambdaExpression;
+  const type TNode = VariableToken;
+
+  use __Private\SharedCodeForUnusedParameterLinters;
+
+  <<__Override>>
+  public function getLintErrorForNode(
+    this::TContext $lambda,
+    this::TNode $name_node,
+  ): ?ASTLintError {
+    if (
+      static::startsWithUnder($name_node) ||
+      !static::isAParameter($lambda, $name_node) ||
+      static::isUsedInBody($lambda->getBody(), $name_node)
+    ) {
+      return null;
+    }
+
+    return new ASTLintError(
+      $this,
+      'Parameter is unused',
+      $name_node,
+      () ==> static::prefixWithUnderscore($name_node),
+    );
+  }
+
+  <<__Override>>
+  public function getTitleForFix(ASTLintError $err): string {
+    $name = static::prefixWithUnderscore($err->getBlameNode() as this::TNode);
+    return Str\format('Rename to `%s`', $name->getText());
+  }
+
+  /**
+   * This linter targets VariableToken, so we get a lot of false requests.
+   * `$x ==> C\count($x)` calls us twice.
+   * Once for the parameter $x and once for `C\count($x)`s $x.
+   * We can't simply target ParameterDeclaration, since `$x ==> 0`
+   * does not introduce a paramter declaration.
+   */
+  private static function isAParameter(
+    this::TContext $lambda,
+    this::TNode $name_node,
+  ): bool {
+    $signature = $lambda->getSignature();
+    return ($signature is VariableToken && $signature === $name_node) ||
+      (
+        $signature is LambdaSignature &&
+        C\any(
+          $signature->getParameters()?->getChildrenOfItems() ?? vec[],
+          $p ==> static::extractVariableFromParam($p) === $name_node,
+        )
+      );
+  }
+}

src/Linters/UnusedParameterLinter.hack

@@ -11,64 +11,35 @@ namespace Facebook\HHAST;
 
 use namespace HH\Lib\Str;
 
+/**
+ * This linter should really be called UnusedFunctionAndMethodParameterLinter,
+ * but renaming this is a useless BC break.
+ */
 final class UnusedParameterLinter extends AutoFixingASTLinter {
   const type TConfig = shape();
-  const type TNode = ParameterDeclaration;
   const type TContext = IFunctionishDeclaration;
+  const type TNode = ParameterDeclaration;
+
+  use __Private\SharedCodeForUnusedParameterLinters;
 
   <<__Override>>
   public function getLintErrorForNode(
-    IFunctionishDeclaration $functionish,
-    ParameterDeclaration $node,
+    this::TContext $functionish,
+    this::TNode $node,
   ): ?ASTLintError {
-    if ($node->getVisibility() !== null) {
-      // Constructor parameter promotion
-      return null;
-    }
-
-    $name_node = $node->getName();
-    if (!$name_node is VariableToken) {
-      return null;
-    }
-
-    $name = $name_node->getText();
-    if (Str\starts_with($name, '$_')) {
-      return null;
-    }
-
-    // If this is a parameter of a lambda function, we should be looking in the
-    // lambda's body, not the enclosing function/method's body.
-    $lambda =
-      $functionish->getClosestAncestorOfDescendantOfType<LambdaExpression>(
-        $node,
-      );
-
-    if ($lambda is nonnull) {
-      $body = $lambda->getBody();
-    } else if ($functionish is FunctionDeclaration) {
-      $body = $functionish->getBody();
-    } else if ($functionish is MethodishDeclaration) {
-      $body = $functionish->getFunctionBody();
-    } else {
-      invariant_violation(
-        "Couldn't find functionish for parameter declaration",
-      );
-    }
+    $name_node = static::extractVariableFromParam($node);
+    $body = static::extractBodyFromFunctionish($functionish);
 
-    if ($body === null || $body is SemicolonToken) {
-      // Don't require `$_` for abstract or interface methods
+    if (
+      $name_node is null ||
+      $body is null ||
+      static::startsWithUnder($name_node) ||
+      static::belongsToALambda($functionish, $name_node) ||
+      static::isUsedInBody($body, $name_node)
+    ) {
       return null;
     }
 
-    foreach ($body->traverse() as $var) {
-      if (!$var is VariableToken) {
-        continue;
-      }
-      if ($var->getText() === $name) {
-        return null;
-      }
-    }
-
     return new ASTLintError(
       $this,
       'Parameter is unused',
@@ -80,20 +51,63 @@ final class UnusedParameterLinter extends AutoFixingASTLinter {
   public function getFixedNode(
     ParameterDeclaration $node,
   ): ParameterDeclaration {
-    $name = $node->getName();
-    if (!$name is VariableToken) {
-      return $node;
+    $fixed_var = static::fixParameterName($node);
+
+    $maybe_decorated = $node->getName();
+    if ($maybe_decorated is DecoratedExpression) {
+      $fixed_var = $maybe_decorated->withExpression($fixed_var);
     }
-    return $node->withName(
-      $name->withText('$_'.Str\strip_prefix($name->getText(), '$')),
-    );
+
+    return $node->withName($fixed_var);
   }
 
   <<__Override>>
   public function getTitleForFix(ASTLintError $err): string {
-    $name = ($err->getBlameNode() as this::TNode)->getName();
-    invariant($name is VariableToken, 'unhandled type');
-    $new_name = '$_'.Str\strip_prefix($name->getText(), '$');
-    return Str\format('Rename to `%s`', $new_name);
+    $name = static::fixParameterName(($err->getBlameNode() as this::TNode));
+    return Str\format('Rename to `%s`', $name->getText());
+  }
+
+  private static function fixParameterName(
+    ParameterDeclaration $p,
+  ): VariableToken {
+    $v = static::extractVariableFromParam($p);
+    invariant($v is nonnull, 'Must be nonnull, since we are requesting a fix');
+    return static::prefixWithUnderscore($v);
+  }
+
+  /**
+   * If this is a parameter of a lambda function, we stop linting.
+   * UnusedLambdaParameterLinter takes care of this.
+   * We can't handle it here, since the `$x ==> $x + 1` notation for
+   * lambda expressions don't have a ParameterDeclaration.
+   * This linter targets ParameterDeclaration.
+   * So if we handled it here, we would only process `($x) ==> $x + 1` notation.
+   */
+  private static function belongsToALambda(
+    this::TContext $f,
+    VariableToken $v,
+  ): bool {
+    return $f->getClosestAncestorOfDescendantOfType<LambdaExpression>($v)
+      is nonnull;
+  }
+
+  private static function extractBodyFromFunctionish(
+    this::TContext $functionish,
+  ): ?CompoundStatement {
+    if ($functionish is FunctionDeclaration) {
+      $body = $functionish->getBody();
+    } else if ($functionish is MethodishDeclaration) {
+      $body = $functionish->getFunctionBody();
+    } else {
+      invariant_violation(
+        'Unhandled functionish type: %s',
+        \get_class($functionish),
+      );
+    }
+
+    // If not a CompoundStatement, it is a `;`, which is an abstract
+    // or interface method. We don't require those parameters to have
+    // an underscore prefix.
+    return $body ?as CompoundStatement;
   }
 }

src/__Private/LSPLib/Server.hack

@@ -64,7 +64,7 @@ abstract class Server<TState as ServerState> {
 
     $was_response = await $this->tryHandleMessageTypeAsync(
       type_alias_structure(LSP\ResponseMessage::class),
-      async $x ==> {},
+      async $_ignored ==> {},
       $json,
     );
     if ($was_response) {

src/__Private/LintRunConfig.hack

@@ -75,6 +75,7 @@ final class LintRunConfig {
     HHAST\NoNewlineAtStartOfControlFlowBlockLinter::class,
     HHAST\MustUseOverrideAttributeLinter::class,
     HHAST\NoPHPEqualityLinter::class,
+    HHAST\UnusedLambdaParameterLinter::class,
     HHAST\UnusedParameterLinter::class,
     HHAST\UnusedVariableLinter::class,
     HHAST\UnusedUseClauseLinter::class,

src/__Private/SharedCodeForUnusedParameterLinters.hack

@@ -0,0 +1,78 @@
+/*
+ *  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\__Private;
+
+use namespace HH\Lib\{C, Str};
+use type Facebook\HHAST\{
+  DecoratedExpression,
+  IParameter,
+  Node,
+  ParameterDeclaration,
+  VariableToken,
+  VariadicParameter,
+};
+
+trait SharedCodeForUnusedParameterLinters {
+  final private static function extractVariableFromParam(
+    IParameter $p,
+  ): ?VariableToken {
+    // This isn't the modern `function _(mixed ... $collecting_vec): void {}` style.
+    // This is the old `function `function _(...): void {}` style.
+    if ($p is VariadicParameter) {
+      return null;
+    }
+
+    invariant(
+      $p is ParameterDeclaration,
+      'A new unknown parameter type was found: %s',
+      \get_class($p),
+    );
+
+    // `public function __construct(private string $x) {}` is never unused.
+    if ($p->getVisibility() is nonnull) {
+      return null;
+    }
+
+    $var = $p->getName();
+    if ($var is DecoratedExpression) {
+      $var = $var->getExpression();
+    }
+
+    invariant(
+      $var is VariableToken,
+      'Unexpected parameter type: %s did not yield a variable token',
+      \get_class($p),
+    );
+
+    return $var;
+  }
+
+  final private static function isUsedInBody(
+    Node $body,
+    VariableToken $var,
+  ): bool {
+    $search_text = $var->getText();
+    return C\any(
+      $body->traverse(),
+      $tok ==> $tok ?as VariableToken?->getText() === $search_text,
+    );
+  }
+
+  final private static function prefixWithUnderscore(
+    VariableToken $v,
+  ): VariableToken {
+    return $v->withText('$_'.Str\strip_prefix($v->getText(), '$'));
+  }
+
+  final private static function startsWithUnder(VariableToken $v): bool {
+    $name = $v->getText();
+    return Str\starts_with($name, '$_');
+  }
+}

tests/UnusedLambdaParameterLinterTest.hack

@@ -0,0 +1,34 @@
+/*
+ *  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 UnusedLambdaParameterLinterTest extends TestCase {
+  use AutoFixingLinterTestTrait<ASTLintError>;
+
+  <<__Override>>
+  protected function getLinter(string $file): UnusedLambdaParameterLinter {
+    return UnusedLambdaParameterLinter::fromPath($file);
+  }
+
+  <<__Override>>
+  public function getCleanExamples(): vec<(string)> {
+    return vec[
+      tuple('<?hh function _(): void {$x ==> $x + 1;}'),
+      tuple('<?hh function _(): void {($x) ==> $x + 1;}'),
+      tuple('<?hh function _(): void {(... $xs) ==> C\count($xs);}'),
+      tuple('<?hh function _(): void {$_unused ==> 0;}'),
+      tuple('<?hh function _(): void {($_unused) ==> 0;}'),
+      tuple('<?hh function _(): void {(... $_unused) ==> 0;}'),
+      tuple('<?hh function _(): void {$x ==> $y ==> $x + $y;}'),
+      tuple('<?hh function _(): void {$x ==> $_y ==> $z ==> $x + $z;}'),
+      tuple('<?hh function _(): void {$x ==> { return $x; };}'),
+    ];
+  }
+}

tests/UnusedParameterLinterTest.hack

@@ -35,6 +35,10 @@ final class UnusedParameterLinterTest extends TestCase {
       tuple(
         '<?hh class Foo { public function __construct(private int $bar) {} }',
       ),
+      tuple(
+        // Lambda's are no longer flagged by this linter
+        '<?hh function _(): void { ($a) ==> 0; (... $a) ==> 0; $a ==> 0; }',
+      ),
     ];
   }
 }

tests/examples/UnusedLambdaParameterLinter/function_param.php.autofix.expect

@@ -0,0 +1,13 @@
+<?hh
+
+function launder_lambda(
+  (function(nothing, nothing): mixed) $_any_lambda,
+): void {}
+
+function _void(): void {
+  launder_lambda(($bar, int $_baz) ==> $bar);
+  launder_lambda(($a, $b) ==> {
+    $_ = ($d1) ==> tuple($d1, $b);
+    $_ = $a ==> $a; // shadowed parameter, currently causes a false negative
+  });
+}

tests/examples/UnusedLambdaParameterLinter/function_param.php.expect

@@ -0,0 +1,7 @@
+[
+    {
+        "blame": "$baz",
+        "blame_pretty": "$baz",
+        "description": "Parameter is unused"
+    }
+]