On 7/1/11 8:06 AM, Johannes Schmitt wrote:
Why should we treat annotation classes any different than any ordinary PHP class?
The point is not to treat annotation classes differently. The problem is that not all annotations are actually classes.
Fabien
Johannes On Fri, Jul 1, 2011 at 7:09 AM, Kris Wallsmith <[email protected] <mailto:[email protected]>> wrote: Does the class_exists('asdf', false) strategy work well when library classes are compiled into one file? I seem to remember this issue coming up in the past, I just can't recall the outcome. Other than that glimmer of a memory I agree with your thinking. A class that consumes annotations should be able to load them all (or provide a mechanism to) ahead of time. Kris On Thu, Jun 30, 2011 at 4:42 PM, Benjamin Eberlei <[email protected] <mailto:[email protected]>> wrote: > I am sorry it is very late to bring this up again, but given the nasty problems I faced today with the way annotationreader works with autoload = true I have to share them (and blow of some steam). While i guess its to late to change this again before 2.0, we might still have to discuss to go away from autoload = true for 2.1. Now for the reasons: > > AnnotationReader with autoload = true (which Symfony2 uses) will not only require a silent autoloader, it requires ALL autoloaders used to be silent. While this is the case for the Symfony Universal loader its not the case for the Doctrine one, and not for many others - and its not even a PSR-0 requirement. For a simple reason: Supporting silent failure means using file_exists, means a file stat, which means apc.stat = 0 is useless. While I don't care so much about it in Doctrine context, because the AnnotationReader default is NOT to autoload annotations this will cause problems for Symfony: Not every library in any Symfony2 app will be included through a silent autoloader. That means given the context users might run into problems using annotations that they have absolutely no way of fixing. And since the AnnotationReader does not know this upfront, potential failures become very real issue. > > Example: I use SecurityExtraBundle and happen to have my SuperDuperLibrary XYZ. That library was developed by my company and contains tons of important business logic but unfortunately uses a non-silent autoloader (for whatever reasons). However i use Symfony to secure access to it by generating secure proxies. Now what happens if an annotation reader runs over that library? Because the current namespace is always considered, for every @var __NAMESPACE_."\\var" is class_exists'ed and then your code fatal errors. I didn't see this problem before, because i don't use annotations myself so much and always in the BC Doctrine 2.0.x way and that slipped my mind. Same goes for Validation with Annotations on external code, or maybe in the future services definitions through annotations. > > All this trouble we are getting into with autoloading annotations is just because we wanted to validate that the annotations used actually exist. But since we changed to a use/import approach rather then re-using namespaces we don't even have the clashing problem anymore that got us into the trouble with class_exists before. The autoloading however also got us and users into other problems: > > 1. We have to maintain a rather large map of "blacklisted" annotations that never throw failure exceptions because they are actually used as doc documentations. As with blacklists this is never a complete list. > 2. Users with intensive doc-block usage are punished by Symfony having to maintain their own blacklist of annotations that never throw exceptions so that their code actually works. > 3. Libraries have to handle the case that a reader is either using autoloading or not through special code. > > While I do think it would have been nice to offer validation for annotation this is a task that should be put on IDEs and developers, since it turns out that its not possible flawlessly within the library itself. The AnnotationReader should always use class_exists($name, false). Docblocks are still comments and the code shouldn't fail because classes are not available that are not even relevant in the context of the current AnnotationReader. Each part of the code that uses an AnnotationReader should require_once the annotations that it can potentially need upfront. That even works for Validation as you can grab the tags from the DIC. That way we have a single mode of operation for the AnnotationReader and not two different ones that are 180 degrees different. OF course one could argue ALWAYS to use class_exists($name, true) instead, but i hope my mail showed why this is not a good idea. > > If for some reason we do want autoloading for annotations then it should be a mechanism different from the PHP one, because they are both not compatible. The reader could have hooks for autoloading and validation mechanisms. Nothing we want to add for Doctrine Common 2.1, but something we could easily play around with for Common 3. > > greetings, > Benjamin > > > > On Thu, 30 Jun 2011 10:48:09 +0200 > Jordi Boggiano <[email protected] <mailto:[email protected]>> wrote: > >> On 30.06.2011 08:02, Johannes Schmitt wrote: >> > Extending a class will introduce a coupling (for example on the >> > validator component). Another, less intruding approach would be to >> > require an annotation on annotations itself (e.g. @Annotation). This >> > seems like the most future-proof approach to me since it allows us to >> > add more annotations like @Target for example. >> >> Not sure why everyone seems to have ignored this, but it sounds to me >> like a better idea than the other one. >> >> Cheers >> >> -- >> Jordi Boggiano >> @seldaek - http://nelm.io/jordi >> > > -- > If you want to report a vulnerability issue on symfony, please send it to security at symfony-project.com <http://symfony-project.com> > > You received this message because you are subscribed to the Google > Groups "symfony developers" group. > To post to this group, send email to [email protected] <mailto:[email protected]> > To unsubscribe from this group, send email to > [email protected] <mailto:symfony-devs%[email protected]> > For more options, visit this group at > http://groups.google.com/group/symfony-devs?hl=en > -- If you want to report a vulnerability issue on symfony, please send it to security at symfony-project.com <http://symfony-project.com> You received this message because you are subscribed to the Google Groups "symfony developers" group. To post to this group, send email to [email protected] <mailto:[email protected]> To unsubscribe from this group, send email to [email protected] <mailto:symfony-devs%[email protected]> For more options, visit this group at http://groups.google.com/group/symfony-devs?hl=en -- If you want to report a vulnerability issue on symfony, please send it to security at symfony-project.com You received this message because you are subscribed to the Google Groups "symfony developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/symfony-devs?hl=en
-- If you want to report a vulnerability issue on symfony, please send it to security at symfony-project.com You received this message because you are subscribed to the Google Groups "symfony developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/symfony-devs?hl=en
