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.

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

Catch and process errors that occure during annotation instantiation #438

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions lib/Doctrine/Common/Annotations/AnnotationException.php
Original file line number Diff line number Diff line change
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
36 changes: 32 additions & 4 deletions lib/Doctrine/Common/Annotations/DocParser.php
Original file line number Diff line number Diff line change
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,31 @@ 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(
7ochem marked this conversation as resolved.
Show resolved Hide resolved
sprintf(
'An error occurred while instantiating the annotation @%s declared on %s: "%s".',
$originalName,
$context,
$exception->getMessage()
),
$exception
);
}
}
}
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ parameters:
ignoreErrors:
- '#Instantiated class Doctrine_Tests_Common_Annotations_Fixtures_ClassNoNamespaceNoComment not found#'
- '#Property Doctrine\\Tests\\Common\\Annotations\\DummyClassNonAnnotationProblem::\$foo has unknown class#'
- '#Call to an undefined static method PHPUnit\\Framework\\TestCase::expectExceptionMessageRegExp\(\)#'
Copy link
Contributor Author

@7ochem 7ochem May 17, 2022

Choose a reason for hiding this comment

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

Here

if (method_exists(get_parent_class($this), 'expectExceptionMessageMatches')) {
parent::expectExceptionMessageMatches($regularExpression);
} else {
parent::expectExceptionMessageRegExp($regularExpression);
}

Either one of those exists: expectExceptionMessageRegExp exists in PHPUnit 7, deprecated in 8 and removed in 9 (and thus PHPStan gives an error). expectExceptionMessageMatches exists in PHPUnit 8 and up


# That tag is empty on purpose
- '#PHPDoc tag @var has invalid value \(\)\: Unexpected token "\*/", expected type at offset 9#'
100 changes: 86 additions & 14 deletions tests/Doctrine/Tests/Common/Annotations/DocParserTest.php
Original file line number Diff line number Diff line change
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,16 @@ 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->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 +908,19 @@ 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->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 +1121,21 @@ 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->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 +1149,19 @@ 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->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 +1724,37 @@ 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) {
$this->assertInstanceOf(TypeError::class, $exc->getPrevious());

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