Doctrine Common

Impossible to skip annotations

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.0.0-BETA4
  • Fix Version/s: 2.0.0-BETA4
  • Component/s: Annotations
  • Labels:
    None

Description

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.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Tim Nagel added a comment -

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

/**

  • @Name
  • @NonExistant(
  • @InnerNonexistant(32)
  • )
    */

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

Show
Tim Nagel added a comment - I have come across another case where this bug causes a fatal PHP Error when you nest a non existant annotation: /** @Name @NonExistant( @InnerNonexistant(32) ) */ 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
Hide
Permalink
Jonathan H. Wage added a comment -

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

Show
Jonathan H. Wage added a comment - Roman, what are your thoughts on this issue? It has been a problem for us with the Symfony integration.
Hide
Permalink
Jonathan H. Wage added a comment -

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

Show
Jonathan H. Wage added a comment - Brandon, can you help me understand better? In your case why is the class not present and why can it not be autoloaded?
Hide
Permalink
Roman S. Borschel added a comment -

@"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.

Show
Roman S. Borschel added a comment - @"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.
Hide
Permalink
Brandon Turner added a comment -

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?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.

Show
Brandon Turner added a comment - 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?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.
Hide
Permalink
Roman S. Borschel added a comment -

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

Show
Roman S. Borschel added a comment - After some discussion, the current consensus is that we will apply the suggested change and clearly document how caching works with an AnnotationReader.
Hide
Permalink
Jonathan H. Wage added a comment -

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

Commit updating the documentation is coming today.

Show
Jonathan H. Wage added a comment - Fixed in http://github.com/doctrine/common/commit/5c90f7b513579bf14603621564db6b4da3fd5665 Commit updating the documentation is coming today.
Hide
Permalink
Jonathan H. Wage added a comment -

Added docs for annotations here: http://github.com/doctrine/common-documentation/commit/896cc3bace84b6dc891a3d050a425fc99249359e

Show
Jonathan H. Wage added a comment - Added docs for annotations here: http://github.com/doctrine/common-documentation/commit/896cc3bace84b6dc891a3d050a425fc99249359e

People

  • Assignee:
    Jonathan H. Wage
    Reporter:
    Brandon Turner
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: