DCOM-11: Impossible to skip annotations #407

Closed
doctrinebot opened this Issue Jul 8, 2010 · 9 comments

2 participants

@doctrinebot

Jira issue originally created by user blt04:

It is currently impossible to skip annotations of the form:
@annotation(

When a doc block comment begins with an "@" and has an open parenthesis it is assumed to be an exist annotation even if the corresponding annotation class doesn't exist or can't be autoloaded. If you are trying to use the AnnotationReader on a file, you have to make sure all annotations are loaded or can be autoloaded - even the ones you don't care about

In a comment to DCOM-2, Roman stated that his recommended way to load annotations is to load them manually via a require call. This is done in Doctrine ORM and is absolutely necessary because ORM annotations are stored in an autoloader unfriendly way (multiple classes per file and namespace path different from filesystem path).

So, if I want to add my own annotations and store them in a non-autoloader friendly way as Doctrine ORM does, I need to ensure that every AnnotationReader acting on that file knows about my annotations. This is not always possible or desirable.

Removing the parenthesis check and relying solely on class_exists fixes this problem.

@doctrinebot

Comment created by merk:

I have come across another case where this bug causes a fatal PHP Error when you nest a non existant annotation:

/****

PHP Fatal error: Doctrine\Common\ClassLoader::loadClass(): Failed opening required 'Doctrine/Tests/Common/Annotations/InnerNonexistant.php'

Test case and fix provided in github: http://github.com/merk/common/commit/95388a5febee95dc0483cf35d991b2b227e89069

@doctrinebot

Comment created by @jwage:

Roman, what are your thoughts on this issue? It has been a problem for us with the Symfony integration.

@doctrinebot

Comment created by @jwage:

Brandon, can you help me understand better? In your case why is the class not present and why can it not be autoloaded?

@doctrinebot

Comment created by romanb:

@"Removing the parenthesis check and relying solely on class_exists fixes this problem."

The main problem with such an approach is that an AnnotationReader works (and caches) under the assumption that all annotations of a doc-block are processed at once. That means the "undefined" annotations would never be visible and not accessible if a cache is used unless the cache is cleared and the annotations requested again, this time with all annotations defined.

@doctrinebot

Comment created by blt04:

Let's say I'm writing an extension that supports annotations. I want my extension to work on the same entity classes that Doctrine ORM uses. It is not possible to hook in to the ORM to read my annotations (http://groups.google.com/group/doctrine-dev/browse_thread/thread/4d478a32f8a12a57/c3fce8707becce5c?lnk=gst&q=brandon#c3fce8707becce5c) so I will need to do it with my own code. That is fine, but I've got three problems:

1) When I process my annotations, I have to make sure I load all the annotation classes needed by Doctrine ORM. This is simple and easy.

2) When Doctrine ORM trys to process entities with my custom annotations, it needs to be able to load my extension's annotation classes. How does it know about them (if not autoloading)

3) What happens if someone else writes an extension that uses custom annotations. And say a user wants a single entity to implement my extension, another extension, and of course Doctrine ORM. Now all three need to know about each other.

Now we could make all classes able to be autoloaded and that may fix the problem, but Roman recommends against that here: http://www.doctrine-project.org/jira/browse/[DCOM-2](http://www.doctrine-project.org/jira/browse/DCOM-2)?focusedCommentId=13070&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_13070

So in summary, the annotation class would be present when I process the entity, but not when Doctrine ORM does.

@doctrinebot

Comment created by romanb:

After some discussion, the current consensus is that we will apply the suggested change and clearly document how caching works with an AnnotationReader.

@doctrinebot

Comment created by @jwage:

Fixed in http://github.com/doctrine/common/commit/5c90f7b513579bf14603621564db6b4da3fd5665

Commit updating the documentation is coming today.

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot doctrinebot added the Bug label Dec 6, 2015
@jwage jwage was assigned by doctrinebot Dec 6, 2015
@doctrinebot doctrinebot added this to the 2.0.0-BETA4 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment