fix AttributeReader:getPropertyAnnotation AttributeReader:getMethodAnnotation

[andriinavrotskii]

| Bug fix? | yes

[scyzoryck]

Hello @andriinavrotskii !

Thank you for your PR! Could you tell a bit more about the issue that is solves? Is it possible to add some unit tests that was not working before this changes?

Best, Marcin.

[andriinavrotskii]

Hello @andriinavrotskii !

Thank you for your PR! Could you tell a bit more about the issue that is solves? Is it possible to add some unit tests that was not working before this changes?

Best, Marcin.

Hi, Marcin.

Problem:

Doctrine\Common\Annotations\PsrCachedReader::getClassAnnotation(): Argument #1 ($class) must be of type ReflectionClass, ReflectionProperty given, called in /var/www/vendor/jms/serializer/src/Metadata/Driver/AttributeDriver/AttributeReader.php on line 68

Method Doctrine\Common\Annotations\Reader::getClassAnnotation(ReflectionClass $class, $annotationName) has attribute "$class" typehinted as ReflectionClass

Method JMS\Serializer\Metadata\Driver\AttributeDriver\AttributeReader::getPropertyAnnotation(ReflectionProperty $property, $annotationName) got "$property" typehinted as ReflectionProperty

Same for getMethodAnnotation - we try to send instance of ReflectionMethod to method getClassAnnotation which expect instance of ReflectionClass

[goetas]

is there a way to provide a test case for this?

[ixarlie]

Looking closer at the AttributeReader class it seems the getMethodAnnotation and getPropertyAnnotation are not using their counterparts from the delegated reader.

I think the solution from this commit is not correct because when you call getMethodAnnotation or getPropertyAnnotation you expect to get the annotation instance from the method or from the property respectively, not from the class annotations.

IMO the fix is to call to the delegated reader with the same method names.

public function getMethodAnnotation(ReflectionMethod $method, $annotationName): ?object
{
    $attributes = $method->getAttributes($annotationName);

    return $this->reader->getMethodAnnotation($method, $annotationName) ?? $this->buildAnnotation($attributes);
}

public function getPropertyAnnotation(ReflectionProperty $property, $annotationName): ?object
{
    $attributes = $property->getAttributes($annotationName);

    return $this->reader->getPropertyAnnotation($property, $annotationName) ?? $this->buildAnnotation($attributes);
}

[ixarlie]

Suggested change

	return $this->reader->getClassAnnotation(new ReflectionClass($method->class), $annotationName)
	?? $this->buildAnnotation($attributes);
	return $this->reader->getMethodAnnotation($method, $annotationName) ?? $this->buildAnnotation($attributes);

[ixarlie]

Suggested change

	return $this->reader->getClassAnnotation(new ReflectionClass($property->class), $annotationName)
	?? $this->buildAnnotation($attributes);
	return $this->reader->getPropertyAnnotation($property, $annotationName) ?? $this->buildAnnotation($attributes);

[goetas]

@mbabker you have worked on this area...would you mind having a look at this?