On Mon, Apr 25, 2016 at 3:42 AM, Dmitry Stogov <dmi...@zend.com> wrote:
> > > On 04/22/2016 06:39 PM, guilhermebla...@gmail.com wrote: > > > On Fri, Apr 22, 2016 at 3:07 AM, Dmitry Stogov <dmi...@zend.com> wrote: > >> >> >> On 04/22/2016 04:05 AM, <guilhermebla...@gmail.com> >> guilhermebla...@gmail.com wrote: >> >> Hi Dmitry, >> >> As a previous suggester of metadata information built-in into PHP, and >> also one of developers of the most used metadata library written in PHP, I >> understand this feature implementation requires several design decisions >> and also a good understanding of specific situations users may require. >> >> While I am a strong supporter of a more robust solution, this is already >> a good start. >> A few things I'd like to ask for my own understanding and also >> suggestions too: >> >> 1- I understand you took a minimalistic approach towards a "dumb" >> implementation for attributes (when I mean "dumb", the idea here is towards >> a non-OO approach). Can you explain your motivations towards this approach? >> >> I see two distinct approaches of implementation for this feature. Both of >> them have some common demands, like lazy initialization of metadata. Here >> they are: >> >> - Simplistic approach, which lets consumers of the feature do all the >> work related to validation, assertion of valid keys, values, etc >> This does not invalidate the ability to leverage of some features that a >> more robust implementation demands. >> >> - Robust approach: language takes the burden of instantiating complex >> structures, validating, assertion of valid keys, values, if this complex >> structure is allowed to be instantiated in that given class, method, etc. >> >> >> I didn't exactly understand what do you suggest. >> If you are talking about Attribute objects initialization during >> compilation - this is just not possible from implementation point of view. >> Now attributes may be stored in opcache SHM and relive request boundary. >> Objects can't relive requests. >> > > > I know that object instances are not cross-requests. Explicitly, I > mentioned that both approaches require lazy-initialization (which means, > whenever you call getAttributes() or getAttribute()). > > What I mentioning is that your approach is basically a new key/value > syntax that are used specifically for Attributes. We could easily turn this > into a more robust approach if instead of defining key/value pairs, we > instantiate objects or call functions. You already demonstrated interest to > support <<ORM\Entity>> reusing the imports (which is our biggest headache > in Doctrine Annotations), so why not issue constructor or function calls > there? That would simplify the work needed for consumers and also add room > for later improvements. > > So basically in this example: > > use Doctrine\ORM; > > <<ORM\Entity("user")>> > class User {} > > $reflClass = new \ReflectionClass("User"); > var_dump($reflClass->getAttributes()); > > We'd be changing from this: > > array(1) { > ["Doctrine\ORM\Entity"]=> > array(1) { > [0]=> > string(4) "user" > } > } > > Into this: > > array(1) { > ["Doctrine\ORM\Entity"]=> > object(Doctrine\ORM\Entity)#1 (1) { > ["tableName"]=> > string(4) "user" > } > } > > > As I showed already, it's very easy to do this transformation at higher > layer. > > $reflClass = new \ReflectionClass("User"); > $attributes = $reflClass->getAttributes() > foreach ($attributes as $key => &$val) { > $val = new $key(...$val); > } > var_dump($attributes); > > Construction objects directly in Reflection*::getAttributes() method, > doesn't make significant benefits and even makes limitation. > Sorry, but I don't see how limitations are added. If you call a function, static method or constructor, you actually add whole new level of possibilities, and I fail to see which limitations are added. Could you provide me one? Calling the function/constructor/static method, not only helps to better segregate userland code, but it also adds subsequents extensibility. I can highlight examples: - Support for Inheritance and overrides, through @Inherit, @Override, etc. While you might not see how it could be used now, other developers might be weirdly creative. - Targeting of annotations, such as limiting its scope to be only class, method or property. We use this extensively in Doctrine, where you cannot define Doctrine\ODM\Entity over a property. - Separating what can be considered as an annotation and what cannot. Built-in @Annotation as a marker would differentiate that I can do call Doctrine\ORM\Entity and not Doctrine\ORM\UnitOfWork. - Make it easier to support an AOP extension, where it could detect annotations being used and override DO_FCALL to call before, after or around through the implementation of interfaces. - If we ever decide to support named parameters, taking advantage of that would become natural, like: <<ORM\Entity(tableName => "user")>> > > > > >> >> 1- Your approach is basically defining an array. Could you explain your >> line of thinking on why you didn't consider a syntax like the one below? >> >> <["key" => "value"]> >> class Foo {} >> >> I didn't try to invite new syntax. Just completely took it from HHVM. >> > > My idea was based on your current proposal, which is basically a way to > define key/value pairs. > If you decide to go minimalistic, that is probably my best line of > thinking. > > >> >> >> >> 2- I see that you added support over functions, classes, constants and >> properties. According to the RFC, getAttributes() was added over >> ReflectionFunction. Is there a reason why support was not added to methods >> (ReflectionMethod extends ReflectionFunctionAbstract, which was not >> mentioned on RFC)? Any reason to not support it in function/method >> parameters? >> >> ReflectionMethod is a child of ReflectinFunction, so it's supported. >> > Attributes are allowed for the same entities as doc-comments (they are not >> allowed for parameters) >> > > I was asking if there was a purpose to not support Attributes over > ReflectionParameter. Example: > > class Foo { > public function bar(<<Qux>> Bar $bar) : bool { > // ... > } > } > > $reflClass = new \ReflectionClas("Foo"); > $reflMethod = $reflClass->getMethod("bar"); > $reflParameter = $reflMethod->getParameters()[0]; > > var_dump($reflParameter->getAttributes()); > > > I understood, we may add this ability later. > I'd say we should add this from day one. A quick use case that comes to my mind are parameters conversion that happens in Symfony2 through their "extra" bundle (doc: http://symfony.com/doc/current/bundles/SensioFrameworkExtraBundle/annotations/converters.html ). In a controller action (it's a class method), you have the ability to convert the Request object into something else that makes more sense for you. Example: class UserController extends Controller { public function viewAction(<<UserParameterConverter("userId")>> User $user = null) { if ($user === null) { throw new NotFoundException("User not found"); } return ["me" => $this->getUser(), "user" => $user]; } } > > > >> >> >> >> 3- Did you put any thought on inheritance? What I mentioned in comment #1 >> is even smaller than what you implemented in RFC. >> Assuming you keep the RFC approach, did you consider support overrides, >> inherit, etc? >> >> >> In my opinion, attributes don't have to be inherited. >> If you think differently - please explain your point. >> > > Of source I can. > A simple case would be to increate visibility of the inherited property. > It was declared in a parent class as protected, but now you want public, > and you still want to keep all parent defined Attributes. > > Very questionable. If you redefine property, it shouldn't inherit > attributes. > This leads to some serious copy/paste, highly error prone... =( > > > Another example is like we do in Doctrine. We support a callback system > which we named as lifetime callbacks. Pre-persist is one of them, which is > called every time a given Entity is about to be persisted into DB. When > you're dealing with inheritance, you can potentially override the method > content and you still want to trigger the same operation as if it was > untouched. Example: > > use Doctrine\ORM; > > trait Timestampable { > protected $created; > protected $updated; > > <<ORM\PrePersist>> > public function prePersist() { > $this->created = $this->updated = new \DateTime("now"); > } > > <<ORM\PreUpdate>> > public function preUpdate() { > $this->updated = new \DateTime("now"); > } > } > > <<ORM\Entity>> > class User { > use Timestampable; > > public function prePersist() { > // Add my custom logic > } > } > > The implication is that through a simplistic approach, inheriting (or > overriding) is not clear and I can't figure it out an easy way to achieve > that. > Now if we go towards calling a function or class constructor like I > mentioned before, then we could easily build structures like __Inherit, > __Override, etc. > > It's definitely, not clear when attribute inheritance make sense and when > completely not. For example, if we mark some method to be JIT-ed, it > doesn't mean that we like to JIT methods of all children. So, I prefer not > to do inheritance at all. The higher layers may emulate "inheritance" of > some attributes their selves (like you do this with doc-comments). > As I said earlier, if you do a call based approach, we could create @Inherit or @Override, which would not only make us safe from support, but also gives more power to developers. > > > > >> >> >> 4- I understand that a more robust attribute solution would be required >> to achieve this, but one of the biggest advantages of AOP is the ability to >> perform custom logic before, after or around... However, I don't know if >> any kind of triggers came in your head or are planned as a future RFC. >> Let me highlight one example: Every time a class, property or method is >> called that is annotated as <<deprecated>>, I would like to issue an >> E_USER_DEPRECATED warning. A trigger-like solution would be required. Did >> this concept came to your mind? >> >> This is not a subject of this RFC. >> Attributes provides a storage for metadata, but don't define how to use >> them. >> Especially, for your use-case: >> 1) it's possible to create preprocessor that embeds corresponding >> trigger_error() call >> 2) it's possible to write a PHP extension that plugs-into compiler chain >> and checks <<deprecated>> attribute for each compiles function, then sets >> ZEND_ACC_DEPRECATED flag >> 3) It's also possible to override DO_FCALL opcodes and perform checks >> there (this is inefficient) >> >> > With this simplistic approach, I agree there's 0 value into considering > this. > However, taking a more robust approach would potentially open this > possibility through a simpler extension. > > > You saw, Sara named even this proposed solution a bit over-designed. > it make no sense to implement all functionality at language level. > Actually, keeping simple base interface, opens doors for more use-cases. > > Thanks. Dmitry. > > > > > >> Thanks. Dmitry. >> >> >> >> >> >> Regards, >> >> On Thu, Apr 21, 2016 at 7:44 PM, Dmitry Stogov < <dmi...@zend.com> >> dmi...@zend.com> wrote: >> >>> >>> >>> On 04/22/2016 02:16 AM, Dominic Grostate wrote: >>> >>>> >>>> This is amazing. It would actually allow us to implement our automated >>>> assertions ourselves, as opposed to requiring it within the language. >>>> >>>> this was the idea - to give a good tool instead of implementing every >>> possible use-case in the language. >>> >>> Could it also support references? >>>> >>>> <<sanitize(&$a)>> >>>> function foo($a) { >>>> >>>> } >>>> >>>> yes. "&$a" is a valid PHP expression. >>> >>> If you plan to use this, I would appreciate, if you to build the patched >>> PHP and try it. >>> The early we find problems the better feature we will get at the end. >>> >>> Thanks. Dmitry. >>> >>> >>> On 21 Apr 2016 10:13 p.m., "Dmitry Stogov" <dmi...@zend.com <mailto: >>>> dmi...@zend.com>> wrote: >>>> >>>> Hi, >>>> >>>> >>>> I would like to present an RFC proposing support for native >>>> annotation. >>>> >>>> The naming, syntax and behavior are mostly influenced by HHVM >>>> Hack, but not exactly the same. >>>> >>>> The most interesting difference is an ability to use arbitrary PHP >>>> expressions as attribute values. >>>> >>>> These expressions are not evaluated, but stored as Abstract Syntax >>>> Trees, and later may be accessed (node by node) in PHP extensions, >>>> preprocessors and PHP scripts their selves. I think this ability >>>> may be useful for "Design By Contract", other formal verification >>>> systems, Aspect Oriented Programming, etc >>>> >>>> >>>> https://wiki.php.net/rfc/attributes >>>> >>>> >>>> Note that this approach is going to be native, in contrast to >>>> doc-comment approach that uses not well defined syntax, and even >>>> not parsed by PHP itself. >>>> >>>> >>>> Additional ideas, endorsement and criticism are welcome. >>>> >>>> >>>> Thanks. Dmitry. >>>> >>>> >>> >> >> >> -- >> Guilherme Blanco >> Lead Architect at E-Block >> >> >> > > > -- > Guilherme Blanco > Lead Architect at E-Block > > > -- Guilherme Blanco Lead Architect at E-Block