Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92612
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain garfieldtech.com from 66.111.4.26 cause and error)
To: internals@lists.php.net
References: <BY2PR0201MB178479B3AF8BD49583233034BF6E0@BY2PR0201MB1784.namprd02.prod.outlook.com>
Message-ID: <57194B85.60507@garfieldtech.com>
Date: Thu, 21 Apr 2016 16:52:05 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0)
 Gecko/20100101 Thunderbird/38.0.1
MIME-Version: 1.0
In-Reply-To: <BY2PR0201MB178479B3AF8BD49583233034BF6E0@BY2PR0201MB1784.namprd02.prod.outlook.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] PHP Attributes
From: larry@garfieldtech.com (Larry Garfield)

On 4/21/16 4:13 PM, Dmitry Stogov 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.

Thanks, Dmitry!  In concept I am in favor of syntax-native annotations, 
although I have some concerns with the specifics of the proposal.  
Thoughts in no particular order:

First, for the getAttributes() reflection method, please oh please don't 
return array-or-false.  That's horrible.  Just return an empty array if 
there aren't any, as that makes getAttributes() entirely type safe and 
saves all callers from a mandatory if-check.  (See 
http://www.garfieldtech.com/blog/empty-return-values for more information.)

The reflection section further indicates that the type of the result is 
variable, which means I cannot know in advance if I'm going to get back 
a scalar or an array.  If we go with this free-form approach, I'd 
honestly prefer to always get back an array, even for single value, so 
that I can always know the type I'm dealing with. (Since I cannot 
enforce a given attribute to be single-value.)

For the expression example:

<<test($a  +  $b   >  0)>>
function  foo($a,  $b)  {
}


It is not at all clear to me what scope the annotation's $a and $b exist 
in.  Are the they same $a and $b as in the function signature? If so, 
what happens if I reflect the function before ever calling it?  How can 
I evaluate test?  Or are they inherited from the global scope at the 
time of declaration?  (That scares me a great deal.)  I don't know what 
to make of that at all.

In the "Attribute syntax" section, the text says the tokens are the left 
and right double-angle character, as used for quotations in some 
European languages.  The rest of the text says it's two left/right 
carrot characters, as seen above the comma and period on US keyboards.  
I'm assuming the former is just a typo/auto-correct bug.

If I read correctly, the following two would be semantically identical:

<<One, Two>>
function foo() {}

<<One>>
<<Two>>
function foo() {}

Is there a reason you chose the name "attribute" rather than 
"annotations", which seems at least in PHP to be the more common term 
for this type of declaration?

It appears that the annotations themselves are entirely free-form. At 
the risk of expanding the typing debate, this concerns me as then all 
we're adding is a new way to parse undocumented, undefined anonymous 
structs.  How can I say what annotations mean what for my ORM, or 
routing system, or whatever?  We're back to, essentially, out-of-band 
documentation of big anonymous structs (aka associative arrays).

A more robust alternative would be something along the same lines that 
Doctrine uses: Make annotations actual classes.  To wit:


<<AThing>>
<<AnotherThing('stuff')>>
<<MoreThing(1, 2, 3)>>
function foo($a, $b) { }

Where AThing, AnotherThing, and MoreThing are defined classes, and 
subject to namespaces and use statements.  Then what gets returned from 
getAttributes() is an array consisting of an instance of AThing, an 
instance of AnotherThing, and an instance of MoreThing. In this example 
we'd just call their constructors with the listed values and let them do 
as they will.  Doctrine uses named properties in the annotation that 
maps to properties on the object, which is even more flexible and 
self-documenting although I don't know how feasible that is without 
opening up the named properties can of worms globally.

Either way, the advantage then is that I know what annotations are 
available, and the class itself serves as documentation for what it is, 
what it does, and what its options are.  It also helps address 
collisions if two different libraries both want to use the same keyword; 
we already have a class name resolution mechanism that works and 
everyone is familiar with.

One concern is that not all classes necessarily make sense as an 
annotation; perhaps only classes with a certain interface can be used.  
Actually (thinking aloud here), that would be a possible solution to the 
named property issue.  To wit:

<<AThing(a => 'a', b => 'b')>>
foo() {}

class AThing implements Attribute {
   public static function attributeCreate(array $params) {
     return new static($param['a'], $param['b']);
   }
}

$r  = new ReflectionFunction('foo');
$a = $r->getAttributes();

$a is now an array of one element, an instance of AThing, created with 
'a' and 'b'.  The specifics here are probably terrible, but the general 
idea of using classes to define annotations is, I think, a big step 
forward for documentation and avoiding multi-library collisions.

While I know some of the things Drupal 8 is using annotations for are 
arguably excessive (and I would agree with that argument in some cases), 
as is I fear the proposed system is too free-form and rudimentary for 
Drupal to switch to them.

-- 
--Larry Garfield