Hi!

That you for bringing real usage examples. This is an interesting point
to consider.

Reading through it, however, I can't help but wonder why these things
are not just data? I mean, why they are kept in the comments? If there
are reasons like performance, there could be solutions developed, and
"keeping things together" doesn't exactly work when the things are
2-screen-long data array and I presume equally complex class - they are
two separate and independent entities anyway, and their synchronization
has to be explicitly managed anyway, nobody can ensure they are in sync
just by looking at it - in fact, given its size, nobody can *anything*
just by looking at it. It's not really human-readable anymore than
database dump is human-readable - the data is there, true, but it's not
exactly how you'd prefer to interact with it.

That said, of course I imagine there are reasons why Drupal developers
made these choices, and I don't think it makes much sense to argue about
them now. I just outlined my first impression, but if it sounds wrong
and uninformed, just ignore it :) It does, however, makes sense arguing
about whether it is something we have to support directly in the
language, and to which lengths we should go to do so.

> defining a given extension.  Saying "well just abandon your approach
> entirely" is not a satisfying answer.

I would not say "abandon your approach" - what works for you works for
you, so trying to convince you otherwise would be wasting your time. I
would say that if somebody is developing a new system and are
considering this approach - I *would* advise checking a different
direction.

That is to say, I would be perfectly happy if PHP had annotations that
do not support such use case, at least not directly. I know that sounds
somewhat insensitive to Drupal developers, and that's not my intent do
dismiss their concerns, but I think we should not take it as a primary
requirement, sine qua non.

My opinion is good design should figure out the best use cases for the
feature and serve them as much as possible, without trying to overload
the system to serve every need and every use case there could be. Doing
80% of cases well is much better than doing 99.999% cases poorly. My
opinion is hundred-element data dumps are well outside of the 80% for
annotations. If whatever design we end up with supports it - great, if
it doesn't - that's fine too.

>  * @Block(
>  *   id = "system_branding_block",
>  *   admin_label = @Translation("Site branding")
>  * )

Why not do just:

@Block([ "id" => "system_branding_block", "admin_label" =>
Translation("Site branding")])

Or, in current RFC syntax:

<<Block([ "id" => "system_branding_block", "admin_label" =>
Translation("Site branding")])>>

Now, we get to a touchy issue of how arrays can be consumed by
annotations clients and what "Translation" is. Here we have two news, as
it should be, good and bad:

- Good news is that AST makes kind of easy to have Translation have any
semantic the client wants
- Bad news is that currently RFC provides no tools for actually
operating on deeper data structures, AFAIK. If such tools are provided,
then I don't see why this won't work.

Also, of course, namespacing is a must, but that was already discussed
and I think being taken care of.

> Yoinks.  Now let's try and turn that into attributes.  Here's my first

Here it's more complicated, because some data items clearly should be
different annotations, like "handlers" - these are probably something like:
<<StorageHandler(Drupal\node\NodeStorage)>>
<<FormHandler(Drupal\node\NodeForm)>>

etc. and some others may be data. Hard to know without understanding
true semantics of the thing.

> OK, let's take that to its logical conclusion and directly map in the
> full annotation:
> <<ContentEntity([

That could work too, but see above about deep data, and also it looks
bad semantically - it's like having a class with one method five miles
long which accepts 50 parameters depending on what needs to be done. Not
enough structure. Again, this is saying without knowing semantics, so
maybe it's all wrong.

In any case, I think if we have: a) namespacing and b) tools to convert
AST to some useable data at least supporting arrays and maybe some kind
of namespace resolution - then I think it still can be supported, with
some effort of course but I would expect moving data of this complexity
to a different system to take some effort in any case.

> Additionally, even then it's still "just an array", by which I mean an
> anonymous struct, by which I mean "you get no help at all on typos." 
> You may have noticed that I typoed "route_provder" instead of
> "route_provider" above.  An an array key, that is a silent runtime error
> that's crazy hard to debug.  Forcing any meaningful data structure into
> anonymous structs is doing everyone a disservice.  (As a Drupal
> developer, I can attest to that fact with personal scars.)

True. If we had named parameters, we could probably avoid that, but we
don't have them. You could make each parameter separate annotation, but
that might look insane. Or maybe not, maybe just unusual.

You could also have a tool that just reads annotation and validates
them. Just like static analysis tools for PHP.

Without really knowing your semantics I don't see how else the system
would know route_provder is not a correct parameter. The only thing that
really can validate such things right now in PHP is interface, and if we
really stretch it, maybe a class - but I'm not sure how that would
really fit. And if we go that route, then we probably couldn't have the
freedom AST allows us.

> 1) ::class constants should resolve at compile time, relative to use
> statements, anywhere in the annotation.

That makes sense, but then it's unclear why should that be unique for
::class? Then all constants should work this way I think.

> 2) Namespacing for attribute names.  This is necessary for both
> collision-avoidance and to minimize typing of obscenely long attribute
> names.  The easiest way to do that would be:

That makes sense too. I think Dmitry said he plans to add that.

> 3) Some way to provide a data definition for the annotation that can be
> checked at compile time.  This could be classes, a la Doctrine. It could
> be a new Annotation type as others have suggested.  It could be

I am somewhat reluctant to define a new conceptual thing just for
annotations. Class seems natural at the first glance but implications of
it are unclear - what does it mean to instantiate such class? How
annotation data is now related to the object of this class?

Another option could be to (ab)use interfaces in a following fashion: if
we define

<<__Attribute>>
interface BlockPlugin() {
        function id();
        function admin_label();
}

Then when we say:

<<BlockPlugin(["id" => "system_branding_block", "admin_label" =>
Translation("Site branding") ])>>

we get an object that implements this interface and id() and
admin_label() returns something relevant. There are problems with this
approach:

1. Using array as parameter is ugly. Named arguments is what we need but
d'oh. Also enshrining array-as-parameters in syntax which is worse.

2. It is not clear what exactly admin_label() should return and how
Translation() should work - is it AST? Is it some kind of object? Should
we just give up on Translation and do it in some other way like this?

<<__Attribute>>
interface BlockPlugin() {
        function id();
        <<Translatable>>
        function admin_label();
}

Note that this is a way to enable nesting of attributes without getting
the data part too complex. But of course then all admin_labels must be
translatable or not-translatable, not half-way.

So the idea is half-baked but maybe it can be used, think about it.
-- 
Stas Malyshev
smalys...@gmail.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to