fix: handle repeatable attributes

[ph-fritsche]

Introduces isTransient on AttributeDriver for better handling of repeatable attributes. Cleans up AttributeReader API a little.

Closes #8755

[greg0ire]

@ph-fritsche can $a really be an array?

[ph-fritsche]

            $annot = /* is_array($a) ? $a[0] : */ $a;

results in phpunit reporting:

There was 1 error:

1) Doctrine\Tests\ORM\Mapping\AttributeDriverTest::testIsTransientWithRepeatableAttributes
TypeError: get_class(): Argument #1 ($object) must be of type object, array given

/workspaces/misc/doctrine-orm/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php:52
/workspaces/misc/doctrine-orm/tests/Doctrine/Tests/ORM/Mapping/AttributeDriverTest.php:85

due to:

orm/lib/Doctrine/ORM/Mapping/Driver/AttributeReader.php

Lines 26 to 28 in 332c314

	public function getClassAnnotations(ReflectionClass $class): array
	{
	return $this->convertToAttributeInstances($class->getAttributes());

orm/lib/Doctrine/ORM/Mapping/Driver/AttributeReader.php

Lines 66 to 85 in 332c314

	private function convertToAttributeInstances(array $attributes): array
	{
	$instances = [];
	foreach ($attributes as $attribute) {
	// Make sure we only get Doctrine Annotations
	if (! is_subclass_of($attribute->getName(), Annotation::class)) {
	continue;
	}
	$instance = $attribute->newInstance();
	if ($this->isRepeatable($attribute->getName())) {
	$instances[$attribute->getName()][] = $instance;
	} else {
	$instances[$attribute->getName()] = $instance;
	}
	}
	return $instances;

[greg0ire]

Indeed. The phpdoc of convertToAttributeInstances is wrong, this is why the build fails.

[greg0ire]

Also, the phpdoc of Doctrine\Common\Annotations\Reader is wrong, and that's in another package 😬

[greg0ire]

Or rather AttributeReader does not implement the interface it says it implements. Widening the return type in the interface wouldn't be a BC break for classes implementing it, but it would be a BC break for calling code. Not sure what the best course of action would be here @beberlei . I think we should widen it in doctrine/annotations, but to be clean, that would require a major release 😬 or since it's just phpdoc, we pray it does not break to many static analysis builds and release it as a minor?

[ph-fritsche]

Maybe Doctrine\ORM\AttributeDriver should not extend Doctrine\Persistence\Mapping\Driver\AnnotationDriver at all.
How about we internally refactor Doctrine\Persistence\Mapping\Driver\AnnotationDriver::getAllClassNames into another class that could be reused by Doctrine\ORM\AttributeDriver?
This way doctrine/persistence and doctrine/annotation would not have any breaking changes.

If you want to be sentimental about the version increment on this package, one could argue that the declaration of Doctrine\ORM\AttributeDriver was broken from the beginning in 2.9 so breaking that inheritance here might be released as a fix.

[beberlei]

That is a course of action we could take for the next minor release. To fix this bug the small solution plus suppression of tooling errors is my preference.

[greg0ire]

I just thought of another quite simple solution that does not imply changing other packages: let's use an iterable object instead of an array to represent a bug of attributes. As a bonus it will make the code clearer and will answer can $a really be an array? because you will have $a instanceof RepeatableAttributeCollection instead of is_array($a). WDYT?

[greg0ire]

Here is what it would look like: ph-fritsche#1

[beberlei]

Can you exclude AttributeReader from php-stan and psalm analysis for now? We should upgrade to check with 8.0 in 2.10 branch.

[beberlei]

@ph-fritsche just thought the ping pong fixing some config changes might going to be tedious and made the change myself for now. Lets see what the CI says about this.

[beberlei]

This is good for now to fix the error.

I would say we could improve this in the future (2.10) by changing AttributeReader to have only three functions getAnnotations() : array<Annotation>, getFilteredAnnotations(string $className) : array<Annotation> and getAnnotation(string $className) : Annotation which throws when the class name is repeatable.