diff --git a/README.md b/README.md index 1aa2561..c98f78d 100644 --- a/README.md +++ b/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 $entities + * @return list + */ +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: diff --git a/phpstan.neon.dist b/phpstan.neon.dist index b90e74a..e148e1a 100644 --- a/phpstan.neon.dist +++ b/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: - diff --git a/rules.neon b/rules.neon index 2ee0e9e..a08b072 100644 --- a/rules.neon +++ b/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 - diff --git a/src/Rule/EnforceClosureParamNativeTypehintRule.php b/src/Rule/EnforceClosureParamNativeTypehintRule.php new file mode 100644 index 0000000..852d3f8 --- /dev/null +++ b/src/Rule/EnforceClosureParamNativeTypehintRule.php @@ -0,0 +1,84 @@ + + */ +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 + */ + 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; + } + +} diff --git a/tests/Rule/EnforceClosureParamNativeTypehintRuleTest.php b/tests/Rule/EnforceClosureParamNativeTypehintRuleTest.php new file mode 100644 index 0000000..4cf29bf --- /dev/null +++ b/tests/Rule/EnforceClosureParamNativeTypehintRuleTest.php @@ -0,0 +1,64 @@ + + */ +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 + } + +} diff --git a/tests/Rule/data/EnforceClosureParamNativeTypehintRule/allow-inferring.php b/tests/Rule/data/EnforceClosureParamNativeTypehintRule/allow-inferring.php new file mode 100644 index 0000000..67a4717 --- /dev/null +++ b/tests/Rule/data/EnforceClosureParamNativeTypehintRule/allow-inferring.php @@ -0,0 +1,30 @@ + $a + * @param list $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) {}; +} diff --git a/tests/Rule/data/EnforceClosureParamNativeTypehintRule/enforce-everywhere.php b/tests/Rule/data/EnforceClosureParamNativeTypehintRule/enforce-everywhere.php new file mode 100644 index 0000000..579ac57 --- /dev/null +++ b/tests/Rule/data/EnforceClosureParamNativeTypehintRule/enforce-everywhere.php @@ -0,0 +1,30 @@ + $a + * @param list $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) {}; +} diff --git a/tests/RuleTestCase.php b/tests/RuleTestCase.php index 6f6f506..45b77f6 100644 --- a/tests/RuleTestCase.php +++ b/tests/RuleTestCase.php @@ -35,7 +35,7 @@ protected function analyseFile(string $file): void * @param list $actualErrors * @return list */ - private function processActualErrors(array $actualErrors): array + protected function processActualErrors(array $actualErrors): array { $resultToAssert = [];