From ff83202664b5257f1d899c481ac6a7bdbf4c9729 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 2 Dec 2025 16:01:35 +0100 Subject: [PATCH 1/7] enable phpstan rules --- composer.json | 1 + phpstan.neon | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/composer.json b/composer.json index a671ca32..c74ae27c 100644 --- a/composer.json +++ b/composer.json @@ -17,6 +17,7 @@ "rector/rector-src": "dev-main", "rector/swiss-knife": "^1.0", "rector/type-perfect": "^2.1", + "symplify/phpstan-rules": "*", "symplify/phpstan-extensions": "^12.0", "symplify/vendor-patches": "^11.5", "tomasvotruba/class-leak": "^1.2", diff --git a/phpstan.neon b/phpstan.neon index ef336f69..ab69f3e8 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,3 +1,7 @@ +includes: + - vendor/symplify/phpstan-rules/config/symplify-rules.neon + - vendor/symplify/phpstan-rules/config/rector-rules.neon + parameters: level: 8 From 38168ff8a5a221c0374388be72f0f0fc179e039a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 2 Dec 2025 16:36:25 +0100 Subject: [PATCH 2/7] bump rector --- composer.json | 2 +- rector.php | 5 ++--- .../Rector/Class_/AddProphecyTraitRector.php | 10 +++------- src/Enum/ProphecyClassName.php | 13 +++++++++++++ 4 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 src/Enum/ProphecyClassName.php diff --git a/composer.json b/composer.json index c74ae27c..d05a48d8 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ "rector/rector-src": "dev-main", "rector/swiss-knife": "^1.0", "rector/type-perfect": "^2.1", - "symplify/phpstan-rules": "*", + "symplify/phpstan-rules": "^14.9.3", "symplify/phpstan-extensions": "^12.0", "symplify/vendor-patches": "^11.5", "tomasvotruba/class-leak": "^1.2", diff --git a/rector.php b/rector.php index 621cc49e..2bbcf3d7 100644 --- a/rector.php +++ b/rector.php @@ -21,11 +21,10 @@ '*/Fixture/*', '*/Expected/*', - // object types + // object types must be string as class might be missing + to keep downgrade safe StringClassNameToClassConstantRector::class => [ - __DIR__ . '/src/NodeAnalyzer/TestsNodeAnalyzer.php', __DIR__ . '/config', - __DIR__ . '/src/NodeFinder/DataProviderClassMethodFinder.php', + __DIR__ . '/src/Enum', ], ]) ->withPhpSets() diff --git a/rules/PHPUnit100/Rector/Class_/AddProphecyTraitRector.php b/rules/PHPUnit100/Rector/Class_/AddProphecyTraitRector.php index e0f0d10c..266f0a4c 100644 --- a/rules/PHPUnit100/Rector/Class_/AddProphecyTraitRector.php +++ b/rules/PHPUnit100/Rector/Class_/AddProphecyTraitRector.php @@ -10,6 +10,7 @@ use PhpParser\Node\Stmt\TraitUse; use PHPStan\Reflection\ClassReflection; use Rector\PhpParser\Node\BetterNodeFinder; +use Rector\PHPUnit\Enum\ProphecyClassName; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\Rector\AbstractRector; use Rector\Reflection\ReflectionResolver; @@ -25,11 +26,6 @@ */ final class AddProphecyTraitRector extends AbstractRector { - /** - * @var string - */ - private const PROPHECY_TRAIT = 'Prophecy\PhpUnit\ProphecyTrait'; - public function __construct( private readonly TestsNodeAnalyzer $testsNodeAnalyzer, private readonly ReflectionResolver $reflectionResolver, @@ -91,7 +87,7 @@ public function refactor(Node $node): ?Node return null; } - $traitUse = new TraitUse([new FullyQualified(self::PROPHECY_TRAIT)]); + $traitUse = new TraitUse([new FullyQualified(ProphecyClassName::PROPHECY_TRAIT)]); $node->stmts = array_merge([$traitUse], $node->stmts); @@ -114,6 +110,6 @@ private function shouldSkipClass(Class_ $class): bool return false; } - return $classReflection->hasTraitUse(self::PROPHECY_TRAIT); + return $classReflection->hasTraitUse(ProphecyClassName::PROPHECY_TRAIT); } } diff --git a/src/Enum/ProphecyClassName.php b/src/Enum/ProphecyClassName.php new file mode 100644 index 00000000..6bb8c2a0 --- /dev/null +++ b/src/Enum/ProphecyClassName.php @@ -0,0 +1,13 @@ + Date: Tue, 2 Dec 2025 16:40:12 +0100 Subject: [PATCH 3/7] [enum] extract phpunit attribute to decouple class names from code --- .../NodeFactory/RequiresAttributeFactory.php | 17 ++++++------- .../RemoveNamedArgsInDataProviderRector.php | 3 +-- src/Enum/PHPUnitAttribute.php | 24 +++++++++++++++++++ 3 files changed, 34 insertions(+), 10 deletions(-) create mode 100644 src/Enum/PHPUnitAttribute.php diff --git a/rules/AnnotationsToAttributes/NodeFactory/RequiresAttributeFactory.php b/rules/AnnotationsToAttributes/NodeFactory/RequiresAttributeFactory.php index fcf754d1..fb1f4075 100644 --- a/rules/AnnotationsToAttributes/NodeFactory/RequiresAttributeFactory.php +++ b/rules/AnnotationsToAttributes/NodeFactory/RequiresAttributeFactory.php @@ -6,6 +6,7 @@ use PhpParser\Node\AttributeGroup; use Rector\PhpAttribute\NodeFactory\PhpAttributeGroupFactory; +use Rector\PHPUnit\Enum\PHPUnitAttribute; final readonly class RequiresAttributeFactory { @@ -23,7 +24,7 @@ public function create(string $annotationValue): ?AttributeGroup switch ($type) { case 'PHP': - $attributeClass = 'PHPUnit\Framework\Attributes\RequiresPhp'; + $attributeClass = PHPUnitAttribute::REQUIRES_PHP; // only version is used, we need to prefix with >= if (is_string($attributeValue) && is_numeric($attributeValue[0])) { @@ -33,7 +34,7 @@ public function create(string $annotationValue): ?AttributeGroup $attributeValue = [$attributeValue]; break; case 'PHPUnit': - $attributeClass = 'PHPUnit\Framework\Attributes\RequiresPhpunit'; + $attributeClass = PHPUnitAttribute::REQUIRES_PHPUNIT; // only version is used, we need to prefix with >= if (is_string($attributeValue) && is_numeric($attributeValue[0])) { @@ -43,30 +44,30 @@ public function create(string $annotationValue): ?AttributeGroup $attributeValue = [$attributeValue]; break; case 'OS': - $attributeClass = 'PHPUnit\Framework\Attributes\RequiresOperatingSystem'; + $attributeClass = PHPUnitAttribute::REQUIRES_OS; $attributeValue = [$attributeValue]; break; case 'OSFAMILY': - $attributeClass = 'PHPUnit\Framework\Attributes\RequiresOperatingSystemFamily'; + $attributeClass = PHPUnitAttribute::REQUIRES_OS_FAMILY; $attributeValue = [$attributeValue]; break; case 'function': if (str_contains((string) $attributeValue, '::')) { - $attributeClass = 'PHPUnit\Framework\Attributes\RequiresMethod'; + $attributeClass = PHPUnitAttribute::REQUIRES_METHOD; $attributeValue = explode('::', (string) $attributeValue); $attributeValue[0] .= '::class'; } else { - $attributeClass = 'PHPUnit\Framework\Attributes\RequiresFunction'; + $attributeClass = PHPUnitAttribute::REQUIRES_FUNCTION; $attributeValue = [$attributeValue]; } break; case 'extension': - $attributeClass = 'PHPUnit\Framework\Attributes\RequiresPhpExtension'; + $attributeClass = PHPUnitAttribute::REQUIRES_PHP_EXTENSION; $attributeValue = explode(' ', (string) $attributeValue, 2); break; case 'setting': - $attributeClass = 'PHPUnit\Framework\Attributes\RequiresSetting'; + $attributeClass = PHPUnitAttribute::REQUIRES_SETTING; $attributeValue = explode(' ', (string) $attributeValue, 2); break; default: diff --git a/rules/PHPUnit100/Rector/Class_/RemoveNamedArgsInDataProviderRector.php b/rules/PHPUnit100/Rector/Class_/RemoveNamedArgsInDataProviderRector.php index 3b91c99b..9b8794d7 100644 --- a/rules/PHPUnit100/Rector/Class_/RemoveNamedArgsInDataProviderRector.php +++ b/rules/PHPUnit100/Rector/Class_/RemoveNamedArgsInDataProviderRector.php @@ -14,13 +14,12 @@ use PhpParser\Node\Stmt\Expression; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\PHPUnit\NodeFinder\DataProviderClassMethodFinder; -use Rector\PHPUnit\Tests\PHPUnit100\Rector\Class_\RemoveNamedArgsInDataProviderRector\RemoveNamedArgsInDataProviderRectorTest; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see RemoveNamedArgsInDataProviderRectorTest + * @see \Rector\PHPUnit\Tests\PHPUnit100\Rector\Class_\RemoveNamedArgsInDataProviderRector\RemoveNamedArgsInDataProviderRectorTest */ final class RemoveNamedArgsInDataProviderRector extends AbstractRector { diff --git a/src/Enum/PHPUnitAttribute.php b/src/Enum/PHPUnitAttribute.php new file mode 100644 index 00000000..218bbebf --- /dev/null +++ b/src/Enum/PHPUnitAttribute.php @@ -0,0 +1,24 @@ + Date: Tue, 2 Dec 2025 16:51:04 +0100 Subject: [PATCH 4/7] [cleanup] AnnotationWithValueToAttributeRector to avoid using local node property --- phpstan.neon | 8 +- .../{some_class.php.inc => some_test.php.inc} | 0 .../AnnotationWithValueToAttributeRector.php | 94 ++++++++++--------- ...esAnnotationWithValueToAttributeRector.php | 11 ++- ...eTestFunctionPrefixWithAttributeRector.php | 3 +- .../Class_/AddParamTypeFromDependsRector.php | 7 +- src/Enum/PHPUnitAttribute.php | 2 + src/Enum/PHPUnitClassName.php | 5 + src/NodeAnalyzer/TestsNodeAnalyzer.php | 10 +- 9 files changed, 83 insertions(+), 57 deletions(-) rename rules-tests/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector/Fixture/{some_class.php.inc => some_test.php.inc} (100%) diff --git a/phpstan.neon b/phpstan.neon index ab69f3e8..331f65b0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -44,4 +44,10 @@ parameters: - '#Doing instanceof PHPStan\\Type\\.+ is error\-prone and deprecated#' - identifier: instanceof.alwaysTrue - - identifier: assign.propertyType + + # false positive + - + identifier: assign.propertyType + message: '#Property PhpParser\\Node\\Identifier\:\:\$name \(non\-empty\-string\) does not accept string#' + path: rules/CodeQuality/Rector/ClassMethod/ReplaceTestFunctionPrefixWithAttributeRector.php + diff --git a/rules-tests/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector/Fixture/some_class.php.inc b/rules-tests/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector/Fixture/some_test.php.inc similarity index 100% rename from rules-tests/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector/Fixture/some_class.php.inc rename to rules-tests/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector/Fixture/some_test.php.inc diff --git a/rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php b/rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php index 03e05f02..cfde6b6d 100644 --- a/rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php +++ b/rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php @@ -34,7 +34,7 @@ final class AnnotationWithValueToAttributeRector extends AbstractRector implemen */ private array $annotationWithValueToAttributes = []; - private ?Class_ $currentClass = null; + private bool $hasChanged = false; public function __construct( private readonly PhpDocTagRemover $phpDocTagRemover, @@ -86,7 +86,7 @@ final class SomeTest extends TestCase */ public function getNodeTypes(): array { - return [Class_::class, ClassMethod::class]; + return [Class_::class]; } public function provideMinPhpVersion(): int @@ -95,24 +95,61 @@ public function provideMinPhpVersion(): int } /** - * @param Class_|ClassMethod $node + * @param Class_ $node */ public function refactor(Node $node): ?Node { + if (! $this->testsNodeAnalyzer->isInTestClass($node)) { return null; } - if ($node instanceof Class_) { - $this->currentClass = $node; + $this->hasChanged = false; + + // handle class level + // $classPhpDocInfo = $this->phpDocInfoFactory->createFromNode($node); + $this->refactorClassMethodOrClass($node, $node); + + foreach ($node->getMethods() as $classMethod) { + $this->refactorClassMethodOrClass($classMethod, $node); } - $phpDocInfo = $this->phpDocInfoFactory->createFromNode($node); - if (! $phpDocInfo instanceof PhpDocInfo) { + if (! $this->hasChanged) { return null; } - $hasChanged = false; + return $node; + } + + /** + * @param mixed[] $configuration + */ + public function configure(array $configuration): void + { + Assert::allIsInstanceOf($configuration, AnnotationWithValueToAttribute::class); + $this->annotationWithValueToAttributes = $configuration; + } + + private function resolveAttributeValue( + GenericTagValueNode $genericTagValueNode, + AnnotationWithValueToAttribute $annotationWithValueToAttribute + ): mixed { + $valueMap = $annotationWithValueToAttribute->getValueMap(); + if ($valueMap === []) { + // no map? convert value as it is + return $genericTagValueNode->value; + } + + $originalValue = strtolower($genericTagValueNode->value); + return $valueMap[$originalValue]; + } + + private function refactorClassMethodOrClass(ClassMethod|Class_ $classOrClass, Class_ $class): void + { + $phpDocInfo = $this->phpDocInfoFactory->createFromNode($classOrClass); + if (! $phpDocInfo instanceof PhpDocInfo) { + return; + } foreach ($this->annotationWithValueToAttributes as $annotationWithValueToAttribute) { /** @var PhpDocTagNode[] $desiredTagValueNodes */ @@ -133,47 +170,18 @@ public function refactor(Node $node): ?Node [$attributeValue] ); - if ($node instanceof ClassMethod && $annotationWithValueToAttribute->getIsOnClassLevel() && $this->currentClass instanceof Class_) { - Assert::isAOf($this->currentClass, Class_::class); - $this->currentClass->attrGroups = array_merge($this->currentClass->attrGroups, [$attributeGroup]); + if ($annotationWithValueToAttribute->getIsOnClassLevel()) { + $class->attrGroups = array_merge($class->attrGroups, [$attributeGroup]); } else { - $node->attrGroups = array_merge($node->attrGroups, [$attributeGroup]); + $classOrClass->attrGroups = array_merge($classOrClass->attrGroups, [$attributeGroup]); } // cleanup $this->phpDocTagRemover->removeTagValueFromNode($phpDocInfo, $desiredTagValueNode); - $hasChanged = true; - } - } - - if ($hasChanged) { - $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node); - return $node; - } - - return null; - } + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($classOrClass); - /** - * @param mixed[] $configuration - */ - public function configure(array $configuration): void - { - Assert::allIsInstanceOf($configuration, AnnotationWithValueToAttribute::class); - $this->annotationWithValueToAttributes = $configuration; - } - - private function resolveAttributeValue( - GenericTagValueNode $genericTagValueNode, - AnnotationWithValueToAttribute $annotationWithValueToAttribute - ): mixed { - $valueMap = $annotationWithValueToAttribute->getValueMap(); - if ($valueMap === []) { - // no map? convert value as it is - return $genericTagValueNode->value; + $this->hasChanged = true; + } } - - $originalValue = strtolower($genericTagValueNode->value); - return $valueMap[$originalValue]; } } diff --git a/rules/AnnotationsToAttributes/Rector/Class_/RequiresAnnotationWithValueToAttributeRector.php b/rules/AnnotationsToAttributes/Rector/Class_/RequiresAnnotationWithValueToAttributeRector.php index b7ba8d8a..f0a097d5 100644 --- a/rules/AnnotationsToAttributes/Rector/Class_/RequiresAnnotationWithValueToAttributeRector.php +++ b/rules/AnnotationsToAttributes/Rector/Class_/RequiresAnnotationWithValueToAttributeRector.php @@ -121,7 +121,7 @@ public function refactor(Node $node): ?Node } /** - * @return array + * @return array */ private function handleRequires(PhpDocInfo $phpDocInfo): array { @@ -133,7 +133,13 @@ private function handleRequires(PhpDocInfo $phpDocInfo): array } $requires = $desiredTagValueNode->value->value; - $attributeGroups[$requires] = $this->requiresAttributeFactory->create($requires); + + $requiresAttributeGroup = $this->requiresAttributeFactory->create($requires); + if (! $requiresAttributeGroup instanceof AttributeGroup) { + continue; + } + + $attributeGroups[$requires] = $requiresAttributeGroup; $this->phpDocTagRemover->removeTagValueFromNode($phpDocInfo, $desiredTagValueNode); } @@ -172,6 +178,7 @@ private function refactorClassMethod(ClassMethod $classMethod): ?ClassMethod $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($classMethod); $classMethod->attrGroups = array_merge($classMethod->attrGroups, $requiresAttributeGroups); + $this->removeMethodRequiresAnnotations($phpDocInfo); return $classMethod; diff --git a/rules/CodeQuality/Rector/ClassMethod/ReplaceTestFunctionPrefixWithAttributeRector.php b/rules/CodeQuality/Rector/ClassMethod/ReplaceTestFunctionPrefixWithAttributeRector.php index a88a5b4b..2fb782c7 100644 --- a/rules/CodeQuality/Rector/ClassMethod/ReplaceTestFunctionPrefixWithAttributeRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/ReplaceTestFunctionPrefixWithAttributeRector.php @@ -9,6 +9,7 @@ use PhpParser\Node\Stmt\ClassMethod; use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer; use Rector\PhpAttribute\NodeFactory\PhpAttributeGroupFactory; +use Rector\PHPUnit\Enum\PHPUnitAttribute; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; @@ -75,7 +76,7 @@ public function refactor(Node $node): ?Node return null; } - if ($this->phpAttributeAnalyzer->hasPhpAttributes($node, ['PHPUnit\\Framework\\Attributes\\Test'])) { + if ($this->phpAttributeAnalyzer->hasPhpAttributes($node, [PHPUnitAttribute::TEST])) { return null; } diff --git a/rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php b/rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php index 8d9ae2ce..de6c7cfa 100644 --- a/rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php +++ b/rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php @@ -131,8 +131,10 @@ public function refactor(Node $node): ?Node return $node; } - private function resolveReturnTypeOfDependsMethod(ClassMethod $classMethod, Class_ $class): ?Node - { + private function resolveReturnTypeOfDependsMethod( + ClassMethod $classMethod, + Class_ $class + ): Node\ComplexType|Node\Identifier|Node\Name|null { $dependsMethodName = $this->resolveDependsAnnotationOrAttributeMethod($classMethod); if ($dependsMethodName === null || $dependsMethodName === '') { @@ -140,7 +142,6 @@ private function resolveReturnTypeOfDependsMethod(ClassMethod $classMethod, Clas } $dependsClassMethod = $class->getMethod($dependsMethodName); - if (! $dependsClassMethod instanceof ClassMethod) { return null; } diff --git a/src/Enum/PHPUnitAttribute.php b/src/Enum/PHPUnitAttribute.php index 218bbebf..98a2bd4e 100644 --- a/src/Enum/PHPUnitAttribute.php +++ b/src/Enum/PHPUnitAttribute.php @@ -21,4 +21,6 @@ final class PHPUnitAttribute public const REQUIRES_PHP_EXTENSION = 'PHPUnit\Framework\Attributes\RequiresPhpExtension'; public const REQUIRES_SETTING = 'PHPUnit\Framework\Attributes\RequiresSetting'; + + public const TEST = 'PHPUnit\Framework\Attributes\Test'; } diff --git a/src/Enum/PHPUnitClassName.php b/src/Enum/PHPUnitClassName.php index 95fe6289..536477b9 100644 --- a/src/Enum/PHPUnitClassName.php +++ b/src/Enum/PHPUnitClassName.php @@ -30,4 +30,9 @@ final class PHPUnitClassName * @var string */ public const TEST_LISTENER = 'PHPUnit\Framework\TestListener'; + + /** + * @var string[] + */ + public const TEST_CLASSES = [self::TEST_CASE, self::TEST_CASE_LEGACY]; } diff --git a/src/NodeAnalyzer/TestsNodeAnalyzer.php b/src/NodeAnalyzer/TestsNodeAnalyzer.php index 5578ed1b..d2b90e5a 100644 --- a/src/NodeAnalyzer/TestsNodeAnalyzer.php +++ b/src/NodeAnalyzer/TestsNodeAnalyzer.php @@ -13,16 +13,12 @@ use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; use Rector\NodeNameResolver\NodeNameResolver; use Rector\NodeTypeResolver\NodeTypeResolver; +use Rector\PHPUnit\Enum\PHPUnitAttribute; use Rector\PHPUnit\Enum\PHPUnitClassName; use Rector\Reflection\ReflectionResolver; final readonly class TestsNodeAnalyzer { - /** - * @var string[] - */ - private const TEST_CASE_OBJECT_CLASSES = [PHPUnitClassName::TEST_CASE, PHPUnitClassName::TEST_CASE_LEGACY]; - public function __construct( private NodeTypeResolver $nodeTypeResolver, private NodeNameResolver $nodeNameResolver, @@ -38,7 +34,7 @@ public function isInTestClass(Node $node): bool return false; } - foreach (self::TEST_CASE_OBJECT_CLASSES as $testCaseObjectClass) { + foreach (PHPUnitClassName::TEST_CLASSES as $testCaseObjectClass) { if ($classReflection->is($testCaseObjectClass)) { return true; } @@ -59,7 +55,7 @@ public function isTestClassMethod(ClassMethod $classMethod): bool foreach ($classMethod->getAttrGroups() as $attributeGroup) { foreach ($attributeGroup->attrs as $attribute) { - if ($attribute->name->toString() === 'PHPUnit\\Framework\\Attributes\\Test') { + if ($this->nodeNameResolver->isName($attribute->name, PHPUnitAttribute::TEST)) { return true; } } From d22b617e63f96c0e42a6f6b71502a92b2d043092 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 2 Dec 2025 16:58:29 +0100 Subject: [PATCH 5/7] remove node traverser stop from AssertFuncCallToPHPUnitAssertRector --- .../Fixture/skip_in_static_method.php.inc | 18 --------- .../static_assert_in_static_method.php.inc | 38 ++++++++++++++++++ .../AssertFuncCallToPHPUnitAssertRector.php | 39 +++++++++++-------- 3 files changed, 61 insertions(+), 34 deletions(-) delete mode 100644 rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_in_static_method.php.inc create mode 100644 rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/static_assert_in_static_method.php.inc diff --git a/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_in_static_method.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_in_static_method.php.inc deleted file mode 100644 index 1c650f65..00000000 --- a/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_in_static_method.php.inc +++ /dev/null @@ -1,18 +0,0 @@ -getMessage(); - } -} - -?> \ No newline at end of file diff --git a/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/static_assert_in_static_method.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/static_assert_in_static_method.php.inc new file mode 100644 index 00000000..e61eb3a2 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/static_assert_in_static_method.php.inc @@ -0,0 +1,38 @@ +getMessage(); + } +} + +?> +----- +getMessage(); + } +} + +?> diff --git a/rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php b/rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php index 70f720fa..c1d4d8e2 100644 --- a/rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php +++ b/rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php @@ -26,6 +26,7 @@ use Rector\PHPStan\ScopeFetcher; use Rector\PHPUnit\Enum\AssertMethod; use Rector\PHPUnit\Enum\PHPUnitClassName; +use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -36,7 +37,8 @@ final class AssertFuncCallToPHPUnitAssertRector extends AbstractRector { public function __construct( - private readonly ValueResolver $valueResolver + private readonly ValueResolver $valueResolver, + private readonly TestsNodeAnalyzer $testsNodeAnalyzer ) { } @@ -88,21 +90,26 @@ public function getNodeTypes(): array */ public function refactor(Node $node): StaticCall|MethodCall|null|int { - if ($node instanceof ClassMethod) { - if ($node->isStatic()) { - return NodeVisitor::DONT_TRAVERSE_CHILDREN; - } - - return null; - } - - if ($node instanceof Closure) { - if ($node->static) { - return NodeVisitor::DONT_TRAVERSE_CHILDREN; - } - - return null; - } + // @todo handle only in test classes! + + // most liekly hook to ClassMethod :) + traverse then + $this->testsNodeAnalyzer->isInTestClass($node); + +// if ($node instanceof ClassMethod) { +// if ($node->isStatic()) { +// return NodeVisitor::DONT_TRAVERSE_CHILDREN; +// } +// +// return null; +// } + +// if ($node instanceof Closure) { +// if ($node->static) { +// return NodeVisitor::DONT_TRAVERSE_CHILDREN; +// } +// +// return null; +// } if ($node->isFirstClassCallable()) { return null; From 1d2f1b678492a68f2356548655fe26271916a649 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 2 Dec 2025 20:23:28 +0100 Subject: [PATCH 6/7] tidy up AssertFuncCallToPHPUnitAssertRector --- phpstan.neon | 4 + .../assert_inside_static_closure.php.inc | 35 ++++ .../skip_inside_static_closure.php.inc | 15 -- .../Fixture/skip_outside_test_case.php.inc | 11 + .../static_assert_in_static_method.php.inc | 3 +- .../Class_/AddParamTypeFromDependsRector.php | 5 +- .../AssertFuncCallToPHPUnitAssertRector.php | 195 +++++++++--------- 7 files changed, 149 insertions(+), 119 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/assert_inside_static_closure.php.inc delete mode 100644 rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_inside_static_closure.php.inc create mode 100644 rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_outside_test_case.php.inc diff --git a/phpstan.neon b/phpstan.neon index 331f65b0..696f79f9 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -51,3 +51,7 @@ parameters: message: '#Property PhpParser\\Node\\Identifier\:\:\$name \(non\-empty\-string\) does not accept string#' path: rules/CodeQuality/Rector/ClassMethod/ReplaceTestFunctionPrefixWithAttributeRector.php + # handle next + - + identifier: symplify.forbiddenNode + message: '#switch#' diff --git a/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/assert_inside_static_closure.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/assert_inside_static_closure.php.inc new file mode 100644 index 00000000..d13621ce --- /dev/null +++ b/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/assert_inside_static_closure.php.inc @@ -0,0 +1,35 @@ + +----- + diff --git a/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_inside_static_closure.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_inside_static_closure.php.inc deleted file mode 100644 index dc22a2d9..00000000 --- a/rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_inside_static_closure.php.inc +++ /dev/null @@ -1,15 +0,0 @@ -getMessage(); } diff --git a/rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php b/rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php index de6c7cfa..dc6737fc 100644 --- a/rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php +++ b/rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php @@ -6,6 +6,9 @@ use PhpParser\Node; use PhpParser\Node\Attribute; +use PhpParser\Node\ComplexType; +use PhpParser\Node\Identifier; +use PhpParser\Node\Name; use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; @@ -134,7 +137,7 @@ public function refactor(Node $node): ?Node private function resolveReturnTypeOfDependsMethod( ClassMethod $classMethod, Class_ $class - ): Node\ComplexType|Node\Identifier|Node\Name|null { + ): ComplexType|Identifier|Name|null { $dependsMethodName = $this->resolveDependsAnnotationOrAttributeMethod($classMethod); if ($dependsMethodName === null || $dependsMethodName === '') { diff --git a/rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php b/rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php index c1d4d8e2..ae3ce0b7 100644 --- a/rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php +++ b/rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php @@ -19,8 +19,7 @@ use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Name\FullyQualified; -use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\NodeVisitor; +use PhpParser\Node\Stmt\Class_; use PHPStan\Reflection\ClassReflection; use Rector\PhpParser\Node\Value\ValueResolver; use Rector\PHPStan\ScopeFetcher; @@ -81,105 +80,115 @@ public function test() */ public function getNodeTypes(): array { - return [ClassMethod::class, Closure::class, FuncCall::class]; + return [Class_::class]; } /** - * @param ClassMethod|Closure|FuncCall $node - * @return StaticCall|MethodCall|null|NodeVisitor::DONT_TRAVERSE_CHILDREN + * @param Class_ $node */ - public function refactor(Node $node): StaticCall|MethodCall|null|int + public function refactor(Node $node): ?Class_ { - // @todo handle only in test classes! - - // most liekly hook to ClassMethod :) + traverse then - $this->testsNodeAnalyzer->isInTestClass($node); - -// if ($node instanceof ClassMethod) { -// if ($node->isStatic()) { -// return NodeVisitor::DONT_TRAVERSE_CHILDREN; -// } -// -// return null; -// } - -// if ($node instanceof Closure) { -// if ($node->static) { -// return NodeVisitor::DONT_TRAVERSE_CHILDREN; -// } -// -// return null; -// } - - if ($node->isFirstClassCallable()) { + if (! $this->testsNodeAnalyzer->isInTestClass($node) && ! $this->isBehatContext($node)) { return null; } - if (! $this->isName($node, 'assert')) { - return null; - } - - if (! $this->isTestFilePath($node) && ! $this->isBehatContext($node)) { - return null; - } - - $comparedExpr = $node->getArgs()[0] - ->value; + $hasChanged = false; - if ($comparedExpr instanceof Equal) { - $methodName = AssertMethod::ASSERT_EQUALS; - $exprs = [$comparedExpr->right, $comparedExpr->left]; - - } elseif ($comparedExpr instanceof Identical) { - $methodName = AssertMethod::ASSERT_SAME; - $exprs = [$comparedExpr->right, $comparedExpr->left]; - - } elseif ($comparedExpr instanceof NotIdentical) { - if ($this->valueResolver->isNull($comparedExpr->right)) { - $methodName = 'assertNotNull'; - $exprs = [$comparedExpr->left]; - } else { - return null; - } - - } elseif ($comparedExpr instanceof Bool_) { - $methodName = 'assertTrue'; - $exprs = [$comparedExpr]; - } elseif ($comparedExpr instanceof FuncCall) { - if ($this->isName($comparedExpr, 'method_exists')) { - $methodName = 'assertTrue'; - $exprs = [$comparedExpr]; - } else { - return null; - } - } elseif ($comparedExpr instanceof Instanceof_) { - // outside TestCase - $methodName = 'assertInstanceOf'; - $exprs = []; - - if ($comparedExpr->class instanceof FullyQualified) { - $classConstFetch = new ClassConstFetch($comparedExpr->class, 'class'); - $exprs[] = $classConstFetch; - } else { - return null; + foreach ($node->getMethods() as $classMethod) { + if ($classMethod->stmts === null) { + continue; } - $exprs[] = $comparedExpr->expr; - } else { - return null; + $useStaticAssert = $classMethod->isStatic() ?: $this->isBehatContext($node); + + $this->traverseNodesWithCallable($classMethod->stmts, function (Node $node) use ( + &$useStaticAssert, + &$hasChanged + ): null|MethodCall|StaticCall { + if ($node instanceof Closure && $node->static) { + $useStaticAssert = true; + return null; + } + + if (! $node instanceof FuncCall) { + return null; + } + + if ($node->isFirstClassCallable()) { + return null; + } + + if (! $this->isName($node, 'assert')) { + return null; + } + + $comparedExpr = $node->getArgs()[0] + ->value; + + if ($comparedExpr instanceof Equal) { + $methodName = AssertMethod::ASSERT_EQUALS; + $exprs = [$comparedExpr->right, $comparedExpr->left]; + + } elseif ($comparedExpr instanceof Identical) { + $methodName = AssertMethod::ASSERT_SAME; + $exprs = [$comparedExpr->right, $comparedExpr->left]; + + } elseif ($comparedExpr instanceof NotIdentical) { + if ($this->valueResolver->isNull($comparedExpr->right)) { + $methodName = 'assertNotNull'; + $exprs = [$comparedExpr->left]; + } else { + return null; + } + + } elseif ($comparedExpr instanceof Bool_) { + $methodName = 'assertTrue'; + $exprs = [$comparedExpr]; + } elseif ($comparedExpr instanceof FuncCall) { + if ($this->isName($comparedExpr, 'method_exists')) { + $methodName = 'assertTrue'; + $exprs = [$comparedExpr]; + } else { + return null; + } + } elseif ($comparedExpr instanceof Instanceof_) { + // outside TestCase + $methodName = 'assertInstanceOf'; + $exprs = []; + + if ($comparedExpr->class instanceof FullyQualified) { + $classConstFetch = new ClassConstFetch($comparedExpr->class, 'class'); + $exprs[] = $classConstFetch; + } else { + return null; + } + + $exprs[] = $comparedExpr->expr; + } else { + return null; + } + + // is there a comment message + if (isset($node->getArgs()[1])) { + $exprs[] = $node->getArgs()[1]->value; + } + + $hasChanged = true; + + return $this->createAssertCall($methodName, $exprs, $useStaticAssert); + }); } - // is there a comment message - if (isset($node->getArgs()[1])) { - $exprs[] = $node->getArgs()[1]->value; + if (! $hasChanged) { + return null; } - return $this->createCall($node, $methodName, $exprs); + return $node; } - private function isBehatContext(FuncCall $funcCall): bool + private function isBehatContext(Class_ $class): bool { - $scope = ScopeFetcher::fetch($funcCall); + $scope = ScopeFetcher::fetch($class); if (! $scope->getClassReflection() instanceof ClassReflection) { return false; } @@ -191,33 +200,17 @@ private function isBehatContext(FuncCall $funcCall): bool return str_ends_with($className, 'Context'); } - private function isTestFilePath(FuncCall $funcCall): bool - { - $scope = ScopeFetcher::fetch($funcCall); - if (! $scope->getClassReflection() instanceof ClassReflection) { - return false; - } - - $className = $scope->getClassReflection() - ->getName(); - if (str_ends_with($className, 'Test')) { - return true; - } - - return str_ends_with($className, 'TestCase'); - } - /** * @param Expr[] $exprs */ - private function createCall(FuncCall $funcCall, string $methodName, array $exprs): MethodCall|StaticCall + private function createAssertCall(string $methodName, array $exprs, bool $useStaticAssert): MethodCall|StaticCall { $args = []; foreach ($exprs as $expr) { $args[] = new Arg($expr); } - if ($this->isBehatContext($funcCall)) { + if ($useStaticAssert) { $assertFullyQualified = new FullyQualified(PHPUnitClassName::ASSERT); return new StaticCall($assertFullyQualified, $methodName, $args); } From 9706b69f855b0087969ad6d9f276471e7005bac9 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 2 Dec 2025 20:40:39 +0100 Subject: [PATCH 7/7] fixup! tidy up AssertFuncCallToPHPUnitAssertRector --- .../Rector/Class_/AnnotationWithValueToAttributeRector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php b/rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php index cfde6b6d..f206d943 100644 --- a/rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php +++ b/rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php @@ -107,9 +107,9 @@ public function refactor(Node $node): ?Node $this->hasChanged = false; // handle class level - // $classPhpDocInfo = $this->phpDocInfoFactory->createFromNode($node); $this->refactorClassMethodOrClass($node, $node); + // handle method level foreach ($node->getMethods() as $classMethod) { $this->refactorClassMethodOrClass($classMethod, $node); }