Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:109708
MIME-Version: 1.0
References: <CAEPJdunSr8PkBtSLO8g_JMLzM4cFVnXsQSEMFmu2RN25XT=grw@mail.gmail.com>
 <CAEPJdumeStFzWRcEtU3NjEgmxZ_08wbfyJezhHOEzZbzdNWC=w@mail.gmail.com> <CANUQDChrRXg1Jk3ig20bBTNKg8QbZZY=JH+9GDY4e_Rtyz4kEA@mail.gmail.com>
In-Reply-To: <CANUQDChrRXg1Jk3ig20bBTNKg8QbZZY=JH+9GDY4e_Rtyz4kEA@mail.gmail.com>
Date: Mon, 20 Apr 2020 00:19:30 +0200
Message-ID: <CAEPJdun+MFYo9WXum02pP0mxTYZ5YaL_4qYbZoTA-18iXTjepQ@mail.gmail.com>
To: Niklas Keller <me@kelunik.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000d7cbf205a3ac2d9c"
Subject: Re: [PHP-DEV] Re: [RFC] Attributes v2
From: kontakt@beberlei.de (Benjamin Eberlei)

--000000000000d7cbf205a3ac2d9c
Content-Type: text/plain; charset="UTF-8"

On Sun, Apr 19, 2020 at 11:30 PM Niklas Keller <me@kelunik.com> wrote:

> Hey Benjamin,
>
> I haven't really followed the discussion, so sorry if anything I'm
> writing has already been discussed.
>
> Attributes / annotations are one of the two things I currently miss
> most in PHP (the other being generics), so thanks for the work on
> that!
>
> There are a few points that seem odd to me, I'll start with the "Php"
> prefix:
>
> ```php
> <?php
>
> namespace My\Attributes;
>
> use PhpAttribute;
>
> <<PhpAttribute>>
> class SingleArgument
> {
>     public $value;
>
>     public function __construct(string $value)
>     {
>         $this->value = $value;
>     }
> }
> ```
>
> I think this should be <<Attribute>> instead, if we go with that
> syntax. However, I'd even propose another syntax, as attributes aren't
> ordinary classes, I'm not sure whether they should be instantiatable
> from userland and / or be able to use inheritance, especially as
> constructors in PHP aren't subject to variance rules. I guess the
> INSTANCE_OF filter also changes whether the attributes are autoloaded
> or not?
>

<<Attribute>> would best require a namespace (PHP\Attributes) as I feel
claminig "Attribute" class in the main namespace might cause problems for
users. But there is no PHP namespace yet and proposals about this have just
gotten to the list. I have therefore looked to PhpToken from nikitas recent
RFC as an example for naming, because several contributors mentioned that
the PHP\Attributes namespace I suggested in an earlier version of the RFC
would be an instant "no" vote, because of the lack of previous decision on
this topic.

The reason these are plain PHP objects and not some special case is that
you can re-use them for configuration from different sources. Symfony
Validator is a good example where the "attributes" could map to the
validator configuration classes that are also used from XML, YAML, ...

Yes, INSTANCE_OF attempts to load each attributes class, but if an
attribute class cannot be looked up (not autoloadable) it gets skipped
without error (subject to error handling of autoloader implementation, but
for Composer it skips).

>
> I expect annotations to be readonly, which classes as outlined in the
> RFC cannot currently enforce in PHP. A syntax similar to interfaces
> might be appropriate, I'm not sure whether that has been discussed:
>

The write-once / readonly RFC was rejected and only internal classes can
implement this behavior (see ext/dom). But userland attributes also map to
userland classes, so as you say this is not possible. However given this
RFC maps to existing concepts.

I don't see how preventing instantiation or requiring readonly in userland
produces any benefits over this described use-case in the RFC. This would
make attributes much stricter than everything else in the language, I don't
see how its fair to impose this on a new feature like Attributes, when the
language doesn't even have concepts for this strictness for regular classes
(containing more important code). Mapping to "normal" classes is the way C#
implements attributes, I don't think this should be more complex than that.

Extensions, and Third Party library authors can easily guard their
attribute classes against writes from the outside with the usual OOP
abstractions and if application authors deem their attributes to be
writable that is their business.


>
> ```php
> <?php
>
> namespace My\Attributes;
>
> attribute SingleArgument
> {
>     public function value(): string;
> }
> ```
>
> Such a Java-like syntax would unfortunately only work with some kind
> of named parameters.
>

Named parameters might some day come to PHP in the future, and this is
precisely the argument for treating an attribute like a regular php class
with a constructor, because the named params syntax would look exactly the
same in attribute declarations then, increasing language consistency.

The reason I went with the C# way of mapping to a "plain class" is
simplicity. The concept of attributes is already not the easiest nut to
crack, inventing another keyword and a structure that looks like a class,
but has very different implications requires a lot of words in the
documentation and doesn't provide the easiest access to beginners.


>
> Finally, the naming of "getAsObject" should IMO be improved,
> especially if I read the RFC correctly and autoloading and constructor
> invocation is performed by this method. I think "createObject" or
> "toObject" might be better names.
>

The name getAsObject is an implementation detail in my perspective. I am
open for a better name.

>
> In summary, I'd probably vote "no" on the current proposal, even if
> it's one of the most missed features, because I think we can do
> better, and there's only one chance.
>

Sorry to hear and I hope you reconsider after reading my reply, as I built
this RFC around extensibility in the future primarily to address a lot of
voter feedback from the last RFCs.

For example things you mentioned: named parameters, readonly properties,
constructor property promotion, PHP namespace. These topics are all
currently or recently discussed and would all automatically lead to
improvements for attributes, when they "just" map to classes and their
constructor. If we would go with special cases for Attributes, we have to
solve all of these problems a second time, and this RFC would explode in
complexity.

>
> Best,
> Niklas
>
>
> Am Fr., 17. Apr. 2020 um 12:49 Uhr schrieb Benjamin Eberlei
> <kontakt@beberlei.de>:
> >
> > As there has only been minimal new discussion after the last changes to
> the
> > RFC I wanted to give a heads up that I will open the vote on Monday
> > afternoon.
> >
> > If you have further remarks or questions about the RFC, please let me
> know.
> >
> > On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei <kontakt@beberlei.de>
> wrote:
> >
> > > Hi all,
> > >
> > > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > > 2016 with a few changes, incorporating feedback from the mailing list
> back
> > > then and from talking to previous no voters.
> > >
> > > The RFC is at https://wiki.php.net/rfc/attributes_v2
> > >
> > > A working patch is at https://github.com/beberlei/php-src/pull/2
> though
> > > work around the details is still necessary.
> > >
> > > The RFC contains a section with common criticism and objections to
> > > attributes, and I hope to have collected and responded to a good amount
> > > already from previous discussions.
> > >
> > > There is also a fair amount of implementation detail still up for
> debate,
> > > which is noted in "Open Issues". I have pre-committed to one approach,
> but
> > > listed alternatives there. On these issues I am looking for your
> feedback.
> > >
> > > greetings
> > > Benjamin
> > >
>

--000000000000d7cbf205a3ac2d9c--