Enforce closure parameter type hints (#57)

Author: simbig

CHANGELOG.md

@@ -9,6 +9,12 @@ See [GitHub releases](https://github.com/mll-lab/php-utils/releases).
 
 ## Unreleased
 
+## v5.19.0
+
+### Added
+
+- Add PHPStan-Rule `MLL\Utils\PHPStan\Rules\MissingClosureParameterTypehintRule.php`
+
 ## v5.18.0
 
 ### Added

phpstan.neon

@@ -1,6 +1,10 @@
 includes:
 - extension.neon
 - rules.neon
+rules:
+#- MLL\Utils\PHPStan\Rules\ThrowableClassNameRule
+- MLL\Utils\PHPStan\Rules\VariableNameIdToIDRule
+- MLL\Utils\PHPStan\Rules\MissingClosureParameterTypehintRule
 parameters:
   level: max
   paths:

rules.neon

@@ -2,6 +2,7 @@ rules:
 # TODO gradually enable those rules
 #- MLL\Utils\PHPStan\Rules\ThrowableClassNameRule
 #- MLL\Utils\PHPStan\Rules\VariableNameIdToIDRule
+#- MLL\Utils\PHPStan\Rules\MissingClosureParameterTypehintRule
 parameters:
   # https://github.com/spaze/phpstan-disallowed-calls/blob/main/docs/custom-rules.md
   disallowedFunctionCalls:

src/IlluminaSampleSheet/V1/DataSection.php

@@ -55,7 +55,7 @@ protected function validateDuplicatedSampleIDs(): void
             ->groupBy(fn (Row $row): string => $row->sampleID);
 
         $duplicates = $groups
-            ->filter(fn ($group): bool => count($group) > 1)
+            ->filter(fn (Collection $group): bool => count($group) > 1)
             ->keys();
         $duplicateIDsAsString = $duplicates->implode(', ');
 

src/Microplate/AbstractMicroplate.php

@@ -49,7 +49,7 @@ public function sortedWells(FlowDirection $flowDirection): Collection
     {
         return $this->wells()->sortBy(
             /** @param TWell $content */
-            function ($content, string $key) use ($flowDirection): string {
+            function ($content, string $key) use ($flowDirection): string { /** @phpstan-ignore missingType.parameter (is in template context) */
                 switch ($flowDirection->value) {
                     case FlowDirection::ROW:
                         return $key;
@@ -72,7 +72,7 @@ public function freeWells(): Collection
     {
         return $this->wells()->filter(
             /** @param TWell $content */
-            static fn ($content): bool => $content === self::EMPTY_WELL
+            static fn ($content): bool => $content === self::EMPTY_WELL /** @phpstan-ignore missingType.parameter (is in template context) */
         );
     }
 
@@ -81,14 +81,14 @@ public function filledWells(): Collection
     {
         return $this->wells()->filter(
             /** @param TWell $content */
-            static fn ($content): bool => $content !== self::EMPTY_WELL
+            static fn ($content): bool => $content !== self::EMPTY_WELL /** @phpstan-ignore missingType.parameter (is in template context) */
         );
     }
 
     /** @return callable(TWell|null $content, string $coordinatesString): bool */
     public function matchRow(string $row): callable
     {
-        return function ($content, string $coordinatesString) use ($row): bool {
+        return function ($content, string $coordinatesString) use ($row): bool { /** @phpstan-ignore missingType.parameter (is in template context) */
             $coordinates = Coordinates::fromString($coordinatesString, $this->coordinateSystem);
 
             return $coordinates->row === $row;
@@ -98,7 +98,7 @@ public function matchRow(string $row): callable
     /** @return callable(TWell|null $content, string $coordinatesString): bool */
     public function matchColumn(int $column): callable
     {
-        return function ($content, string $coordinatesString) use ($column): bool {
+        return function ($content, string $coordinatesString) use ($column): bool { /** @phpstan-ignore missingType.parameter (is in template context) */
             $coordinates = Coordinates::fromString($coordinatesString, $this->coordinateSystem);
 
             return $coordinates->column === $column;
@@ -119,7 +119,7 @@ public function toWellWithCoordinateMapper(): callable
     public function toWellWithCoordinatesMapper(): callable
     {
         // @phpstan-ignore return.type (generic not inferred)
-        return fn ($content, string $coordinatesString): WellWithCoordinates => new WellWithCoordinates(
+        return fn ($content, string $coordinatesString): WellWithCoordinates => new WellWithCoordinates( /** @phpstan-ignore missingType.parameter (is in template context) */
             $content,
             Coordinates::fromString($coordinatesString, $this->coordinateSystem)
         );
@@ -135,7 +135,7 @@ public function isConsecutive(FlowDirection $flowDirection): bool
         $positions = $this->filledWells()
             ->map(
                 /** @param TWell $content */
-                fn ($content, string $coordinatesString): int => Coordinates::fromString($coordinatesString, $this->coordinateSystem)->position($flowDirection)
+                fn ($content, string $coordinatesString): int => Coordinates::fromString($coordinatesString, $this->coordinateSystem)->position($flowDirection) /** @phpstan-ignore missingType.parameter (is in template context) */
             );
 
         if ($positions->isEmpty()) {

src/PHPStan/Rules/MissingClosureParameterTypehintRule.php

@@ -0,0 +1,61 @@
+<?php declare(strict_types=1);
+
+namespace MLL\Utils\PHPStan\Rules;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\ArrowFunction;
+use PhpParser\Node\Expr\Closure;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+/**
+ * @implements Rule<Node\Expr>
+ */
+final class MissingClosureParameterTypehintRule implements Rule
+{
+    /** @return class-string<Node\Expr> */
+    public function getNodeType(): string
+    {
+        return Node\Expr::class;
+    }
+
+    /**
+     * @param Node\Expr $node
+     *
+     * @return list<IdentifierRuleError>
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (! $node instanceof Closure && ! $node instanceof ArrowFunction) {
+            return [];
+        }
+
+        $errors = [];
+        foreach ($node->params as $param) {
+            if ($param->type !== null) {
+                continue;
+            }
+
+            $paramVar = $param->var;
+
+            if (! $paramVar instanceof Variable) {
+                continue;
+            }
+
+            if (! is_string($paramVar->name)) {
+                continue;
+            }
+
+            $varName = $paramVar->name;
+
+            $errors[] = RuleErrorBuilder::message("Closure parameter {$varName} is missing a native type hint.")
+                ->identifier('missingType.parameter')
+                ->build();
+        }
+
+        return $errors;
+    }
+}

src/QxManager/QxManagerSampleSheet.php

@@ -29,7 +29,7 @@ public function toCsvString(Microplate $microplate, CarbonInterface $createdDate
 CSV);
 
         $body = $microplate->sortedWells(FlowDirection::ROW())
-            ->map(function ($well, string $coordinateString) use ($microplate): string {
+            ->map(function ($well, string $coordinateString) use ($microplate): string { /** @phpstan-ignore missingType.parameter (is in template context) */
                 $coordinates = Coordinates::fromString($coordinateString, $microplate->coordinateSystem);
 
                 if ($well instanceof FilledWell) {

src/Specification.php

@@ -21,7 +21,7 @@ class Specification
      */
     public static function not(callable $specification): callable
     {
-        return fn ($value): bool => ! $specification($value);
+        return fn ($value): bool => ! $specification($value); /** @phpstan-ignore missingType.parameter (is in template context) */
     }
 
     /**
@@ -33,7 +33,7 @@ public static function not(callable $specification): callable
      */
     public static function or(callable ...$specifications): callable
     {
-        return function ($value) use ($specifications): bool {
+        return function ($value) use ($specifications): bool { /** @phpstan-ignore missingType.parameter (is in template context) */
             foreach ($specifications as $specification) {
                 if ($specification($value)) {
                     return true;
@@ -53,7 +53,7 @@ public static function or(callable ...$specifications): callable
      */
     public static function and(callable ...$specifications): callable
     {
-        return function ($value) use ($specifications): bool {
+        return function ($value) use ($specifications): bool { /** @phpstan-ignore missingType.parameter (is in template context) */
             foreach ($specifications as $specification) {
                 if (! $specification($value)) {
                     return false;

src/Tecan/Rack/BaseRack.php

@@ -20,7 +20,7 @@ public function __construct(CoordinateSystem $coordinateSystem)
         $this->coordinateSystem = $coordinateSystem;
         /** @phpstan-ignore-next-line types are correct, but phpstan doesn't understand it */
         $this->positions = Collection::times($this->positionCount(), fn () => self::EMPTY_POSITION)
-            ->mapWithKeys(fn ($content, int $position): array => [$position + 1 => $content]);
+            ->mapWithKeys(fn ($content, int $position): array => [$position + 1 => $content]); /** @phpstan-ignore missingType.parameter (is in template context) */
     }
 
     public function id(): ?string
@@ -52,7 +52,7 @@ public function assignLastEmptyPosition($content): int
     public function findFirstEmptyPosition(): int
     {
         $firstEmpty = $this->positions
-            ->filter(fn ($content): bool => $content === self::EMPTY_POSITION)
+            ->filter(fn ($content): bool => $content === self::EMPTY_POSITION) /** @phpstan-ignore missingType.parameter (is in template context) */
             ->keys()
             ->first();
 
@@ -66,7 +66,7 @@ public function findFirstEmptyPosition(): int
     public function findLastEmptyPosition(): int
     {
         $lastEmpty = $this->positions
-            ->filter(fn ($content): bool => $content === self::EMPTY_POSITION)
+            ->filter(fn ($content): bool => $content === self::EMPTY_POSITION) /** @phpstan-ignore missingType.parameter (is in template context) */
             ->keys()
             ->last();
 

tests/SpecificationTest.php

@@ -9,7 +9,7 @@ final class SpecificationTest extends TestCase
 {
     public function testNot(): void
     {
-        $truthy = fn ($value): bool => (bool) $value;
+        $truthy = fn (string $value): bool => (bool) $value;
 
         self::assertTrue($truthy('truthy'));