From 77e5a628ea64b31e994da055fb26dbfccdb29b50 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 12 Sep 2025 14:55:57 +0200 Subject: [PATCH 1/3] [type-declaration-docblocks] Add AddParamArrayDocblockFromDataProviderRector --- config/set/type-declaration-docblocks.php | 2 + ...rrayDocblockFromDataProviderRectorTest.php | 28 ++++ .../Fixture/some_test_with_data_provider.php | 21 +++ .../some_test_with_data_provider.php.inc | 46 ++++++ .../config/configured_rule.php | 9 ++ .../ValueObject/DataProviderNodes.php | 2 +- .../Enum/TestClassName.php | 13 ++ .../NodeFinder/DataProviderMethodsFinder.php | 60 +++++++ ...ramArrayDocblockFromDataProviderRector.php | 149 ++++++++++++++++++ 9 files changed, 329 insertions(+), 1 deletion(-) create mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/AddParamArrayDocblockFromDataProviderRectorTest.php create mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php create mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php.inc create mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/config/configured_rule.php create mode 100644 rules/TypeDeclarationDocblocks/Enum/TestClassName.php create mode 100644 rules/TypeDeclarationDocblocks/NodeFinder/DataProviderMethodsFinder.php create mode 100644 rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php diff --git a/config/set/type-declaration-docblocks.php b/config/set/type-declaration-docblocks.php index 79b2806614e..90b75fcc9ae 100644 --- a/config/set/type-declaration-docblocks.php +++ b/config/set/type-declaration-docblocks.php @@ -6,6 +6,7 @@ use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnArrayDocblockBasedOnArrayMapRector; use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnDocblockForScalarArrayFromAssignsRector; use Rector\TypeDeclarationDocblocks\Rector\Class_\DocblockVarFromParamDocblockInConstructorRector; +use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\AddParamArrayDocblockFromDataProviderRector; use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\AddParamArrayDocblockFromDimFetchAccessRector; use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\AddReturnDocblockForCommonObjectDenominatorRector; use Rector\TypeDeclarationDocblocks\Rector\ClassMethod\DocblockGetterReturnArrayFromPropertyDocblockVarRector; @@ -22,5 +23,6 @@ DocblockGetterReturnArrayFromPropertyDocblockVarRector::class, AddReturnDocblockForCommonObjectDenominatorRector::class, AddParamArrayDocblockFromDimFetchAccessRector::class, + AddParamArrayDocblockFromDataProviderRector::class, ]); }; diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/AddParamArrayDocblockFromDataProviderRectorTest.php b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/AddParamArrayDocblockFromDataProviderRectorTest.php new file mode 100644 index 00000000000..b9bcc91f16f --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/AddParamArrayDocblockFromDataProviderRectorTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php new file mode 100644 index 00000000000..77e7b143cce --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php @@ -0,0 +1,21 @@ + diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php.inc b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php.inc new file mode 100644 index 00000000000..2a295c77cab --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php.inc @@ -0,0 +1,46 @@ + +----- + diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/config/configured_rule.php b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/config/configured_rule.php new file mode 100644 index 00000000000..1bfdc172f37 --- /dev/null +++ b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules([AddParamArrayDocblockFromDataProviderRector::class]); diff --git a/rules/TypeDeclaration/ValueObject/DataProviderNodes.php b/rules/TypeDeclaration/ValueObject/DataProviderNodes.php index 89d52da3690..71c6817c2b3 100644 --- a/rules/TypeDeclaration/ValueObject/DataProviderNodes.php +++ b/rules/TypeDeclaration/ValueObject/DataProviderNodes.php @@ -10,7 +10,7 @@ final readonly class DataProviderNodes { /** - * @param array $nodes + * @param array $nodes */ public function __construct( public array $nodes, diff --git a/rules/TypeDeclarationDocblocks/Enum/TestClassName.php b/rules/TypeDeclarationDocblocks/Enum/TestClassName.php new file mode 100644 index 00000000000..46d52440678 --- /dev/null +++ b/rules/TypeDeclarationDocblocks/Enum/TestClassName.php @@ -0,0 +1,13 @@ +phpDocInfoFactory->createFromNode($classMethod); + if ($phpDocInfo instanceof PhpDocInfo) { + $phpdocNodes = $phpDocInfo->getTagsByName('@dataProvider'); + } else { + $phpdocNodes = []; + } + + $attributes = $this->findDataProviderAttributes($classMethod); + + return new DataProviderNodes([...$attributes, ...$phpdocNodes]); + } + + /** + * @return array + */ + private function findDataProviderAttributes(ClassMethod $classMethod): array + { + $dataProviders = []; + + foreach ($classMethod->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attribute) { + if (! $this->nodeNameResolver->isName($attribute->name, TestClassName::DATA_PROVIDER)) { + continue; + } + + $dataProviders[] = $attribute; + } + } + + return $dataProviders; + } +} diff --git a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php new file mode 100644 index 00000000000..c6f8ca76d78 --- /dev/null +++ b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php @@ -0,0 +1,149 @@ +> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?Node + { + if ($node->getParams() === []) { + return null; + } + + if (! $this->testsNodeAnalyzer->isTestClassMethod($node)) { + return null; + } + + $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); + + // nothing relevant here + $dataProviderNodes = $this->dataProviderMethodsFinder->findDataProviderNodes($node); + if ($dataProviderNodes->isEmpty()) { + return null; + } + + $hasChanged = false; + + foreach ($node->getParams() as $param) { + if (! $param->type instanceof Node) { + continue; + } + + if (! $this->isName($param->type, 'array')) { + continue; + } + + /** @var string $paramName */ + $paramName = $this->getName($param->var); + + $paramTagValueNode = $phpDocInfo->getParamTagValueByName($paramName); + + // already defined, lets skip it + if ($paramTagValueNode instanceof ParamTagValueNode) { + continue; + } + + dump($dataProviderNodes); + die; + + // @todo start here + // $paramTagValueNode = $this->createParamTagValueNode($paramName, 'string'); + // $phpDocInfo->addTagValueNode($paramTagValueNode); + // $hasChanged = true; + // continue; + } + + // if ($hasChanged === false) { + // return null; + // } + + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node); + + return $node; + } +} From 8f251785ea075853d3e6ed478a981018103ed132 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 12 Sep 2025 15:13:50 +0200 Subject: [PATCH 2/3] improve data provider methods finder --- ...amTypeBasedOnPHPUnitDataProviderRector.php | 115 ++---------------- .../ValueObject/DataProviderNodes.php | 78 +++++++++++- .../NodeFinder/DataProviderMethodsFinder.php | 9 +- ...ramArrayDocblockFromDataProviderRector.php | 79 ++++++------ 4 files changed, 133 insertions(+), 148 deletions(-) diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php index 91ca5eb6bc7..ca1f7abb17e 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php @@ -4,25 +4,17 @@ namespace Rector\TypeDeclaration\Rector\ClassMethod; -use Nette\Utils\Strings; use PhpParser\Node; use PhpParser\Node\ArrayItem; -use PhpParser\Node\Attribute; -use PhpParser\Node\AttributeGroup; use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\Yield_; -use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Return_; -use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode; -use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\MixedType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; -use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo; -use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory; use Rector\PhpParser\Node\BetterNodeFinder; use Rector\PHPStanStaticTypeMapper\Enum\TypeKind; @@ -30,6 +22,7 @@ use Rector\Rector\AbstractRector; use Rector\StaticTypeMapper\StaticTypeMapper; use Rector\TypeDeclaration\ValueObject\DataProviderNodes; +use Rector\TypeDeclarationDocblocks\NodeFinder\DataProviderMethodsFinder; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -43,16 +36,10 @@ final class AddParamTypeBasedOnPHPUnitDataProviderRector extends AbstractRector */ private const ERROR_MESSAGE = 'Adds param type declaration based on PHPUnit provider return type declaration'; - /** - * @see https://regex101.com/r/hW09Vt/1 - * @var string - */ - private const METHOD_NAME_REGEX = '#^(?\w+)(\(\))?#'; - public function __construct( private readonly TypeFactory $typeFactory, private readonly TestsNodeAnalyzer $testsNodeAnalyzer, - private readonly PhpDocInfoFactory $phpDocInfoFactory, + private readonly DataProviderMethodsFinder $dataProviderMethodsFinder, private readonly BetterNodeFinder $betterNodeFinder, private readonly StaticTypeMapper $staticTypeMapper, ) { @@ -131,12 +118,12 @@ public function refactor(Node $node): ?Node continue; } - $dataProviderNodes = $this->resolveDataProviderNodes($classMethod); - if ($dataProviderNodes->isEmpty()) { + $dataProviderNodes = $this->dataProviderMethodsFinder->findDataProviderNodes($node, $classMethod); + if ($dataProviderNodes->getClassMethods() === []) { continue; } - $hasClassMethodChanged = $this->refactorClassMethod($classMethod, $node, $dataProviderNodes->nodes); + $hasClassMethodChanged = $this->refactorClassMethod($classMethod, $dataProviderNodes); if ($hasClassMethodChanged) { $hasChanged = true; } @@ -149,16 +136,8 @@ public function refactor(Node $node): ?Node return null; } - private function inferParam( - Class_ $class, - int $parameterPosition, - PhpDocTagNode | Attribute $dataProviderNode - ): Type { - $dataProviderClassMethod = $this->resolveDataProviderClassMethod($class, $dataProviderNode); - if (! $dataProviderClassMethod instanceof ClassMethod) { - return new MixedType(); - } - + private function inferParam(int $parameterPosition, ClassMethod $dataProviderClassMethod): Type + { $returns = $this->betterNodeFinder->findReturnsScoped($dataProviderClassMethod); if ($returns !== []) { return $this->resolveReturnStaticArrayTypeByParameterPosition($returns, $parameterPosition); @@ -169,33 +148,6 @@ private function inferParam( return $this->resolveYieldStaticArrayTypeByParameterPosition($yields, $parameterPosition); } - private function resolveDataProviderClassMethod( - Class_ $class, - Attribute | PhpDocTagNode $dataProviderNode - ): ?ClassMethod { - if ($dataProviderNode instanceof Attribute) { - $value = $dataProviderNode->args[0]->value; - - if (! $value instanceof String_) { - return null; - } - - $content = $value->value; - } elseif ($dataProviderNode->value instanceof GenericTagValueNode) { - $content = $dataProviderNode->value->value; - } else { - return null; - } - - $match = Strings::match($content, self::METHOD_NAME_REGEX); - if ($match === null) { - return null; - } - - $methodName = $match['method_name']; - return $class->getMethod($methodName); - } - /** * @param Return_[] $returns */ @@ -289,47 +241,7 @@ private function resolveParamOnPositionTypes(Array_ $array, int $parameterPositi return $paramOnPositionTypes; } - private function resolveDataProviderNodes(ClassMethod $classMethod): DataProviderNodes - { - $attributes = $this->getPhpDataProviderAttributes($classMethod); - - $classMethodPhpDocInfo = $this->phpDocInfoFactory->createFromNode($classMethod); - - $phpdocNodes = $classMethodPhpDocInfo instanceof PhpDocInfo ? - $classMethodPhpDocInfo->getTagsByName('@dataProvider') : []; - - return new DataProviderNodes([...$attributes, ...$phpdocNodes]); - } - - /** - * @return array - */ - private function getPhpDataProviderAttributes(ClassMethod $classMethod): array - { - $attributeName = 'PHPUnit\Framework\Attributes\DataProvider'; - - /** @var AttributeGroup[] $attrGroups */ - $attrGroups = $classMethod->attrGroups; - - $dataProviders = []; - - foreach ($attrGroups as $attrGroup) { - foreach ($attrGroup->attrs as $attribute) { - if (! $this->isName($attribute->name, $attributeName)) { - continue; - } - - $dataProviders[] = $attribute; - } - } - - return $dataProviders; - } - - /** - * @param array $dataProviderNodes - */ - private function refactorClassMethod(ClassMethod $classMethod, Class_ $class, array $dataProviderNodes): bool + private function refactorClassMethod(ClassMethod $classMethod, DataProviderNodes $dataProviderNodes): bool { $hasChanged = false; @@ -343,19 +255,18 @@ private function refactorClassMethod(ClassMethod $classMethod, Class_ $class, ar } $paramTypes = []; - foreach ($dataProviderNodes as $dataProviderNode) { - $paramTypes[] = $this->inferParam($class, $parameterPosition, $dataProviderNode); + foreach ($dataProviderNodes->getClassMethods() as $dataProviderClassMethod) { + $paramTypes[] = $this->inferParam($parameterPosition, $dataProviderClassMethod); } $paramTypeDeclaration = TypeCombinator::union(...$paramTypes); - if ($paramTypeDeclaration instanceof MixedType) { continue; } - $type = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($paramTypeDeclaration, TypeKind::PARAM); - if ($type instanceof Node) { - $param->type = $type; + $typeNode = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($paramTypeDeclaration, TypeKind::PARAM); + if ($typeNode instanceof Node) { + $param->type = $typeNode; $hasChanged = true; } } diff --git a/rules/TypeDeclaration/ValueObject/DataProviderNodes.php b/rules/TypeDeclaration/ValueObject/DataProviderNodes.php index 71c6817c2b3..e6bba080c7a 100644 --- a/rules/TypeDeclaration/ValueObject/DataProviderNodes.php +++ b/rules/TypeDeclaration/ValueObject/DataProviderNodes.php @@ -4,21 +4,93 @@ namespace Rector\TypeDeclaration\ValueObject; +use Nette\Utils\Strings; use PhpParser\Node\Attribute; +use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\Class_; +use PhpParser\Node\Stmt\ClassMethod; +use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; +use Webmozart\Assert\Assert; final readonly class DataProviderNodes { /** - * @param array $nodes + * @see https://regex101.com/r/hW09Vt/1 + * @var string + */ + private const METHOD_NAME_REGEX = '#^(?\w+)(\(\))?#'; + + /** + * @param Attribute[] $attributes + * @param PhpDocTagNode[] $phpDocTagNodes */ public function __construct( - public array $nodes, + private Class_ $class, + private array $attributes, + private array $phpDocTagNodes, ) { + Assert::allIsInstanceOf($attributes, Attribute::class); + Assert::allIsInstanceOf($phpDocTagNodes, PhpDocTagNode::class); } public function isEmpty(): bool { - return $this->nodes === []; + return $this->getClassMethods() === []; + } + + /** + * @return ClassMethod[] + */ + public function getClassMethods(): array + { + $classMethods = []; + + foreach ($this->phpDocTagNodes as $phpDocTagNode) { + if ($phpDocTagNode->value instanceof GenericTagValueNode) { + $methodName = $this->matchMethodName($phpDocTagNode->value->value); + if (! is_string($methodName)) { + continue; + } + + $classMethod = $this->class->getMethod($methodName); + if (! $classMethod instanceof ClassMethod) { + continue; + } + + $classMethods[] = $classMethod; + } + } + + foreach ($this->attributes as $attribute) { + $value = $attribute->args[0]->value; + if (! $value instanceof String_) { + continue; + } + + $methodName = $this->matchMethodName($value->value); + if (! is_string($methodName)) { + continue; + } + + $classMethod = $this->class->getMethod($methodName); + if (! $classMethod instanceof ClassMethod) { + continue; + } + + $classMethods[] = $classMethod; + } + + return $classMethods; + } + + private function matchMethodName(string $content): ?string + { + $match = Strings::match($content, self::METHOD_NAME_REGEX); + if ($match === null) { + return null; + } + + return $match['method_name']; } } diff --git a/rules/TypeDeclarationDocblocks/NodeFinder/DataProviderMethodsFinder.php b/rules/TypeDeclarationDocblocks/NodeFinder/DataProviderMethodsFinder.php index a08902a2124..3670320368c 100644 --- a/rules/TypeDeclarationDocblocks/NodeFinder/DataProviderMethodsFinder.php +++ b/rules/TypeDeclarationDocblocks/NodeFinder/DataProviderMethodsFinder.php @@ -5,6 +5,7 @@ namespace Rector\TypeDeclarationDocblocks\NodeFinder; use PhpParser\Node\Attribute; +use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; @@ -12,10 +13,6 @@ use Rector\TypeDeclaration\ValueObject\DataProviderNodes; use Rector\TypeDeclarationDocblocks\Enum\TestClassName; -/** - * @todo re-use in rector-phpunit - * @see AddParamTypeBasedOnPHPUnitDataProviderRector - */ final readonly class DataProviderMethodsFinder { public function __construct( @@ -24,7 +21,7 @@ public function __construct( ) { } - public function findDataProviderNodes(ClassMethod $classMethod): DataProviderNodes + public function findDataProviderNodes(Class_ $class, ClassMethod $classMethod): DataProviderNodes { $phpDocInfo = $this->phpDocInfoFactory->createFromNode($classMethod); if ($phpDocInfo instanceof PhpDocInfo) { @@ -35,7 +32,7 @@ public function findDataProviderNodes(ClassMethod $classMethod): DataProviderNod $attributes = $this->findDataProviderAttributes($classMethod); - return new DataProviderNodes([...$attributes, ...$phpdocNodes]); + return new DataProviderNodes($class, $attributes, $phpdocNodes); } /** diff --git a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php index c6f8ca76d78..7c4f884fe40 100644 --- a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php +++ b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php @@ -5,7 +5,7 @@ namespace Rector\TypeDeclarationDocblocks\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Class_; use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; use Rector\Comments\NodeDocBlock\DocBlockUpdater; @@ -83,66 +83,71 @@ public static function provideData() */ public function getNodeTypes(): array { - return [ClassMethod::class]; + return [Class_::class]; } /** - * @param ClassMethod $node + * @param Class_ $node */ public function refactor(Node $node): ?Node { - if ($node->getParams() === []) { - return null; - } - - if (! $this->testsNodeAnalyzer->isTestClassMethod($node)) { - return null; - } - - $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); - - // nothing relevant here - $dataProviderNodes = $this->dataProviderMethodsFinder->findDataProviderNodes($node); - if ($dataProviderNodes->isEmpty()) { + if (! $this->testsNodeAnalyzer->isInTestClass($node)) { return null; } $hasChanged = false; - foreach ($node->getParams() as $param) { - if (! $param->type instanceof Node) { + foreach ($node->getMethods() as $classMethod) { + if ($classMethod->getParams() === []) { continue; } - if (! $this->isName($param->type, 'array')) { + if (! $this->testsNodeAnalyzer->isTestClassMethod($classMethod)) { continue; } - /** @var string $paramName */ - $paramName = $this->getName($param->var); - - $paramTagValueNode = $phpDocInfo->getParamTagValueByName($paramName); + $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod); - // already defined, lets skip it - if ($paramTagValueNode instanceof ParamTagValueNode) { + $dataProviderNodes = $this->dataProviderMethodsFinder->findDataProviderNodes($node, $classMethod); + if ($dataProviderNodes->getClassMethods() === []) { continue; } - dump($dataProviderNodes); - die; + foreach ($classMethod->getParams() as $paramPosition => $param) { + // we are intersted only in array params + if (! $param->type instanceof Node || ! $this->isName($param->type, 'array')) { + continue; + } + + /** @var string $paramName */ + $paramName = $this->getName($param->var); + + $paramTagValueNode = $phpDocInfo->getParamTagValueByName($paramName); + + // already defined, lets skip it + if ($paramTagValueNode instanceof ParamTagValueNode) { + continue; + } + + foreach ($dataProviderNodes->getClassMethods() as $dataProviderClassMethod) { + // try to resolve array type on position X + dump($paramPosition); + die; + } + + // @todo start here + // $paramTagValueNode = $this->createParamTagValueNode($paramName, 'string'); + // $phpDocInfo->addTagValueNode($paramTagValueNode); + // $hasChanged = true; + // continue; + } - // @todo start here - // $paramTagValueNode = $this->createParamTagValueNode($paramName, 'string'); - // $phpDocInfo->addTagValueNode($paramTagValueNode); - // $hasChanged = true; - // continue; + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node); } - // if ($hasChanged === false) { - // return null; - // } - - $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node); + if (! $hasChanged) { + return null; + } return $node; } From 7be191032f4d2e11337e69cab0a47c3c72bd40ff Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 12 Sep 2025 15:32:01 +0200 Subject: [PATCH 3/3] extract ParameterTypeFromDataProviderResolver --- .../Fixture/some_test_with_data_provider.php | 21 --- ...amTypeBasedOnPHPUnitDataProviderRector.php | 127 +-------------- .../ParameterTypeFromDataProviderResolver.php | 151 ++++++++++++++++++ ...ramArrayDocblockFromDataProviderRector.php | 42 +++-- 4 files changed, 185 insertions(+), 156 deletions(-) delete mode 100644 rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php create mode 100644 rules/TypeDeclaration/TypeAnalyzer/ParameterTypeFromDataProviderResolver.php diff --git a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php b/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php deleted file mode 100644 index 77e7b143cce..00000000000 --- a/rules-tests/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector/Fixture/some_test_with_data_provider.php +++ /dev/null @@ -1,21 +0,0 @@ - diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php index ca1f7abb17e..95d8d2534cf 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php @@ -5,22 +5,14 @@ namespace Rector\TypeDeclaration\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\ArrayItem; -use PhpParser\Node\Expr\Array_; -use PhpParser\Node\Expr\Yield_; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Return_; -use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\MixedType; -use PHPStan\Type\Type; -use PHPStan\Type\TypeCombinator; -use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory; -use Rector\PhpParser\Node\BetterNodeFinder; use Rector\PHPStanStaticTypeMapper\Enum\TypeKind; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\Rector\AbstractRector; use Rector\StaticTypeMapper\StaticTypeMapper; +use Rector\TypeDeclaration\TypeAnalyzer\ParameterTypeFromDataProviderResolver; use Rector\TypeDeclaration\ValueObject\DataProviderNodes; use Rector\TypeDeclarationDocblocks\NodeFinder\DataProviderMethodsFinder; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; @@ -37,10 +29,9 @@ final class AddParamTypeBasedOnPHPUnitDataProviderRector extends AbstractRector private const ERROR_MESSAGE = 'Adds param type declaration based on PHPUnit provider return type declaration'; public function __construct( - private readonly TypeFactory $typeFactory, private readonly TestsNodeAnalyzer $testsNodeAnalyzer, private readonly DataProviderMethodsFinder $dataProviderMethodsFinder, - private readonly BetterNodeFinder $betterNodeFinder, + private readonly ParameterTypeFromDataProviderResolver $parameterTypeFromDataProviderResolver, private readonly StaticTypeMapper $staticTypeMapper, ) { } @@ -136,111 +127,6 @@ public function refactor(Node $node): ?Node return null; } - private function inferParam(int $parameterPosition, ClassMethod $dataProviderClassMethod): Type - { - $returns = $this->betterNodeFinder->findReturnsScoped($dataProviderClassMethod); - if ($returns !== []) { - return $this->resolveReturnStaticArrayTypeByParameterPosition($returns, $parameterPosition); - } - - /** @var Yield_[] $yields */ - $yields = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped($dataProviderClassMethod, Yield_::class); - return $this->resolveYieldStaticArrayTypeByParameterPosition($yields, $parameterPosition); - } - - /** - * @param Return_[] $returns - */ - private function resolveReturnStaticArrayTypeByParameterPosition(array $returns, int $parameterPosition): Type - { - $firstReturnedExpr = $returns[0]->expr; - - if (! $firstReturnedExpr instanceof Array_) { - return new MixedType(); - } - - $paramOnPositionTypes = $this->resolveParamOnPositionTypes($firstReturnedExpr, $parameterPosition); - if ($paramOnPositionTypes === []) { - return new MixedType(); - } - - return $this->typeFactory->createMixedPassedOrUnionType($paramOnPositionTypes); - } - - /** - * @param Yield_[] $yields - */ - private function resolveYieldStaticArrayTypeByParameterPosition(array $yields, int $parameterPosition): Type - { - $paramOnPositionTypes = []; - - foreach ($yields as $yield) { - if (! $yield->value instanceof Array_) { - continue; - } - - $type = $this->getTypeFromClassMethodYield($yield->value); - - if (! $type instanceof ConstantArrayType) { - return $type; - } - - foreach ($type->getValueTypes() as $position => $valueType) { - if ($position !== $parameterPosition) { - continue; - } - - $paramOnPositionTypes[] = $valueType; - } - } - - if ($paramOnPositionTypes === []) { - return new MixedType(); - } - - return $this->typeFactory->createMixedPassedOrUnionType($paramOnPositionTypes); - } - - private function getTypeFromClassMethodYield(Array_ $classMethodYieldArray): MixedType | ConstantArrayType - { - $arrayType = $this->nodeTypeResolver->getType($classMethodYieldArray); - - // impossible to resolve - if (! $arrayType instanceof ConstantArrayType) { - return new MixedType(); - } - - return $arrayType; - } - - /** - * @return Type[] - */ - private function resolveParamOnPositionTypes(Array_ $array, int $parameterPosition): array - { - $paramOnPositionTypes = []; - - foreach ($array->items as $singleDataProvidedSet) { - if (! $singleDataProvidedSet instanceof ArrayItem || ! $singleDataProvidedSet->value instanceof Array_) { - return []; - } - - foreach ($singleDataProvidedSet->value->items as $position => $singleDataProvidedSetItem) { - if ($position !== $parameterPosition) { - continue; - } - - if (! $singleDataProvidedSetItem instanceof ArrayItem) { - continue; - } - - $paramOnPositionTypes[] = $this->nodeTypeResolver->getType($singleDataProvidedSetItem->value); - } - } - - return $paramOnPositionTypes; - } - private function refactorClassMethod(ClassMethod $classMethod, DataProviderNodes $dataProviderNodes): bool { $hasChanged = false; @@ -254,12 +140,11 @@ private function refactorClassMethod(ClassMethod $classMethod, DataProviderNodes continue; } - $paramTypes = []; - foreach ($dataProviderNodes->getClassMethods() as $dataProviderClassMethod) { - $paramTypes[] = $this->inferParam($parameterPosition, $dataProviderClassMethod); - } + $paramTypeDeclaration = $this->parameterTypeFromDataProviderResolver->resolve( + $parameterPosition, + $dataProviderNodes->getClassMethods() + ); - $paramTypeDeclaration = TypeCombinator::union(...$paramTypes); if ($paramTypeDeclaration instanceof MixedType) { continue; } diff --git a/rules/TypeDeclaration/TypeAnalyzer/ParameterTypeFromDataProviderResolver.php b/rules/TypeDeclaration/TypeAnalyzer/ParameterTypeFromDataProviderResolver.php new file mode 100644 index 00000000000..f24d7e72517 --- /dev/null +++ b/rules/TypeDeclaration/TypeAnalyzer/ParameterTypeFromDataProviderResolver.php @@ -0,0 +1,151 @@ +resolveParameterTypeFromDataProvider($parameterPosition, $dataProviderClassMethod); + } + + return TypeCombinator::union(...$paramTypes); + } + + private function resolveParameterTypeFromDataProvider( + int $parameterPosition, + ClassMethod $dataProviderClassMethod + ): Type { + $returns = $this->betterNodeFinder->findReturnsScoped($dataProviderClassMethod); + if ($returns !== []) { + return $this->resolveReturnStaticArrayTypeByParameterPosition($returns, $parameterPosition); + } + + /** @var Yield_[] $yields */ + $yields = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped($dataProviderClassMethod, Yield_::class); + return $this->resolveYieldStaticArrayTypeByParameterPosition($yields, $parameterPosition); + } + + /** + * @param Return_[] $returns + */ + private function resolveReturnStaticArrayTypeByParameterPosition(array $returns, int $parameterPosition): Type + { + $firstReturnedExpr = $returns[0]->expr; + + if (! $firstReturnedExpr instanceof Array_) { + return new MixedType(); + } + + $paramOnPositionTypes = $this->resolveParamOnPositionTypes($firstReturnedExpr, $parameterPosition); + if ($paramOnPositionTypes === []) { + return new MixedType(); + } + + return $this->typeFactory->createMixedPassedOrUnionType($paramOnPositionTypes); + } + + /** + * @param Yield_[] $yields + */ + private function resolveYieldStaticArrayTypeByParameterPosition(array $yields, int $parameterPosition): Type + { + $paramOnPositionTypes = []; + + foreach ($yields as $yield) { + if (! $yield->value instanceof Array_) { + continue; + } + + $type = $this->getTypeFromClassMethodYield($yield->value); + + if (! $type instanceof ConstantArrayType) { + return $type; + } + + foreach ($type->getValueTypes() as $position => $valueType) { + if ($position !== $parameterPosition) { + continue; + } + + $paramOnPositionTypes[] = $valueType; + } + } + + if ($paramOnPositionTypes === []) { + return new MixedType(); + } + + return $this->typeFactory->createMixedPassedOrUnionType($paramOnPositionTypes); + } + + private function getTypeFromClassMethodYield(Array_ $classMethodYieldArray): MixedType | ConstantArrayType + { + $arrayType = $this->nodeTypeResolver->getType($classMethodYieldArray); + + // impossible to resolve + if (! $arrayType instanceof ConstantArrayType) { + return new MixedType(); + } + + return $arrayType; + } + + /** + * @return Type[] + */ + private function resolveParamOnPositionTypes(Array_ $array, int $parameterPosition): array + { + $paramOnPositionTypes = []; + + foreach ($array->items as $singleDataProvidedSet) { + if (! $singleDataProvidedSet instanceof ArrayItem || ! $singleDataProvidedSet->value instanceof Array_) { + return []; + } + + foreach ($singleDataProvidedSet->value->items as $position => $singleDataProvidedSetItem) { + if ($position !== $parameterPosition) { + continue; + } + + if (! $singleDataProvidedSetItem instanceof ArrayItem) { + continue; + } + + $paramOnPositionTypes[] = $this->nodeTypeResolver->getType($singleDataProvidedSetItem->value); + } + } + + return $paramOnPositionTypes; + } +} diff --git a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php index 7c4f884fe40..38b5fb1c078 100644 --- a/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php +++ b/rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php @@ -10,7 +10,10 @@ use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; use Rector\Comments\NodeDocBlock\DocBlockUpdater; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; +use Rector\Privatization\TypeManipulator\TypeNormalizer; use Rector\Rector\AbstractRector; +use Rector\StaticTypeMapper\StaticTypeMapper; +use Rector\TypeDeclaration\TypeAnalyzer\ParameterTypeFromDataProviderResolver; use Rector\TypeDeclarationDocblocks\NodeFinder\DataProviderMethodsFinder; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -24,7 +27,10 @@ public function __construct( private readonly PhpDocInfoFactory $phpDocInfoFactory, private readonly DocBlockUpdater $docBlockUpdater, private readonly TestsNodeAnalyzer $testsNodeAnalyzer, - private readonly DataProviderMethodsFinder $dataProviderMethodsFinder + private readonly DataProviderMethodsFinder $dataProviderMethodsFinder, + private readonly ParameterTypeFromDataProviderResolver $parameterTypeFromDataProviderResolver, + private readonly StaticTypeMapper $staticTypeMapper, + private readonly TypeNormalizer $typeNormalizer, ) { } @@ -114,8 +120,12 @@ public function refactor(Node $node): ?Node } foreach ($classMethod->getParams() as $paramPosition => $param) { - // we are intersted only in array params - if (! $param->type instanceof Node || ! $this->isName($param->type, 'array')) { + // we are interested only in array params + if (! $param->type instanceof Node) { + continue; + } + + if (! $this->isName($param->type, 'array')) { continue; } @@ -129,20 +139,24 @@ public function refactor(Node $node): ?Node continue; } - foreach ($dataProviderNodes->getClassMethods() as $dataProviderClassMethod) { - // try to resolve array type on position X - dump($paramPosition); - die; - } + $parameterType = $this->parameterTypeFromDataProviderResolver->resolve( + $paramPosition, + $dataProviderNodes->getClassMethods() + ); + + $generalizedParameterType = $this->typeNormalizer->generalizeConstantBoolTypes($parameterType); + + $parameterTypeNode = $this->staticTypeMapper->mapPHPStanTypeToPHPStanPhpDocTypeNode( + $generalizedParameterType + ); + + $paramTagValueNode = new ParamTagValueNode($parameterTypeNode, false, '$' . $paramName, '', false); + $phpDocInfo->addTagValueNode($paramTagValueNode); + $hasChanged = true; - // @todo start here - // $paramTagValueNode = $this->createParamTagValueNode($paramName, 'string'); - // $phpDocInfo->addTagValueNode($paramTagValueNode); - // $hasChanged = true; - // continue; + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($classMethod); } - $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node); } if (! $hasChanged) {