On 6/28/2016 6:31 PM, Rasmus Schultz wrote: > Since the discussion isn't completely dead yet (thank you Pedro and > Marco), I will just briefly explain what I had in mind for another > RFC. >
I want to see this feature happen too but the right way, so let us just spend some more time wrapping our heads around it. We could also discuss and work on the RFC at a different place (GitHub with issues?), since many people are always complaining about too many emails that are sent to this ML. On 6/28/2016 6:31 PM, Rasmus Schultz wrote: > The syntax would no longer allow arbitrary expressions - instead, it > would allow object annotations only, declared in one of three ways: > > << RequiresLogin >> // equivalent to new RequiresLogin() > > << Length(20) >> // equivalent to new Length(20) > > << HttpMethod::post() >> // equivalent to static method-call > HttpMethod::post() > > The latter permits you to declare and call static factory methods, > e.g. "named constructors" for some types of annotations. > > Annotations are now consistently objects, which simplifies filtering > annotations, etc. > > This declaration syntax enables better in-language analysis - because > the declarations always include the annotation class-name, this means > it can now support deferred as well as conditional creation, by having > a slightly different reflection API: > > $annotations = $class_reflection->getAnnotations(); > > At this point, we have a list of ReflectionAnnotation objects - the > actual annotation objects have *not* been created at this point, which > would allow for conditional evaluation, such as: > > foreach ($annotations as $annotation) { > echo $annotation->className; // => "Length" etc. > > if ($annotation->classExists()) { > var_dump($annotation->getInstance()); // => Length object, etc. > } else { > var_dump($annotation->getInstance()); // => UnknownAnnotation > object > } > } > > The UnknownAnnotation object has three properties: class-name, > method-name ("__construct" if constructor was used) and the list of > arguments. > > In other words, this would provide support for "optional > dependencies", in much the same way that unserialize() does it - the > getInstance() method will trigger autoloading, and if the class is > unavailable, it takes this "typeless" information and puts it in a > general object, so that the information itself isn't lost. > Awesome! This is exactly the kind of thing that one wants from reflection, information and not exceptions or worse. :) On 6/28/2016 6:31 PM, Rasmus Schultz wrote: > It may *seem* like this solves the optional dependency problem, but > let me be clear, it does *not* fully address that issue, because > something like << Method(Http::GET) >> could still fail, if the Method > class is defined, but the Http class is not. That's a much more > marginal case, but it's not completely unlikely that you would use > constants from one dependency and pass them to annotation classes from > another dependency, e.g. if the HTTP method constants were part of an > HTTP package, and the annotation itself were part of a different > package. Arguably it's much less likely to occur though - if you have > the annotation package installed, most likely you have the dependent > package of that package installed as well. > > So this is much safer, but just to be clear, the only way you can make > it any safer than that, is by completely disallowing anything but > values as arguments to annotation constructors - which, in my opinion, > simply isn't useful. > I would argue that this should fail if the Method class from the example exists and is actually being created. However, if the Method class does not exist (and we construct an UnknownAnnotation stub for it) nothing should go wrong. I mean, the assumption that the dependency of a missing dependency is/might be missing sounds logical too me. Of course we would need to have something like an UnknownAnnotationArgument stub too. Note that this allows you in consequence to create other kinds of objects within annotations: << Annotation(new Argument) >> final class {} This would allow nesting as some DocBlock libraries allow it currently. On 6/28/2016 6:31 PM, Rasmus Schultz wrote: > Alternatively to the above, you could imagine a simpler reflection > facility that does eagerly construct annotations, but constructs > UnknownAnnotation instances for missing classes - this would perhaps > balance more towards immediate usefulness and less towards performance > perfectionism, as it increases the chance of loading an unused > annotation. > > This would be my preference though - as previously argued, the odds of > having a large number of unrelated annotations on the same member are > extremely low in practice; an annotated entity tends to be a persisted > property, a form input element, or an action method, etc... rarely do > you mix annotations from different domains onto the same member; > annotations that are applicable to a given domain are usually relevant > to a consumer belonging to the same (or a closely related) domain, so > this performance concern is mostly a theoretical micro optimization. > > For another, eager construction means we can filter annotations with > instanceof rather than by class-name, which is much powerful, enabling > you to leverage inheritance - for example, you'd be able to ask > directly for all instances of ValidationAnnotation and get those with > a single call, which is incredibly useful. > Isn't it possible to go for a generator (yield) like approach and combine the best of both worlds? That being said, I am in favor of this approach as well, because, as you said, instanceof is more powerful and one expects all annotations to be present, the UnknownAnnotation would be a special case. On 6/28/2016 6:31 PM, Rasmus Schultz wrote: > Anyways, let me know your feelings about that idea? If there's any > interest, I'd be happy to write yet another RFC based on that. > +1 with one thing, go for the @ and not <<>>. It is possible with the parser and so far most people stated that they prefer it, or put it to vote with a separate poll. @Route('/blog/{id}', new Method(Http::GET)) @ParamConverter('post', Post::class) public function indexAction(Post $post) {} -- Richard "Fleshgrinder" Fussenegger
signature.asc
Description: OpenPGP digital signature