EnforceClosureParamNativeTypehintRule (#192)

README.md

@@ -30,6 +30,9 @@ parameters:
         classSuffixNaming:
             enabled: true
             superclassToSuffixMapping: []
+        enforceClosureParamNativeTypehint:
+            enabled: true
+            allowMissingTypeWhenInferred: false
         enforceEnumMatch:
             enabled: true
         enforceIteratorToArrayPreserveKeys:
@@ -210,6 +213,27 @@ enum MyEnum: string { // missing @implements tag
 ```
 
 
+### enforceClosureParamNativeTypehint
+- Enforces usage of native typehints for closure & arrow function parameters
+- Does nothing on PHP 7.4 and below as native `mixed` is not available there
+- Can be configured by `allowMissingTypeWhenInferred: true` to allow missing typehint when it can be inferred from the context
+
+```php
+/**
+ * @param list<Entity> $entities
+ * @return list<Uuid>
+ */
+public function getIds(array $entities): array {
+    return array_map(
+        function ($entity) { // missing native typehint; not reported with allowMissingTypeWhenInferred: true
+            return $entity->id;
+        },
+        $entities
+    );
+}
+```
+
+
 ### enforceEnumMatchRule
 - Enforces usage of `match ($enum)` instead of exhaustive conditions like `if ($enum === Enum::One) elseif ($enum === Enum::Two)`
 - This rule aims to "fix" a bit problematic behaviour of PHPStan (introduced at 1.10.0 and fixed in [1.10.34](https://github.com/phpstan/phpstan-src/commit/fc7c0283176e5dc3867ade26ac835ee7f52599a9)). It understands enum cases very well and forces you to adjust following code:

phpstan.neon.dist

@@ -38,6 +38,8 @@ parameters:
                 ShipMonk\PHPStan\RuleTestCase: RuleTest
         forbidAssignmentNotMatchingVarDoc:
             enabled: false # native check is better now; this rule will be dropped / reworked in 3.0
+        enforceClosureParamNativeTypehint:
+            enabled: false # we support even PHP 7.4, some typehints cannot be used
 
     ignoreErrors:
         -

rules.neon

@@ -9,6 +9,9 @@ parameters:
         classSuffixNaming:
             enabled: true
             superclassToSuffixMapping: []
+        enforceClosureParamNativeTypehint:
+            enabled: true
+            allowMissingTypeWhenInferred: false
         enforceEnumMatch:
             enabled: true
         enforceIteratorToArrayPreserveKeys:
@@ -119,6 +122,10 @@ parametersSchema:
             enabled: bool()
             superclassToSuffixMapping: arrayOf(string(), string())
         ])
+        enforceClosureParamNativeTypehint: structure([
+            enabled: bool()
+            allowMissingTypeWhenInferred: bool()
+        ])
         enforceEnumMatch: structure([
             enabled: bool()
         ])
@@ -238,6 +245,8 @@ conditionalTags:
         phpstan.rules.rule: %shipmonkRules.backedEnumGenerics.enabled%
     ShipMonk\PHPStan\Rule\ClassSuffixNamingRule:
         phpstan.rules.rule: %shipmonkRules.classSuffixNaming.enabled%
+    ShipMonk\PHPStan\Rule\EnforceClosureParamNativeTypehintRule:
+        phpstan.rules.rule: %shipmonkRules.enforceClosureParamNativeTypehint.enabled%
     ShipMonk\PHPStan\Rule\EnforceEnumMatchRule:
         phpstan.rules.rule: %shipmonkRules.enforceEnumMatch.enabled%
     ShipMonk\PHPStan\Rule\EnforceIteratorToArrayPreserveKeysRule:
@@ -334,6 +343,10 @@ services:
         class: ShipMonk\PHPStan\Rule\ClassSuffixNamingRule
         arguments:
             superclassToSuffixMapping: %shipmonkRules.classSuffixNaming.superclassToSuffixMapping%
+    -
+        class: ShipMonk\PHPStan\Rule\EnforceClosureParamNativeTypehintRule
+        arguments:
+            allowMissingTypeWhenInferred: %shipmonkRules.enforceClosureParamNativeTypehint.allowMissingTypeWhenInferred%
     -
         class: ShipMonk\PHPStan\Rule\EnforceEnumMatchRule
     -

src/Rule/EnforceClosureParamNativeTypehintRule.php

@@ -0,0 +1,84 @@
+<?php declare(strict_types = 1);
+
+namespace ShipMonk\PHPStan\Rule;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\InArrowFunctionNode;
+use PHPStan\Node\InClosureNode;
+use PHPStan\Php\PhpVersion;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\MixedType;
+use function is_string;
+
+/**
+ * @implements Rule<Node>
+ */
+class EnforceClosureParamNativeTypehintRule implements Rule
+{
+
+    private PhpVersion $phpVersion;
+
+    private bool $allowMissingTypeWhenInferred;
+
+    public function __construct(
+        PhpVersion $phpVersion,
+        bool $allowMissingTypeWhenInferred
+    )
+    {
+        $this->phpVersion = $phpVersion;
+        $this->allowMissingTypeWhenInferred = $allowMissingTypeWhenInferred;
+    }
+
+    public function getNodeType(): string
+    {
+        return Node::class;
+    }
+
+    /**
+     * @return list<RuleError>
+     */
+    public function processNode(
+        Node $node,
+        Scope $scope
+    ): array
+    {
+        if (!$node instanceof InClosureNode && !$node instanceof InArrowFunctionNode) { // @phpstan-ignore-line bc promise
+            return [];
+        }
+
+        if ($this->phpVersion->getVersionId() < 80_000) {
+            return []; // unable to add mixed native typehint there
+        }
+
+        $errors = [];
+        $type = $node instanceof InClosureNode ? 'closure' : 'arrow function';
+
+        foreach ($node->getOriginalNode()->getParams() as $param) {
+            if (!$param->var instanceof Variable || !is_string($param->var->name)) {
+                continue;
+            }
+
+            if ($param->type !== null) {
+                continue;
+            }
+
+            $paramType = $scope->getType($param->var);
+
+            if ($this->allowMissingTypeWhenInferred && (!$paramType instanceof MixedType || $paramType->isExplicitMixed())) {
+                continue;
+            }
+
+            $errors[] = RuleErrorBuilder::message("Missing parameter typehint for {$type} parameter \${$param->var->name}.")
+                ->identifier('shipmonk.unknownClosureParamType')
+                ->line($param->getLine())
+                ->build();
+        }
+
+        return $errors;
+    }
+
+}

tests/Rule/EnforceClosureParamNativeTypehintRuleTest.php

@@ -0,0 +1,64 @@
+<?php declare(strict_types = 1);
+
+namespace ShipMonk\PHPStan\Rule;
+
+use LogicException;
+use PHPStan\Php\PhpVersion;
+use PHPStan\Rules\Rule;
+use ShipMonk\PHPStan\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<EnforceClosureParamNativeTypehintRule>
+ */
+class EnforceClosureParamNativeTypehintRuleTest extends RuleTestCase
+{
+
+    private ?bool $allowMissingTypeWhenInferred = null;
+
+    private ?PhpVersion $phpVersion = null;
+
+    protected function getRule(): Rule
+    {
+        if ($this->allowMissingTypeWhenInferred === null || $this->phpVersion === null) {
+            throw new LogicException('Missing phpVersion or allowMissingTypeWhenInferred');
+        }
+
+        return new EnforceClosureParamNativeTypehintRule(
+            $this->phpVersion,
+            $this->allowMissingTypeWhenInferred,
+        );
+    }
+
+    public function testAllowInferring(): void
+    {
+        $this->allowMissingTypeWhenInferred = true;
+        $this->phpVersion = $this->createPhpVersion(80_000);
+
+        $this->analyseFile(__DIR__ . '/data/EnforceClosureParamNativeTypehintRule/allow-inferring.php');
+    }
+
+    public function testEnforceEverywhere(): void
+    {
+        $this->allowMissingTypeWhenInferred = false;
+        $this->phpVersion = $this->createPhpVersion(80_000);
+
+        $this->analyseFile(__DIR__ . '/data/EnforceClosureParamNativeTypehintRule/enforce-everywhere.php');
+    }
+
+    public function testNoErrorOnPhp74(): void
+    {
+        $this->allowMissingTypeWhenInferred = false;
+        $this->phpVersion = $this->createPhpVersion(70_400);
+
+        self::assertEmpty($this->processActualErrors($this->gatherAnalyserErrors([
+            __DIR__ . '/data/EnforceClosureParamNativeTypehintRule/allow-inferring.php',
+            __DIR__ . '/data/EnforceClosureParamNativeTypehintRule/enforce-everywhere.php',
+        ])));
+    }
+
+    private function createPhpVersion(int $version): PhpVersion
+    {
+        return new PhpVersion($version); // @phpstan-ignore-line ignore bc promise
+    }
+
+}

tests/Rule/data/EnforceClosureParamNativeTypehintRule/allow-inferring.php

@@ -0,0 +1,30 @@
+<?php declare(strict_types = 1);
+
+namespace ForbidImplicitMixedInClosureParamsRule;
+
+
+/**
+ * @param list<string> $a
+ * @param list<mixed> $b
+ */
+function test($a, $b, $c, array $d): void
+{
+    array_map(function ($item) {}, [1]);
+    array_map(function ($item) {}, $a);
+    array_map(function ($item) {}, $b);
+    array_map(function ($item) {}, $c); // error: Missing parameter typehint for closure parameter $item.
+    array_map(function ($item) {}, $d); // error: Missing parameter typehint for closure parameter $item.
+    array_map(function (int $item) {}, $c);
+    array_map(function (int $item) {}, $d);
+
+    array_map(static fn($item) => 1, [1]);
+    array_map(static fn($item) => 1, $a);
+    array_map(static fn($item) => 1, $b);
+    array_map(static fn($item) => 1, $c); // error: Missing parameter typehint for arrow function parameter $item.
+    array_map(static fn($item) => 1, $d); // error: Missing parameter typehint for arrow function parameter $item.
+    array_map(static fn(int $item) => 1, $c);
+    array_map(static fn(int $item) => 1, $d);
+
+    function ($item2) {}; // error: Missing parameter typehint for closure parameter $item2.
+    function (mixed $item2) {};
+}

tests/Rule/data/EnforceClosureParamNativeTypehintRule/enforce-everywhere.php

@@ -0,0 +1,30 @@
+<?php declare(strict_types = 1);
+
+namespace ForbidImplicitMixedInClosureParamsRule\Everywhere;
+
+
+/**
+ * @param list<string> $a
+ * @param list<mixed> $b
+ */
+function test($a, $b, $c, array $d): void
+{
+    array_map(function ($item) {}, [1]); // error: Missing parameter typehint for closure parameter $item.
+    array_map(function ($item) {}, $a); // error: Missing parameter typehint for closure parameter $item.
+    array_map(function ($item) {}, $b); // error: Missing parameter typehint for closure parameter $item.
+    array_map(function ($item) {}, $c); // error: Missing parameter typehint for closure parameter $item.
+    array_map(function ($item) {}, $d); // error: Missing parameter typehint for closure parameter $item.
+    array_map(function (int $item) {}, $c);
+    array_map(function (int $item) {}, $d);
+
+    array_map(static fn($item) => 1, [1]); // error: Missing parameter typehint for arrow function parameter $item.
+    array_map(static fn($item) => 1, $a); // error: Missing parameter typehint for arrow function parameter $item.
+    array_map(static fn($item) => 1, $b); // error: Missing parameter typehint for arrow function parameter $item.
+    array_map(static fn($item) => 1, $c); // error: Missing parameter typehint for arrow function parameter $item.
+    array_map(static fn($item) => 1, $d); // error: Missing parameter typehint for arrow function parameter $item.
+    array_map(static fn(int $item) => 1, $c);
+    array_map(static fn(int $item) => 1, $d);
+
+    function ($item2) {}; // error: Missing parameter typehint for closure parameter $item2.
+    function (mixed $item2) {};
+}

tests/RuleTestCase.php

@@ -35,7 +35,7 @@ protected function analyseFile(string $file): void
      * @param list<Error> $actualErrors
      * @return list<string>
      */
-    private function processActualErrors(array $actualErrors): array
+    protected function processActualErrors(array $actualErrors): array
     {
         $resultToAssert = [];