diff --git a/.github/workflows/code_analysis.yaml b/.github/workflows/code_analysis.yaml index c1d643e8e13..831ebb1012f 100644 --- a/.github/workflows/code_analysis.yaml +++ b/.github/workflows/code_analysis.yaml @@ -53,7 +53,7 @@ jobs: - name: 'Active Classes' run: | - vendor/bin/class-leak check bin config src rules utils --skip-suffix "Rector" --skip-type="Rector\\Utils\\Compiler\\Unprefixer" --skip-type="Rector\\NodeCollector\\BinaryOpConditionsCollector" --skip-type="Rector\\DependencyInjection\\PHPStan\\RichParserFactory" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\PreferGenericTemplateOnSameUnionTypeRule\\Source\\SameUnionParamAndReturn" --skip-type="Rector\\Utils\\PHPStan\\Tests\\Rule\\PreferGenericTemplateOnSameUnionTypeRule\\Source\\SkipGenericTemplate" + vendor/bin/class-leak check bin config src rules utils --skip-suffix "Rector" --skip-type="Rector\\Utils\\Compiler\\Unprefixer" --skip-type="Rector\\NodeCollector\\BinaryOpConditionsCollector" --skip-type="Rector\\DependencyInjection\\PHPStan\\RichParserFactory" --skip-path="Source" - name: 'Compatible PHPStan versions' diff --git a/phpstan.neon b/phpstan.neon index c80acc14b90..2c146381ddb 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -5,6 +5,7 @@ rules: - Rector\Utils\PHPStan\Rule\RegisterRelatedPolyfillRectorRule - Rector\Utils\PHPStan\Rule\NoScopeFetchBeforeCheapGuardRule - Rector\Utils\PHPStan\Rule\PreferGenericTemplateOnSameUnionTypeRule + - Rector\Utils\PHPStan\Rule\NarrowReturnToUnionParamRule parameters: level: 8 diff --git a/rules/Arguments/ArgumentDefaultValueReplacer.php b/rules/Arguments/ArgumentDefaultValueReplacer.php index 16527bc1502..3e92627089a 100644 --- a/rules/Arguments/ArgumentDefaultValueReplacer.php +++ b/rules/Arguments/ArgumentDefaultValueReplacer.php @@ -105,7 +105,7 @@ private function processParams( private function processArgs( MethodCall|StaticCall|FuncCall|New_ $expr, ReplaceArgumentDefaultValueInterface $replaceArgumentDefaultValue - ): ?Expr { + ): MethodCall|StaticCall|FuncCall|New_|null { if ($expr->isFirstClassCallable()) { return null; } diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveNullTagValueNodeRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveNullTagValueNodeRector.php index 6430df91de8..45003a9deb4 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveNullTagValueNodeRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveNullTagValueNodeRector.php @@ -159,7 +159,12 @@ static function (AstNode $astNode) use ($paramNames): ?int { ); } - private function processVarTagNull(Expression|Property $node): ?Node + /** + * @template TNode of Expression|Property + * @param TNode $node + * @return TNode|null + */ + private function processVarTagNull(Expression|Property $node): Expression|Property|null { $phpdocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); $varTagValueNode = $phpdocInfo->getVarTagValueNode(); diff --git a/rules/Php54/Rector/Break_/RemoveZeroBreakContinueRector.php b/rules/Php54/Rector/Break_/RemoveZeroBreakContinueRector.php index efe0f99f6c6..0aab61a89e6 100644 --- a/rules/Php54/Rector/Break_/RemoveZeroBreakContinueRector.php +++ b/rules/Php54/Rector/Break_/RemoveZeroBreakContinueRector.php @@ -110,7 +110,12 @@ public function refactor(Node $node): ?Node return null; } - private function processVariableNum(Break_|Continue_ $stmt, Variable $numVariable): ?Node + /** + * @template TNode of Break_|Continue_ + * @param TNode $stmt + * @return TNode|null + */ + private function processVariableNum(Break_|Continue_ $stmt, Variable $numVariable): Break_|Continue_|null { $staticType = $this->getType($numVariable); diff --git a/utils/phpstan/src/Rule/NarrowReturnToUnionParamRule.php b/utils/phpstan/src/Rule/NarrowReturnToUnionParamRule.php new file mode 100644 index 00000000000..337222e98e8 --- /dev/null +++ b/utils/phpstan/src/Rule/NarrowReturnToUnionParamRule.php @@ -0,0 +1,199 @@ + + * @see \Rector\Utils\PHPStan\Tests\Rule\NarrowReturnToUnionParamRule\NarrowReturnToUnionParamRuleTest + */ +final class NarrowReturnToUnionParamRule implements Rule +{ + private const string ERROR_MESSAGE = 'Return type "%s" is too broad for a returned union param. Declare the explicit union return type "%s" instead.'; + + public function getNodeType(): string + { + return ClassMethod::class; + } + + /** + * @param ClassMethod $node + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + $returnName = $this->resolveSingleClassReturnName($node->returnType); + if ($returnName === null) { + return []; + } + + foreach ($node->params as $param) { + $paramNames = $this->resolveClassUnionNames($param->type); + if ($paramNames === null) { + continue; + } + + // return type is one of the union members, not a broad shared parent - already narrow enough + if (in_array($returnName, $paramNames, true)) { + continue; + } + + if (! $param->var instanceof Variable || ! is_string($param->var->name)) { + continue; + } + + if (! $this->methodReturnsVariable($node, $param->var->name)) { + continue; + } + + $shortNames = array_map(static function (string $name): string { + $parts = explode('\\', $name); + return end($parts); + }, $paramNames); + + if ($this->isNullableReturnType($node->returnType)) { + $shortNames[] = 'null'; + } + + return [ + RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $returnName, implode('|', $shortNames))) + ->identifier('rector.explicitUnionReturnType') + ->line($node->getStartLine()) + ->build(), + ]; + } + + return []; + } + + private function methodReturnsVariable(ClassMethod $classMethod, string $variableName): bool + { + if ($classMethod->stmts === null) { + return false; + } + + $foundReturn = new NodeFinder()->findFirst( + $classMethod->stmts, + static fn (Node $subNode): bool => $subNode instanceof Return_ + && $subNode->expr instanceof Variable + && $subNode->expr->name === $variableName + ); + + return $foundReturn instanceof Return_; + } + + private function isNullableReturnType(null|Identifier|Name|ComplexType $type): bool + { + if ($type instanceof NullableType) { + return true; + } + + if (! $type instanceof UnionType) { + return false; + } + + return array_any($type->types, fn (Identifier|Name|IntersectionType $memberType): bool => $memberType instanceof Identifier && $memberType->toLowerString() === 'null'); + } + + /** + * Returns the single class return type name (nullable allowed), or null when the return type is + * missing, scalar, or a union of 2+ class types - the latter is already an explicit union. + */ + private function resolveSingleClassReturnName(null|Identifier|Name|ComplexType $type): ?string + { + if ($type instanceof NullableType) { + $type = $type->type; + } + + if ($type instanceof Name) { + return $type->toString(); + } + + if (! $type instanceof UnionType) { + return null; + } + + $names = []; + foreach ($type->types as $memberType) { + if ($memberType instanceof Identifier) { + if ($memberType->toLowerString() === 'null') { + continue; + } + + return null; + } + + if (! $memberType instanceof Name) { + return null; + } + + $names[] = $memberType->toString(); + } + + if (count($names) !== 1) { + return null; + } + + return $names[0]; + } + + /** + * Returns sorted class-type member names (null stripped), or null when the type is not a + * union of 2+ class types - scalars, array and intersections disqualify it. + * + * @return list|null + */ + private function resolveClassUnionNames(null|Identifier|Name|ComplexType $type): ?array + { + if (! $type instanceof UnionType) { + return null; + } + + $names = []; + foreach ($type->types as $memberType) { + if ($memberType instanceof Identifier) { + if ($memberType->toLowerString() === 'null') { + continue; + } + + return null; + } + + if (! $memberType instanceof Name) { + return null; + } + + $names[] = $memberType->toString(); + } + + if (count($names) < 2) { + return null; + } + + $names = array_unique($names); + sort($names); + + return $names; + } +} diff --git a/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/NarrowReturnToUnionParamRuleTest.php b/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/NarrowReturnToUnionParamRuleTest.php new file mode 100644 index 00000000000..71c588cdaa5 --- /dev/null +++ b/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/NarrowReturnToUnionParamRuleTest.php @@ -0,0 +1,32 @@ + + */ +final class NarrowReturnToUnionParamRuleTest extends RuleTestCase +{ + public function testBroadReturnOfUnionParam(): void + { + $expectedErrorMessage = 'Return type "PhpParser\Node" is too broad for a returned union param. Declare the explicit union return type "MethodCall|New_|null" instead.'; + + $this->analyse([__DIR__ . '/Source/BroadReturnOfUnionParam.php'], [[$expectedErrorMessage, 13]]); + } + + public function testSkip(): void + { + $this->analyse([__DIR__ . '/Source/SkipBroadReturn.php'], []); + } + + protected function getRule(): Rule + { + return new NarrowReturnToUnionParamRule(); + } +} diff --git a/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/Source/BroadReturnOfUnionParam.php b/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/Source/BroadReturnOfUnionParam.php new file mode 100644 index 00000000000..b7c7b049fca --- /dev/null +++ b/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/Source/BroadReturnOfUnionParam.php @@ -0,0 +1,21 @@ +var; + } + + if (! $expr instanceof Variable) { + return null; + } + + return $expr; + } +}