Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110382
MIME-Version: 1.0
References: <CAEPJdum6xCwABjpor1YYvuosSB1dEb4xnCZEMABKsEEvNXvcQQ@mail.gmail.com>
 <CAEPJdukfeRs2H8+qsXpHZgTeEaZnvDYXUtCGOhTu1ZZDBwehHA@mail.gmail.com>
 <CANeKSiQK6mGwCANDB=O8JsLieC8JM1uGeuRoguTQPDBgqO553g@mail.gmail.com> <CAEPJdumeH+DY=v8xQyV=wYYPbO8eb60CVrHv-00JSD+gK8i49A@mail.gmail.com>
In-Reply-To: <CAEPJdumeH+DY=v8xQyV=wYYPbO8eb60CVrHv-00JSD+gK8i49A@mail.gmail.com>
Date: Thu, 4 Jun 2020 23:57:57 -0400
Message-ID: <CAFp73Xvt9U5viFKJG66oLJPpm-v311RNmwAJ2J_zq_Fx5R7uWw@mail.gmail.com>
To: Benjamin Eberlei <kontakt@beberlei.de>
Cc: Benas IML <benas.molis.iml@gmail.com>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] Re: [RFC] Amendments to Attributes
From: guilhermeblanco@gmail.com ("guilhermeblanco@gmail.com")

Hi Benjamin,

Overall, all these amendments are good in my opinion, but I'd like to
challenge a few things:

1- On item 3, the acceptable targets would be: class, function,
method, property, class constant, parameter or all.
If possible, I'd like to ask if it's possible to expand this list and
also allow attribute and constructor.

2- Also on item 3, the validation of PhpAttribute targets, it feels
more natural to have this as an array instead of a bitwise or
operator.
Have you evaluated the performance penalty to judge your decision of
bitwise vs array?

3- Repeatability should be on its own PhpAttribute. It would not block
the expansion of the repeatability in future efforts.
One possibility could be group repeated attributes as another
PhpAttribute. Example: Multiple <<Schedule>> be folded into a
<<Schedules>>.
This code:

<<Schedule("0 0 12 * * MON-FRI")>>
<<Schedule("0 0 18 * * SUN,SAT")>>

Would be equivalent to (sorry if syntax is not 100% correct):

<<Schedules([
    <<Schedule("0 0 12 * * MON-FRI")>>,
    <<Schedule("0 0 18 * * SUN,SAT")>>
])>>

Considering:

<<PhpAttribute(PhpAttribute::TARGET_CLASS)>>
<<PhpRepeatable(Schedules::class)>>
class Schedule { public string $cron; /* ... */ }

<<PhpAttribute(PhpAttribute::TARGET_CLASS)>>
class Schedules { public array value = []; // Holds an array of Schedule  }

4- Now you might have recall about my initial thoughts on this
subject, but inheritance is something that would be very interesting
to see as part of this amendment.
If we introduce something like <<PhpAttributeInherited>> to the
Attribute definition, we could then mark the Attribute to be inherited
to subclasses of attributed class, while keeping the default to do not
inherit anything (like we have today).

Cheers,

On Thu, Jun 4, 2020 at 1:49 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:
>
> On Thu, Jun 4, 2020 at 12:54 PM Benas IML <benas.molis.iml@gmail.com> wrote:
>
> > Thank you for the update! Given that there is still an open issue, is the
> > RFC proposing flags or a separate `<<Repeatable>>` attribute?
> >
>
> Good point, we came to the conclusion to simplify. Should attributes be in
> the global namespace, then we shouldn't arbitrarily add more, so it will be
> a flag.
> At that point, because you rarely declare new flags we decided to merge
> target and flags and only have one flag. You could do the following:
>
> <<PhpAttribute(self::TARGET_METHOD | self::IS_REPEATABLE)>>
>
> >
> > Best regards,
> > Benas
> >
> > On Thu, Jun 4, 2020, 12:29 PM Benjamin Eberlei <kontakt@beberlei.de>
> > wrote:
> >
> >> I have changed back the rename from namespacing to Attributes\Attribute to
> >> using just Attribute after a few discussions off list. The reasoning is
> >> that it becomes more clear that a majority of core contributors strongly
> >> prefers using the global namespace as the PHP namespace and opening up
> >> this
> >> point again makes no sense. So the state of the RFC is again what it was
> >> when I originally posted it with renaming PhpAttribute to Attribute.
> >>
> >> Unless there is some new significant feedback I am going to open up this
> >> RFC for voting on Monday next week.
> >>
> >> On Wed, May 20, 2020 at 7:07 PM Benjamin Eberlei <kontakt@beberlei.de>
> >> wrote:
> >>
> >> > Hi everyone,
> >> >
> >> > the Attributes RFC was rather large already, so a few things were left
> >> > open or discussions during the vote have made us rethink a things.
> >> >
> >> > https://wiki.php.net/rfc/attribute_amendments
> >> >
> >> > These points are handled by the Amendments RFC to Attributes:
> >> >
> >> > 1. Proposing to add a grouped syntax <<Attr1, Attr2>
> >> > 2. Rename PhpAttribute to Attribute in global namespace (independent of
> >> > the namespace RFC)
> >> > 3. Add validation of attribute class targets, which internal attributes
> >> > can do, but userland can't
> >> > 4. Specification if an attribute is repeatable or not on the same
> >> > declaration and fail otherwise.
> >> >
> >> > Each of them is a rather small issue, so I hope its ok to aggregate all
> >> > four of them in a single RFC. Please let me know if it's not.
> >> >
> >> > greetings
> >> > Benjamin
> >> >
> >>
> >



-- 
Guilherme Blanco
SVP Technology at Statflo Inc.
Mobile: +1 647 232 5599