Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CodeQuality] Skip intersection property var type on ImproveDoctrineCollectionDocTypeInEntityRector #344

Closed
wants to merge 2 commits into from
Closed

[CodeQuality] Skip intersection property var type on ImproveDoctrineCollectionDocTypeInEntityRector #344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b43419c
[CodeQuality] Skip intersection on ImproveDoctrineCollectionDocTypeIn…
samsonasik Sep 1, 2024
1a93fe8
[CodeQuality] Skip intersection on ImproveDoctrineCollectionDocTypeIn…
samsonasik Sep 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions ...nEntityRector/Fixture/OneToMany/Attribute/skip_collection_intersection_selectable.php.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

namespace Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Fixture\OneToMany\Attribute;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Selectable;
use Doctrine\Common\Collections\Collection;
use Doctrine\ORM\Mapping as ORM;
use Rector\Doctrine\Tests\CodeQuality\Rector\Property\ImproveDoctrineCollectionDocTypeInEntityRector\Source\CountryRef;

#[ORM\Entity]
class SkipCollectionIntersectionSelectable
{
/**
* @var Collection<int, CountryRef>&Selectable
*/
#[ORM\OneToMany(mappedBy: 'country', targetEntity: CountryRef::class, cascade: ['persist'])]
private Collection $countryRefs;
Comment on lines +14 to +18
Copy link
Member

@TomasVotruba TomasVotruba Sep 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this fixes proposed issue, it doesn't make sense from PHP points of view.

This code should be updated as well:

private Collection $countryRefs;

Also, in the case of using matching() in many places, this would be correct:

private ArrayCollection $countryRefs;

But this is out of scope of this rule. I'd recommend to create custom rule to use ArrayCollection if possible. This is not a propper fix, as it creates ambiguous difference between PHP and docblock analysis.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not possible to use ArrayCollection as type. Depending on the state of the object, it is either ArrayCollection or PersistentCollection.


public function __construct()
{
$this->countryRefs = new ArrayCollection();
}
}
8 changes: 7 additions & 1 deletion rules/CodeQuality/Rector/Property/ImproveDoctrineCollectionDocTypeInEntityRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger;
use Rector\BetterPhpDocParser\ValueObject\Type\BracketsAwareIntersectionTypeNode;
use Rector\Doctrine\CodeQuality\Enum\CollectionMapping;
use Rector\Doctrine\CodeQuality\Enum\EntityMappingKey;
use Rector\Doctrine\CodeQuality\SetterCollectionResolver;
Expand Down Expand Up @@ -230,10 +231,11 @@ private function refactorAttribute(Expr $expr, PhpDocInfo $phpDocInfo, Property
$property,
CollectionMapping::TO_MANY_CLASSES
);

$phpDocVarTagValueNode = $phpDocInfo->getVarTagValueNode();
if ($toManyAttribute instanceof Attribute) {
$targetEntityClassName = $this->targetEntityResolver->resolveFromAttribute($toManyAttribute);
} else {
$phpDocVarTagValueNode = $phpDocInfo->getVarTagValueNode();
$phpDocCollectionVarTagValueNode = $this->collectionVarTagValueNodeResolver->resolve($property);

if ($phpDocVarTagValueNode instanceof VarTagValueNode && ! $phpDocCollectionVarTagValueNode instanceof VarTagValueNode) {
Expand All @@ -249,6 +251,10 @@ private function refactorAttribute(Expr $expr, PhpDocInfo $phpDocInfo, Property

$fullyQualifiedObjectType = new FullyQualifiedObjectType($targetEntityClassName);

if ($phpDocVarTagValueNode instanceof VarTagValueNode && $phpDocVarTagValueNode->type instanceof BracketsAwareIntersectionTypeNode) {
return null;
}

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

Expand Down
5 changes: 4 additions & 1 deletion stubs/Common/Collections/ArrayCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@
return;
}

class ArrayCollection implements Collection
class ArrayCollection implements Collection, Selectable
{
public function matching(Criteria $criteria)
{
}

public function add(mixed $element)
{
Expand Down
14 changes: 14 additions & 0 deletions stubs/Common/Collections/Selectable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace Doctrine\Common\Collections;

if (interface_exists('Doctrine\Common\Collections\Selectable')) {
return;
}

interface Selectable
{
public function matching(Criteria $criteria);
}
Loading