diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 96eb377f..1eaabba7 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -59,6 +59,10 @@ This library ships custom PHPStan rules (`src/PHPStan/Rules/`) and disallowed ca Consumer projects get these automatically via `phpstan/extension-installer`. The `phpstan.neon` in this repo includes additional rules enabled only for this project itself. +A rule enabled there needs an `ignoreErrors` entry scoped to `tests/PHPStan/data/`, since fixtures violate rules on purpose. + +Test new rules with PHPStan's `RuleTestCase` against a fixture in `tests/PHPStan/data/`. +That directory is excluded from rector and php-cs-fixer — both would otherwise normalize away the violations under test. ## Conventions diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php index ca30df19..c51fe754 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.php @@ -4,6 +4,7 @@ $finder = PhpCsFixer\Finder::create() ->notPath('vendor') + ->exclude('tests/PHPStan/data') // Fixtures intentionally violate the rules under test ->in(__DIR__) ->name('*.php') ->ignoreDotFiles(true) diff --git a/phpstan.neon b/phpstan.neon index e3ad45ee..266d79a8 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -6,6 +6,7 @@ rules: #- MLL\Utils\PHPStan\Rules\ThrowableClassNameRule - MLL\Utils\PHPStan\Rules\VariableNameIdToIDRule - MLL\Utils\PHPStan\Rules\MissingClosureParameterTypehintRule +- MLL\Utils\PHPStan\Rules\MissingClosureReturnTypehintRule parameters: level: max paths: @@ -36,6 +37,22 @@ parameters: paths: - tests/PHPStan/data/ + # Test fixtures intentionally omit closure type hints + - message: '#is missing a native return type hint\.#' + paths: + - tests/PHPStan/data/ + - message: '#is missing a native type hint\.#' + paths: + - tests/PHPStan/data/ + + # Test fixtures include untyped functions and methods to prove the closure rules ignore them + - message: '#has no return type specified\.#' + paths: + - tests/PHPStan/data/ + - message: '#with no type specified\.#' + paths: + - tests/PHPStan/data/ + # PHPStan internal API usage is acceptable in tests - message: '#is not covered by backward compatibility promise#' paths: diff --git a/phpstan/php-below-8.1.neon b/phpstan/php-below-8.1.neon index 9a45de45..815380c6 100644 --- a/phpstan/php-below-8.1.neon +++ b/phpstan/php-below-8.1.neon @@ -4,7 +4,7 @@ parameters: - '#Unknown PHPDoc tag: @phpstan-ignore#' # Older PHPStan has stricter/different closure parameter typehint checking - - '#Closure parameter .* is missing a native type hint\.#' + - '#(Closure|Arrow function) parameter .* is missing a native type hint\.#' # Differences in type inference between PHPStan versions - '#Cannot access property .* on mixed\.#' @@ -29,7 +29,7 @@ parameters: - '#PHPDoc tag @param has invalid value.*covariant.*#' # Return type differences in older PHPStan rule interfaces - - '#Method MLL\\Utils\\PHPStan\\Rules\\MissingClosureParameterTypehintRule::processNode\(\) should return array but returns array\.#' + - '#Method MLL\\Utils\\PHPStan\\Rules\\MissingClosure(Parameter|Return)TypehintRule::processClosure\(\) should return array but returns array\.#' # Existing code with @phpstan-ignore that older versions don't understand - message: '#Cannot access property \$name on SimpleXMLElement\|null\.#' diff --git a/rector.php b/rector.php index 88c0015d..56d17aef 100644 --- a/rector.php +++ b/rector.php @@ -24,6 +24,7 @@ Rector\PHPUnit\CodeQuality\Rector\Class_\PreferPHPUnitSelfCallRector::class, ]) ->withSkip([ + __DIR__ . '/tests/PHPStan/data', // fixtures intentionally violate the rules under test Rector\PHPUnit\CodeQuality\Rector\Class_\PreferPHPUnitThisCallRector::class, // breaks tests Rector\CodeQuality\Rector\Concat\JoinStringConcatRector::class => [ __DIR__ . '/tests/CSVArrayTest.php', // keep `\r\n` for readability diff --git a/rules.neon b/rules.neon index f354ac38..fe965166 100644 --- a/rules.neon +++ b/rules.neon @@ -4,6 +4,7 @@ rules: #- MLL\Utils\PHPStan\Rules\VariableNameIdToIDRule #- MLL\Utils\PHPStan\Rules\PropertyNameIdToIDRule #- MLL\Utils\PHPStan\Rules\MissingClosureParameterTypehintRule +#- MLL\Utils\PHPStan\Rules\MissingClosureReturnTypehintRule parameters: # https://github.com/spaze/phpstan-disallowed-calls/blob/main/docs/custom-rules.md disallowedFunctionCalls: diff --git a/src/PHPStan/Rules/ClosureTypehintRule.php b/src/PHPStan/Rules/ClosureTypehintRule.php new file mode 100644 index 00000000..3ac9797d --- /dev/null +++ b/src/PHPStan/Rules/ClosureTypehintRule.php @@ -0,0 +1,51 @@ + + */ +abstract class ClosureTypehintRule implements Rule +{ + /** + * @param Closure|ArrowFunction $closure + * + * @return list + */ + abstract protected function processClosure(Node\FunctionLike $closure): array; + + /** @param Closure|ArrowFunction $closure */ + protected function closureKind(Node\FunctionLike $closure): string + { + return $closure instanceof ArrowFunction + ? 'Arrow function' + : 'Closure'; + } + + /** @return class-string */ + public function getNodeType(): string + { + return Node\FunctionLike::class; + } + + /** + * @param Node\FunctionLike $node + * + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + if (! $node instanceof Closure && ! $node instanceof ArrowFunction) { + return []; + } + + return $this->processClosure($node); + } +} diff --git a/src/PHPStan/Rules/MissingClosureParameterTypehintRule.php b/src/PHPStan/Rules/MissingClosureParameterTypehintRule.php index d5c41062..d8294ac1 100644 --- a/src/PHPStan/Rules/MissingClosureParameterTypehintRule.php +++ b/src/PHPStan/Rules/MissingClosureParameterTypehintRule.php @@ -3,38 +3,17 @@ 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 - */ -final class MissingClosureParameterTypehintRule implements Rule +final class MissingClosureParameterTypehintRule extends ClosureTypehintRule { - /** @return class-string */ - public function getNodeType(): string + protected function processClosure(Node\FunctionLike $closure): array { - return Node\Expr::class; - } - - /** - * @param Node\Expr $node - * - * @return list - */ - public function processNode(Node $node, Scope $scope): array - { - if (! $node instanceof Closure && ! $node instanceof ArrowFunction) { - return []; - } + $kind = $this->closureKind($closure); $errors = []; - foreach ($node->params as $param) { + foreach ($closure->getParams() as $param) { if ($param->type !== null) { continue; } @@ -45,14 +24,15 @@ public function processNode(Node $node, Scope $scope): array continue; } - if (! is_string($paramVar->name)) { + $varName = $paramVar->name; + + if (! is_string($varName)) { continue; } - $varName = $paramVar->name; - - $errors[] = RuleErrorBuilder::message("Closure parameter {$varName} is missing a native type hint.") + $errors[] = RuleErrorBuilder::message("{$kind} parameter {$varName} is missing a native type hint.") ->identifier('missingType.parameter') + ->line($param->getStartLine()) ->build(); } diff --git a/src/PHPStan/Rules/MissingClosureReturnTypehintRule.php b/src/PHPStan/Rules/MissingClosureReturnTypehintRule.php new file mode 100644 index 00000000..03c0b123 --- /dev/null +++ b/src/PHPStan/Rules/MissingClosureReturnTypehintRule.php @@ -0,0 +1,29 @@ +getReturnType() instanceof Node) { + return []; + } + + $kind = $this->closureKind($closure); + + return [ + RuleErrorBuilder::message("{$kind} is missing a native return type hint.") + ->identifier('missingType.closureReturn') + ->build(), + ]; + } +} diff --git a/tests/PHPStan/MissingClosureParameterTypehintRuleTest.php b/tests/PHPStan/MissingClosureParameterTypehintRuleTest.php new file mode 100644 index 00000000..df10fe23 --- /dev/null +++ b/tests/PHPStan/MissingClosureParameterTypehintRuleTest.php @@ -0,0 +1,28 @@ + + */ +final class MissingClosureParameterTypehintRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new MissingClosureParameterTypehintRule(); + } + + public function testMissingParameterTypes(): void + { + $this->analyse([__DIR__ . '/data/closure-parameter-types.php'], [ + ['Closure parameter factor is missing a native type hint.', 3], + ['Arrow function parameter factor is missing a native type hint.', 7], + ['Closure parameter first is missing a native type hint.', 16], + ['Closure parameter second is missing a native type hint.', 17], + ]); + } +} diff --git a/tests/PHPStan/MissingClosureReturnTypehintRuleTest.php b/tests/PHPStan/MissingClosureReturnTypehintRuleTest.php new file mode 100644 index 00000000..db50b345 --- /dev/null +++ b/tests/PHPStan/MissingClosureReturnTypehintRuleTest.php @@ -0,0 +1,26 @@ + + */ +final class MissingClosureReturnTypehintRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new MissingClosureReturnTypehintRule(); + } + + public function testMissingReturnTypes(): void + { + $this->analyse([__DIR__ . '/data/closure-return-types.php'], [ + ['Closure is missing a native return type hint.', 3], + ['Arrow function is missing a native return type hint.', 7], + ]); + } +} diff --git a/tests/PHPStan/data/closure-parameter-types.php b/tests/PHPStan/data/closure-parameter-types.php new file mode 100644 index 00000000..957cf7c3 --- /dev/null +++ b/tests/PHPStan/data/closure-parameter-types.php @@ -0,0 +1,33 @@ + 2; + +$typedClosureParameter = static function (int $factor): int { + return 2 * $factor; +}; + +$typedArrowParameter = static fn (int $factor): int => 2 * $factor; + +$missingMultiLineParameters = static function ( + $first, + $second +): int { + return 2; +}; + +function plainFunctionWithoutParameterType($factor): int +{ + return 2; +} + +class MethodWithoutParameterType +{ + public function untyped($factor): int + { + return 2; + } +} diff --git a/tests/PHPStan/data/closure-return-types.php b/tests/PHPStan/data/closure-return-types.php new file mode 100644 index 00000000..6515ba30 --- /dev/null +++ b/tests/PHPStan/data/closure-return-types.php @@ -0,0 +1,26 @@ + $value * 2; + +$typedClosure = static function (int $value): int { + return $value * 2; +}; + +$typedArrow = static fn (int $value): int => $value * 2; + +function plainFunctionWithoutReturnType() +{ + return 1; +} + +class MethodWithoutReturnType +{ + public function untyped() + { + return 1; + } +}