Skip to content

Commit

Permalink
Catch and process errors that occure during annotation instantiation
Browse files Browse the repository at this point in the history 
When an annotation class is instantiated and values are passed to the
constructor, TypeErrors and ArgumentCountErrors can be thrown. This
change catches those exceptions and shows an AnnotationException
containing the annotation name and context.
  • Loading branch information
@7ochem
7ochem committed May 17, 2022
1 parent 5b668ae commit 41bce60
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 20 deletions.
5 changes: 3 additions & 2 deletions lib/Doctrine/Common/Annotations/AnnotationException.php
View file
Expand Up @@ -3,6 +3,7 @@
namespace Doctrine\Common\Annotations;

use Exception;
use Throwable;

use function get_class;
use function gettype;
Expand Down Expand Up @@ -47,9 +48,9 @@ public static function semanticalError($message)
*
* @return AnnotationException
*/
public static function creationError($message)
public static function creationError($message, ?Throwable $previous = null)
{
return new self('[Creation Error] ' . $message);
return new self('[Creation Error] ' . $message, 0, $previous);
}

/**
Expand Down
40 changes: 36 additions & 4 deletions lib/Doctrine/Common/Annotations/DocParser.php
View file
Expand Up @@ -12,6 +12,7 @@
use ReflectionProperty;
use RuntimeException;
use stdClass;
use Throwable;

use function array_keys;
use function array_map;
Expand Down Expand Up @@ -941,7 +942,7 @@ private function Annotation()

if (self::$annotationMetadata[$name]['has_named_argument_constructor']) {
if (PHP_VERSION_ID >= 80000) {
return new $name(...$values);
return $this->instantiateAnnotiation($originalName, $this->context, $name, $values);
}

$positionalValues = [];
Expand All @@ -968,16 +969,16 @@ private function Annotation()
$positionalValues[self::$annotationMetadata[$name]['constructor_args'][$property]['position']] = $value;
}

return new $name(...$positionalValues);
return $this->instantiateAnnotiation($originalName, $this->context, $name, $positionalValues);
}

// check if the annotation expects values via the constructor,
// or directly injected into public properties
if (self::$annotationMetadata[$name]['has_constructor'] === true) {
return new $name($values);
return $this->instantiateAnnotiation($originalName, $this->context, $name, [$values]);
}

$instance = new $name();
$instance = $this->instantiateAnnotiation($originalName, $this->context, $name, []);

foreach ($values as $property => $value) {
if (! isset(self::$annotationMetadata[$name]['properties'][$property])) {
Expand Down Expand Up @@ -1456,4 +1457,35 @@ private function resolvePositionalValues(array $arguments, string $name): array

return $values;
}

/**
* Try to instantiate the annotation and catch and process any exceptions related to failure
*
* @param class-string $name
* @param array<string,mixed> $arguments
*
* @return object
*
* @throws AnnotationException
*/
private function instantiateAnnotiation(string $originalName, string $context, string $name, array $arguments)
{
try {
return new $name(...$arguments);
} catch (Throwable $exception) {
throw AnnotationException::creationError(
sprintf(
<<<'EXCEPTION'
An error occurred while instantiating the annotation
@%s declared on %s: "%s".
EXCEPTION
,
$originalName,
$context,
$exception->getMessage()
),
$exception
);
}
}
}
106 changes: 92 additions & 14 deletions tests/Doctrine/Tests/Common/Annotations/DocParserTest.php
View file
Expand Up @@ -14,15 +14,19 @@
use Doctrine\Tests\Common\Annotations\Fixtures\ClassWithConstants;
use Doctrine\Tests\Common\Annotations\Fixtures\InterfaceWithConstants;
use InvalidArgumentException;
use PHPUnit\Framework\Constraint\ExceptionMessage;
use PHPUnit\Framework\TestCase;
use ReflectionClass;
use TypeError;

use function array_column;
use function array_combine;
use function assert;
use function class_exists;
use function extension_loaded;
use function get_parent_class;
use function ini_get;
use function method_exists;
use function sprintf;
use function ucfirst;

Expand Down Expand Up @@ -886,9 +890,17 @@ public function testAnnotationEnumInvalidTypeDeclarationException(): void
$docblock = '@Doctrine\Tests\Common\Annotations\Fixtures\AnnotationEnumInvalid("foo")';

