Catch TypeError during annotation instantiation and throw a more meaningful AnnotationException

[7ochem]

I ran into this error:

In OneToMany.php line 45:

  [TypeError]
  Argument 3 passed to Doctrine\ORM\Mapping\OneToMany::__construct() must be of the type array or null,
  string given, called in vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php on line 971

Exception trace:
  at vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/OneToMany.php:45
 Doctrine\ORM\Mapping\OneToMany->__construct() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:971
 Doctrine\Common\Annotations\DocParser->Annotation() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:719
 Doctrine\Common\Annotations\DocParser->Annotations() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:376
 Doctrine\Common\Annotations\DocParser->parse() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php:178
 Doctrine\Common\Annotations\AnnotationReader->getPropertyAnnotations() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/PsrCachedReader.php:155

I traced calls and variables back to a @OneToMany annotation on a property of an entity containing a misconfiguration and fixed it:

     /**
      * @var Collection<int, Page>|null
-     * @ORM\OneToMany(targetEntity="App\Entity\Page", mappedBy="user", cascade="persist")
+     * @ORM\OneToMany(targetEntity="App\Entity\Page", mappedBy="user", cascade={"persist"})
      */
    private ?Collection $pages;

It's not clear from the error neither the trace (coming from a query being executed and objects being hydrated) on which entity and which property this occurred.

I looked into \Doctrine\Common\Annotations\DocParser and there are many checks being done and many places where there might be a \Doctrine\Common\Annotations\AnnotationException being thrown with clear messages.

Should we catch TypeErrors upon the annotation instantiation here and here and throw an AnnotationException with a proper message containing the annotation and property on which it occurred?

[malarzm]

If that's the matter of a simple try/catch/throw-decorated then I'd say got for it 👍

[7ochem]

If that's the matter of a simple try/catch/throw-decorated then I'd say got for it 👍

That's exactly what it is 😄 I think it's the two lines I mentioned, plus another two a bit lower in the code

[malarzm]

Plus tests 👀

[7ochem]

I've created a draft ➡️ #438

It feels a bit tricky and bloated to catch exceptions and preg_match exception messages to pick the values needed. Also there are PHP 7 and 8 differences.

I could also collect the constructor argument types with the already collected data of the constructor arguments here:

annotations/lib/Doctrine/Common/Annotations/DocParser.php

Lines 614 to 619 in d13947c

	foreach ($constructor->getParameters() as $parameter) {
	$metadata['constructor_args'][$parameter->getName()] = [
	'position' => $parameter->getPosition(),
	'default' => $parameter->isOptional() ? $parameter->getDefaultValue() : null,
	];
	}

However comparing types which are more complex is not something I want to do (union types, intersection types, interfaces, etc.).

---

Update:

@stof gave some input on the draft and I think not preg_match'ing the messages but throw a general AnnotationException containing the context and attaching the original exception with it is the best solution.

[malarzm]

@stof gave some input on the draft and I think not preg_match'ing the messages but throw a general AnnotationException containing the context and attaching the original exception with it is the best solution.

That'd be way simpler and effective, was going to propose same approach seeing all those regexps 👍

[7ochem]

I pushed my new code in #438 . On the reported code, I now get this error:

In AnnotationException.php line 53:

  [Doctrine\Common\Annotations\AnnotationException]
  [Creation Error] An error occurred while instantiating the annotation @ORM\ManyToMany declared on
  property App\Entity\User::$pages: "Argument 4 passed to Doctrine\ORM\Mapping\ManyToMany::__construct()
  must be of the type array or null, string given, called in
  vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php on line 1474".                                                                                                                                                                                                        

Exception trace:
  at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationException.php:53
 Doctrine\Common\Annotations\AnnotationException::creationError() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:1477
 Doctrine\Common\Annotations\DocParser->instantiateAnnotiation() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:972
 Doctrine\Common\Annotations\DocParser->Annotation() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:720
 Doctrine\Common\Annotations\DocParser->Annotations() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/DocParser.php:377
 Doctrine\Common\Annotations\DocParser->parse() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php:178
 Doctrine\Common\Annotations\AnnotationReader->getPropertyAnnotations() at vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/PsrCachedReader.php:155

It now clearly states that this happened on the @ORM\ManyToMany annotation of the App\Entity\User::$pages property, so I know where to start looking 😄