[RFC] PHP Attributes

> > > On Mon, Apr 25, 2016 at 3:42 AM, Dmitry Stogov <dmitry@zend.com> wrote: > >> >> >> On 04/22/2016 06:39 PM, guilhermeblanco@gmail.com wrote: >> >> >> On Fri, Apr 22, 2016 at 3:07 AM, Dmitry Stogov <dmitry@zend.com> wrote: >> >>> >>> >>> On 04/22/2016 04:05 AM, <guilhermeblanco@gmail.com> >>> guilhermeblanco@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 < <dmitry@zend.com> >>> dmitry@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" <dmitry@zend.com <mailto: >>>>> dmitry@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 >

> > > On Mon, Apr 25, 2016 at 3:42 AM, Dmitry Stogov <dmitry@zend.com > <mailto:dmitry@zend.com>> wrote: > > > > On 04/22/2016 06:39 PM, guilhermeblanco@gmail.com > <mailto:guilhermeblanco@gmail.com> wrote: >> >> On Fri, Apr 22, 2016 at 3:07 AM, Dmitry Stogov <dmitry@zend.com >> <mailto:dmitry@zend.com>> wrote: >> >> >> >> On 04/22/2016 04:05 AM, guilhermeblanco@gmail.com >> <mailto:guilhermeblanco@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.

> I am not arguing against the RFC nor the feature itself, on the > contrary, I like it. I just do not like certain aspects and design > decisions of it; that is all. > > Configuration and AOP are the best usecases for annotations and those > should be stressed in the RFC. They are not mentioned at all! > > Another usecase that I am missing completely is the usage of it for > documentation and static code analysis. I already mentioned the /throws/ > annotation, this could help e.g. IDEs to warn you better about uncatched > exceptions form methods you call. > > DbC is a possible usecase but better implemented at language level. The > RFC could mention the possibility of it. However, right now it is the > sole usecase beside the not very PHP applicable `<<inline>>` and > `<<jit>>` examples. > > You see, this is more a problem of the RFC text and not of the feature. ;) > > Another think I complained about is the proposed syntax because it makes > annotations look like function calls, which they simply are not and will > not be. The syntax is misleading and a possible built-in functionality > of reactive annotations (not saying we need them) at the language level > for userland is blocked. I know I just repeated myself. > > The extension you mentioned works just fine without the brackets. > > @invariant CONDITION; > class A { > > @ensure CONDITION; > @require CONDITION; > function f(){} > > } >

> You may try to replace attribute syntax with @attr(...) (without > semicolon) into our PHP parser. > Note that we have LALR grammar + restrictions caused by semantic actions. > If you are able to do this, I'll add it into the RFC as an option.