From d249df76f20ea322a54e5e4bbaf5994580cccc45 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 16 Oct 2026 11:53:58 +0200 Subject: [PATCH 1/2] move NoScopeFetchBeforeCheapGuardRule and NarrowReturnToUnionParamRule to symplify/phpstan-rules rector set --- phpstan.neon | 3 +- .../src/Rule/NarrowReturnToUnionParamRule.php | 199 ------------------ .../Rule/NoScopeFetchBeforeCheapGuardRule.php | 151 ------------- .../NarrowReturnToUnionParamRuleTest.php | 32 --- .../Source/BroadReturnOfUnionParam.php | 21 -- .../Source/SkipBroadReturn.php | 51 ----- .../NoScopeFetchBeforeCheapGuardRuleTest.php | 32 --- .../Source/ScopeFetchBeforeGuardRector.php | 22 -- .../Source/SkipScopeFetchRector.php | 27 --- 9 files changed, 1 insertion(+), 537 deletions(-) delete mode 100644 utils/phpstan/src/Rule/NarrowReturnToUnionParamRule.php delete mode 100644 utils/phpstan/src/Rule/NoScopeFetchBeforeCheapGuardRule.php delete mode 100644 utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/NarrowReturnToUnionParamRuleTest.php delete mode 100644 utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/Source/BroadReturnOfUnionParam.php delete mode 100644 utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/Source/SkipBroadReturn.php delete mode 100644 utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php delete mode 100644 utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php delete mode 100644 utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php diff --git a/phpstan.neon b/phpstan.neon index 9d5e1d70a33..4c58a22ec66 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -3,8 +3,6 @@ includes: rules: - Rector\Utils\PHPStan\Rule\RegisterRelatedPolyfillRectorRule - - Rector\Utils\PHPStan\Rule\NoScopeFetchBeforeCheapGuardRule - - Rector\Utils\PHPStan\Rule\NarrowReturnToUnionParamRule parameters: level: 8 @@ -56,6 +54,7 @@ parameters: phpunit: true mocks: true ctor: true + modern: true # see https://github.com/TomasVotruba/type-coverage type_coverage: diff --git a/utils/phpstan/src/Rule/NarrowReturnToUnionParamRule.php b/utils/phpstan/src/Rule/NarrowReturnToUnionParamRule.php deleted file mode 100644 index 337222e98e8..00000000000 --- a/utils/phpstan/src/Rule/NarrowReturnToUnionParamRule.php +++ /dev/null @@ -1,199 +0,0 @@ - - * @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/src/Rule/NoScopeFetchBeforeCheapGuardRule.php b/utils/phpstan/src/Rule/NoScopeFetchBeforeCheapGuardRule.php deleted file mode 100644 index 6f4ed34c916..00000000000 --- a/utils/phpstan/src/Rule/NoScopeFetchBeforeCheapGuardRule.php +++ /dev/null @@ -1,151 +0,0 @@ - - * @see \Rector\Utils\PHPStan\Tests\Rule\NoScopeFetchBeforeCheapGuardRule\NoScopeFetchBeforeCheapGuardRuleTest - */ -final class NoScopeFetchBeforeCheapGuardRule implements Rule -{ - private const string ERROR_MESSAGE = 'Move the "return null" guard above ScopeFetcher::fetch(), as it does not use the scope - most nodes then bail out before scope resolution.'; - - public function getNodeType(): string - { - return ClassMethod::class; - } - - /** - * @param ClassMethod $node - * @return list - */ - public function processNode(Node $node, Scope $scope): array - { - if ($node->name->toString() !== 'refactor') { - return []; - } - - $stmts = $node->stmts; - if ($stmts === null) { - return []; - } - - foreach ($stmts as $key => $stmt) { - $scopeVariableName = $this->matchScopeFetchVariableName($stmt); - if ($scopeVariableName === null) { - continue; - } - - $nextStmt = $stmts[$key + 1] ?? null; - if (! $nextStmt instanceof If_) { - continue; - } - - if (! $this->isReturnNullGuard($nextStmt)) { - continue; - } - - if ($this->usesVariable($nextStmt->cond, $scopeVariableName)) { - continue; - } - - return [ - RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier('rector.scopeFetchBeforeCheapGuard') - ->line($stmt->getStartLine()) - ->build(), - ]; - } - - return []; - } - - private function matchScopeFetchVariableName(Stmt $stmt): ?string - { - if (! $stmt instanceof Expression) { - return null; - } - - if (! $stmt->expr instanceof Assign) { - return null; - } - - $assign = $stmt->expr; - if (! $assign->var instanceof Variable || ! is_string($assign->var->name)) { - return null; - } - - if (! $assign->expr instanceof StaticCall) { - return null; - } - - $staticCall = $assign->expr; - if (! $staticCall->class instanceof Name) { - return null; - } - - if ($staticCall->class->toString() !== ScopeFetcher::class) { - return null; - } - - if (! $staticCall->name instanceof Identifier || $staticCall->name->toString() !== 'fetch') { - return null; - } - - return $assign->var->name; - } - - private function isReturnNullGuard(If_ $if): bool - { - if (count($if->stmts) !== 1) { - return false; - } - - $onlyStmt = $if->stmts[0]; - if (! $onlyStmt instanceof Return_) { - return false; - } - - return ! $onlyStmt->expr instanceof Expr || $this->isNullConstant($onlyStmt->expr); - } - - private function isNullConstant(Expr $expr): bool - { - return $expr instanceof ConstFetch && $expr->name->toLowerString() === 'null'; - } - - private function usesVariable(Expr $expr, string $variableName): bool - { - $foundVariable = new NodeFinder()->findFirst( - [$expr], - static fn (Node $subNode): bool => $subNode instanceof Variable && $subNode->name === $variableName - ); - - return $foundVariable instanceof Variable; - } -} diff --git a/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/NarrowReturnToUnionParamRuleTest.php b/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/NarrowReturnToUnionParamRuleTest.php deleted file mode 100644 index 71c588cdaa5..00000000000 --- a/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/NarrowReturnToUnionParamRuleTest.php +++ /dev/null @@ -1,32 +0,0 @@ - - */ -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 deleted file mode 100644 index b7c7b049fca..00000000000 --- a/utils/phpstan/tests/Rule/NarrowReturnToUnionParamRule/Source/BroadReturnOfUnionParam.php +++ /dev/null @@ -1,21 +0,0 @@ -var; - } - - if (! $expr instanceof Variable) { - return null; - } - - return $expr; - } -} diff --git a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php deleted file mode 100644 index 8e381f7c14b..00000000000 --- a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php +++ /dev/null @@ -1,32 +0,0 @@ - - */ -final class NoScopeFetchBeforeCheapGuardRuleTest extends RuleTestCase -{ - public function testScopeFetchBeforeGuard(): void - { - $expectedErrorMessage = 'Move the "return null" guard above ScopeFetcher::fetch(), as it does not use the scope - most nodes then bail out before scope resolution.'; - - $this->analyse([__DIR__ . '/Source/ScopeFetchBeforeGuardRector.php'], [[$expectedErrorMessage, 15]]); - } - - public function testSkip(): void - { - $this->analyse([__DIR__ . '/Source/SkipScopeFetchRector.php'], []); - } - - protected function getRule(): Rule - { - return new NoScopeFetchBeforeCheapGuardRule(); - } -} diff --git a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php deleted file mode 100644 index 10441b9ebf0..00000000000 --- a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php +++ /dev/null @@ -1,22 +0,0 @@ -returnType instanceof Node) { - return null; - } - - return $node; - } -} diff --git a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php deleted file mode 100644 index d8d5ee9d41f..00000000000 --- a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php +++ /dev/null @@ -1,27 +0,0 @@ -returnType instanceof Node) { - return null; - } - - $scope = ScopeFetcher::fetch($node); - if (! $scope->isInClass()) { - return null; - } - - return $node; - } -} From be0e63053d877aa8f6a86136fcd5881dc4bcf841 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 16 Oct 2026 11:56:14 +0200 Subject: [PATCH 2/2] cs --- .../Rector/ClassMethod/AddParamTypeDeclarationRector.php | 2 +- .../Rector/Class_/ChildDoctrineRepositoryClassTypeRector.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php index 745851e56dc..c7f08102e6b 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php @@ -88,7 +88,7 @@ public function refactor(Node $node): ?Node $this->hasChanged = false; // skip class without parents/interfaces, there is no param type to mimic - if ($node instanceof Class_ && !$node->extends instanceof Name && $node->implements === []) { + if ($node instanceof Class_ && ! $node->extends instanceof Name && $node->implements === []) { return null; } diff --git a/rules/TypeDeclaration/Rector/Class_/ChildDoctrineRepositoryClassTypeRector.php b/rules/TypeDeclaration/Rector/Class_/ChildDoctrineRepositoryClassTypeRector.php index a34f09a8f17..5236c56faec 100644 --- a/rules/TypeDeclaration/Rector/Class_/ChildDoctrineRepositoryClassTypeRector.php +++ b/rules/TypeDeclaration/Rector/Class_/ChildDoctrineRepositoryClassTypeRector.php @@ -96,7 +96,7 @@ public function getNodeTypes(): array public function refactor(Node $node): ?Node { // entity type is resolved from the @extends docblock, bail early when absent - if (!$node->getDocComment() instanceof Doc) { + if (! $node->getDocComment() instanceof Doc) { return null; }