Skip to content

Commit

Permalink
[CodeQuality] Resolve type on Collection<> docblock to correct target…
Browse files Browse the repository at this point in the history
… type when has indexBy
  • Loading branch information
samsonasik committed Dec 22, 2024
1 parent 1523ce5 commit 1b6a19d
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Trainer
private $trainings = [];

/**
* @return \Doctrine\Common\Collections\Collection<\Rector\Doctrine\Tests\CodeQuality\Rector\Class_\AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector\Source\Training>
* @return \Doctrine\Common\Collections\Collection<string, \Rector\Doctrine\Tests\CodeQuality\Rector\Class_\AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector\Source\Training>
*/
public function getTrainings()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class WithIndexBy
{
/**
* @ORM\OneToMany(targetEntity=Training::class, mappedBy="trainer", "indexBy"="id")
* @var \Doctrine\Common\Collections\Collection<\Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training>
* @var \Doctrine\Common\Collections\Collection<string, \Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\Training>
*/
private $trainings = [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@

namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source;

use Doctrine\ORM\Mapping as ORM;

final class Training
{

/**
* @ORM\Column(name: 'id', type: 'string')
* @ORM\Id
*/
private string $id;
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ public function refactor(Node $node): ?Node
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);
$newVarType = $this->collectionTypeFactory->createType(
$collectionObjectType,
$this->collectionTypeResolver->hasIndexBy($property)
$this->collectionTypeResolver->hasIndexBy($property),
$property
);
$this->phpDocTypeChanger->changeReturnType($classMethod, $phpDocInfo, $newVarType);
$hasChanged = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ private function refactorPropertyPhpDocInfo(Property $property, PhpDocInfo $phpD

$newVarType = $this->collectionTypeFactory->createType(
$collectionObjectType,
$this->collectionTypeResolver->hasIndexBy($property)
$this->collectionTypeResolver->hasIndexBy($property),
$property
);
$this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $newVarType);
} else {
Expand All @@ -221,7 +222,8 @@ private function refactorPropertyPhpDocInfo(Property $property, PhpDocInfo $phpD

$newVarType = $this->collectionTypeFactory->createType(
$collectionObjectType,
$this->collectionTypeResolver->hasIndexBy($property)
$this->collectionTypeResolver->hasIndexBy($property),
$property
);
$this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $newVarType);
}
Expand Down Expand Up @@ -257,7 +259,8 @@ private function refactorAttribute(Expr $expr, PhpDocInfo $phpDocInfo, Property

$genericObjectType = $this->collectionTypeFactory->createType(
$fullyQualifiedObjectType,
$this->collectionTypeResolver->hasIndexBy($property)
$this->collectionTypeResolver->hasIndexBy($property),
$property
);
$this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $genericObjectType);

Expand Down
3 changes: 2 additions & 1 deletion rules/CodeQuality/SetterCollectionResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ public function resolveAssignedGenericCollectionType(Class_ $class, ClassMethod
if ($soleType instanceof ArrayType && $soleType->getItemType() instanceof ObjectType) {
return $this->collectionTypeFactory->createType(
$soleType->getItemType(),
$this->collectionTypeResolver->hasIndexBy($property)
$this->collectionTypeResolver->hasIndexBy($property),
$property
);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/NodeManipulator/ToManyRelationPropertyTypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ private function resolveTypeFromTargetEntity(Expr|string $targetEntity, Property

return $this->collectionTypeFactory->createType(
$fullyQualifiedObjectType,
$this->collectionTypeResolver->hasIndexBy($property)
$this->collectionTypeResolver->hasIndexBy($property),
$property
);
}
}
31 changes: 28 additions & 3 deletions src/TypeAnalyzer/CollectionTypeFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,41 @@

namespace Rector\Doctrine\TypeAnalyzer;

use PhpParser\Node\Param;
use PhpParser\Node\Stmt\Property;
use PHPStan\Type\Generic\GenericObjectType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use Rector\PhpParser\AstResolver;

final class CollectionTypeFactory
final readonly class CollectionTypeFactory
{
public function createType(ObjectType $objectType, bool $withIndexBy): GenericObjectType
public function __construct(
private AstResolver $astResolver

Check failure on line 18 in src/TypeAnalyzer/CollectionTypeFactory.php

View workflow job for this annotation

GitHub Actions / code_analysis_reusable / PHPStan

Property Rector\Doctrine\TypeAnalyzer\CollectionTypeFactory::$astResolver is never read, only written.
) {
}

public function createType(ObjectType $objectType, bool $withIndexBy, Property|Param $property): GenericObjectType
{
$genericTypes = $withIndexBy ? [$objectType] : [new IntegerType(), $objectType];
$keyType = new IntegerType();

if ($withIndexBy) {
$keyType = $this->resolveKeyType($property, $objectType->getClassName());
}

$genericTypes = [$keyType, $objectType];

return new GenericObjectType('Doctrine\Common\Collections\Collection', $genericTypes);
}

private function resolveKeyType(Property|Param $property, string $className): IntegerType|StringType

Check failure on line 35 in src/TypeAnalyzer/CollectionTypeFactory.php

View workflow job for this annotation

GitHub Actions / code_analysis_reusable / PHPStan

Method Rector\Doctrine\TypeAnalyzer\CollectionTypeFactory::resolveKeyType() never returns PHPStan\Type\StringType so it can be removed from the return type.
{
// todo, resolve type from annotation/attribute
// -> use AstResolver to get target Class
// -> get property type from it
// -> resolve from its annotation/attribute
// fallback to IntegerType
return new IntegerType();
}
}

0 comments on commit 1b6a19d

Please sign in to comment.