$parser->setIgnoreNotImportedAnnotations(false);
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('@Enum supports only scalar values "array" given.');
$parser->parse($docblock);
$this->expectException(AnnotationException::class);
try {
$parser->parse($docblock);
} catch (AnnotationException $exc) {
$previous = $exc->getPrevious();
$this->assertNotEmpty($previous);
$this->assertInstanceOf(InvalidArgumentException::class, $previous);
$this->assertThat($previous, new ExceptionMessage('@Enum supports only scalar values "array" given.'));

throw $exc;
}
}

public function testAnnotationEnumInvalidLiteralDeclarationException(): void
Expand All @@ -897,9 +909,20 @@ public function testAnnotationEnumInvalidLiteralDeclarationException(): void
$docblock = '@Doctrine\Tests\Common\Annotations\Fixtures\AnnotationEnumLiteralInvalid("foo")';

$parser->setIgnoreNotImportedAnnotations(false);
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('Undefined enumerator value "3" for literal "AnnotationEnumLiteral::THREE".');
$parser->parse($docblock);
$this->expectException(AnnotationException::class);
try {
$parser->parse($docblock);
} catch (AnnotationException $exc) {
$previous = $exc->getPrevious();
$this->assertNotEmpty($previous);
$this->assertInstanceOf(InvalidArgumentException::class, $previous);
$this->assertThat(
$previous,
new ExceptionMessage('Undefined enumerator value "3" for literal "AnnotationEnumLiteral::THREE".')
);

throw $exc;
}
}

/**
Expand Down Expand Up @@ -1100,11 +1123,22 @@ public function testAnnotationWithInvalidTargetDeclarationError(): void
DOCBLOCK;

$parser->setTarget(Target::TARGET_CLASS);
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage(
'Invalid Target "Foo". Available targets: [ALL, CLASS, METHOD, PROPERTY, FUNCTION, ANNOTATION]'
);
$parser->parse($docblock, $context);
$this->expectException(AnnotationException::class);
try {
$parser->parse($docblock, $context);
} catch (AnnotationException $exc) {
$previous = $exc->getPrevious();
$this->assertNotEmpty($previous);
$this->assertInstanceOf(InvalidArgumentException::class, $previous);
$this->assertThat(
$previous,
new ExceptionMessage(
'Invalid Target "Foo". Available targets: [ALL, CLASS, METHOD, PROPERTY, FUNCTION, ANNOTATION]'
)
);

throw $exc;
}
}

public function testAnnotationWithTargetEmptyError(): void
Expand All @@ -1118,9 +1152,20 @@ public function testAnnotationWithTargetEmptyError(): void
DOCBLOCK;

$parser->setTarget(Target::TARGET_CLASS);
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('@Target expects either a string value, or an array of strings, "NULL" given.');
$parser->parse($docblock, $context);
$this->expectException(AnnotationException::class);
try {
$parser->parse($docblock, $context);
} catch (AnnotationException $exc) {
$previous = $exc->getPrevious();
$this->assertNotEmpty($previous);
$this->assertInstanceOf(InvalidArgumentException::class, $previous);
$this->assertThat(
$previous,
new ExceptionMessage('@Target expects either a string value, or an array of strings, "NULL" given.')
);

throw $exc;
}
}

/**
Expand Down Expand Up @@ -1683,6 +1728,39 @@ public function testNamedArgumentsConstructorAnnotationWithInvalidArguments(): v
);
$parser->parse('/** @AnotherNamedAnnotation("foo", bar=666, "hey") */');
}

public function testNamedArgumentsConstructorAnnotationWithWrongArgumentType(): void
{
$context = 'property SomeClassName::invalidProperty.';
$docblock = '@NamedAnnotationWithArray(foo = "no array!")';
$parser = $this->createTestParser();
$this->expectException(AnnotationException::class);
$this->expectExceptionMessageMatches(
'/\[Creation Error\] An error occurred while instantiating the annotation
@NamedAnnotationWithArray declared on property SomeClassName::invalidProperty\.: ".*"\.$/'
);
try {
$parser->parse($docblock, $context);
} catch (AnnotationException $exc) {
$previous = $exc->getPrevious();
$this->assertNotEmpty($previous);
$this->assertInstanceOf(TypeError::class, $previous);

throw $exc;
}
}

/**
* Override for BC with PHPUnit <8
*/
public function expectExceptionMessageMatches(string $regularExpression): void
{
if (method_exists(get_parent_class($this), 'expectExceptionMessageMatches')) {
parent::expectExceptionMessageMatches($regularExpression);
} else {
parent::expectExceptionMessageRegExp($regularExpression);
}
}
}

/** @Annotation */
Expand Down

0 comments on commit 41bce60

Please sign in to comment.