Re: [PHP-DEV] List of attributes

[Rowan Tommins]

On 24/11/2020 23:09, Theodore Brown wrote:

That's not necessarily the case for nested attributes. A developer
may want to declare that an attribute constructor takes a single
attribute with a particular type as one of its arguments.



I'm still not sure what the use case for such an attribute constructor 
would be. Your example with "CustomError" seemed to be abusing nested 
attributes as a way to get a value object into a static declaration; and 
all the examples I've seen of nested attributes/annotations "in the 
wild" are wrapping a list of attributes in some kind of container.




Moreover, re-purposing the syntax for function
calls or class instantiation is likely to cause confusion, since it
looks like the code is doing something other than what it actually
does (and of course it would prevent ever extending constant
expressions to support function calls or class instantiation).


Then maybe we should make it *be* instantiation. The example you gave 
earlier, of CustomError, would be just as useful outside attributes, 
e.g. in a property initialiser, so perhaps what we need for that use 
case is not nested attributes at all, but the ability to create some 
subset of objects ("structs"?) inside static contexts.


That can be entirely parallel to a feature that lets you wrap a list of 
attributes inside another attribute, and does some recursive magic with 
reflection to let you delay calling their constructors.


Regards,

--
Rowan Tommins (né Collins)
[IMSoP]

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

Re: [PHP-DEV] List of attributes

[Theodore Brown]

On Sat, Oct 31, 2020, at 5:06 AM, Rowan Tommins wrote:

> > This would be an artificial limitation on attributes to patch
> > over the inherent inconsistency of the grouped syntax for nested
> > attributes.
>
> There is no artificial limitation; there is a binary choice: does
> #[Foo] represent an object or a list with one item in? This is
> completely new syntax, so there is nothing for it to be inconsistent
> with other than itself.

It is very much an artificial limitation. Developers would be prevented
from declaring that an attribute must be passed a single attribute 
with a particular type as one of its parameters. This isn't a natural
or inherent limitation of attributes, but an artificial one in order
to try to make the `#[Attr1, Attr2]` grouped syntax usable with nested
attributes.

> I am arguing that having it represent a list with one item in is
> actually *more* consistent, because when you attach attributes to a
> declaration, they are always retrieved as a list.

That's not necessarily the case for nested attributes. A developer
may want to declare that an attribute constructor takes a single
attribute with a particular type as one of its arguments. Why should
PHP prevent this by only allowing a (possibly empty) list to be
passed? Again, this would be an artificial limitation that reduces
the flexibility and usefulness of nested attributes.

On Sat, Oct 31, 2020 at 7:38 PM Larry Garfield  wrote:

> Perhaps a naive question, but I'm missing the downside of:
> 
> #[Foo(Bar(name="baz"))]
> 
> #[Attribute]
> class Foo {
>   public function construct(public Bar $bar) {}
> }
> 
> class Bar {
>   public function construct(public string $name) {}
> }
> 
> Why is that not OK?  Someone mentioned it means you couldn't call a
> function there, but... that's not a huge problem because I can't
> imagine why you would.  If we really wanted to avoid that:
> 
> #[Foo(new Bar(name="baz"))]
>
> That would be unambiguous, if a bit ugly.

As I mentioned in my first email in this thread, there was an attempt
to do this back when the original `<<>>` attribute syntax was being
implemented. But this approach ran into difficulties since it would
require significant changes to constant expressions.

Even if someone finds a reasonable way to make such a syntax *work*,
it still has the downside of requiring a different syntax for top-
level and nested attributes (docblock annotations allow using the
same syntax for both). Moreover, re-purposing the syntax for function
calls or class instantiation is likely to cause confusion, since it
looks like the code is doing something other than what it actually
does (and of course it would prevent ever extending constant
expressions to support function calls or class instantiation).

One of my main motivations for proposing the `@@` shorter attribute
syntax was to avoid the complexity of grouped declarations and allow
using a single consistent syntax for both top level and nested
attributes. I fear that the change to the `#[]` syntax with grouping
will prevent PHP from getting a flexible and intuitive nested
attribute implementation, and devs will be forced to stick with
docblock annotations for these use cases.

Kind regards,  
Theodore

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

Re: [PHP-DEV] List of attributes

[Larry Garfield]

On Sat, Oct 31, 2020, at 5:06 AM, Rowan Tommins wrote:

> > This would be
> >an artificial limitation on attributes to patch over the inherent
> >inconsistency of the grouped syntax for nested attributes.
> 
> 
> There is no artificial limitation; there is a binary choice: does 
> #[Foo] represent an object or a list with one item in? This is 
> completely new syntax, so there is nothing for it to be inconsistent 
> with other than itself.
> 
> I am arguing that having it represent a list with one item in is 
> actually *more* consistent, because when you attach attributes to a 
> declaration, they are always retrieved as a list.

Perhaps a naive question, but I'm missing the downside of:

#[Foo(Bar(name="baz"))]

#[Attribute]
class Foo {
  public function construct(public Bar $bar) {}
}

class Bar {
  public function construct(public string $name) {}
}

Why is that not OK?  Someone mentioned it means you couldn't call a function 
there, but... that's not a huge problem because I can't imagine why you would.  
If we really wanted to avoid that:

#[Foo(new Bar(name="baz"))]

That would be unambiguous, if a bit ugly.

--Larry Garfield

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

Re: [PHP-DEV] List of attributes

[Rowan Tommins]

On 30 October 2020 21:54:10 GMT+00:00, Theodore Brown  
wrote:
>The fundamental advantage of using an attribute here instead of an
>associative array is that it enables IDE autocompletion, and allows
>static analyzers to catch any mistakes in the parameter names/types
>before the code runs in production.


Yes, in general being able to declare structures rather than associative arrays 
is useful. Note though that that's not unique to attributes, nor are third 
party static analysers limited by the native facilities of the language.


>> So there's nothing really "attribute-y" about the CustomError class,
>
>What do you mean by "attribute-y"? 


I mean that unlike the Symfony examples, CustomError is not something that 
you'd use as an attribute on its own, actually attached to a declaration; it's 
just a container for some data. 



>The `CustomError` class would be a
>normal attribute, and as such when calling `getArguments()` on the
>`Limit` ReflectionAttribute it would return a ReflectionAttribute for
>`CustomError`, which would allow its arguments to be inspected without
>loading the class.


I don't understand what the advantage of that would be in this case. If you 
"inspect" the arguments without running the constructor or even loading the 
class definition, isn't that just the same as having an associative array?


>> and any "static object declaration" syntax would do, e.g.
>> 
>> #[Limit(
>> min: 10,
>> max: 100,
>> error: CustomError{message="...", notify=true,
>withTrace=false}
>> )]
>
>It's not clear what exactly you're proposing here, or how it would
>work.


I don't want to get bogged down in the details, it was just a quick example; 
the point I was trying to make was that if CustomError is just a data object, 
then the requirement is some syntax for creating data objects in scopes where 
that's not currently allowed. That includes in the arguments to attributes, but 
also includes, for instance, constants, property defaults, and parameter 
defaults. 


>There is no general problem of not being able to specify that a
>parameter takes a single object with a particular type. 


No, but there's a general problem of not being able to specify that a parameter 
takes a list of a certain type, and this would be one of many, many places 
where that is a minor nuisance largely solved by third party static analysis 
tools.



> This would be
>an artificial limitation on attributes to patch over the inherent
>inconsistency of the grouped syntax for nested attributes.


There is no artificial limitation; there is a binary choice: does #[Foo] 
represent an object or a list with one item in? This is completely new syntax, 
so there is nothing for it to be inconsistent with other than itself.

I am arguing that having it represent a list with one item in is actually 
*more* consistent, because when you attach attributes to a declaration, they 
are always retrieved as a list.


Regards,

-- 
Rowan Tommins
[IMSoP]

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

Re: [PHP-DEV] List of attributes

[Theodore Brown]

On Fri, Oct 30, 2020 at 2:28 PM Rowan Tommins  wrote:

> On 30/10/2020 18:47, Theodore Brown wrote:
> > While passing all nested attributes as an array would at least enable
> > consistent semantics, it has the notable disadvantage of preventing
> > some use cases from being expressed in PHP's type system. Specifically,
> > how would you express that an attribute parameter must be passed a
> > single attribute of a particular type?
> 
> I acknowledged this use case, but suspected, and still suspect, that
> it's rarer than the list-of-attributes case.
> 
> > For example, suppose a developer wants to create/use an attribute
> > like the following:
> >
> > #[Limit(
> > min: 10,
> > max: 100,
> > error: #[CustomError(message: "...", notify: true, withTrace: 
> > false)]
> > )]
> 
> What is the fundamental advantage of using an attribute as the argument
> here? The same thing can be expressed with an associative array:
> 
> #[Limit(
> min: 10,
> max: 100,
> error: ["message" => "...", "notify" => true, "withTrace" => false]
> )]
> 
> The only thing that seems to lose is the ability to specify the allowed
> names and types of options - but those won't be checked by the
> ReflectionAttribute anyway, only once the actual constructor is run.


The fundamental advantage of using an attribute here instead of an
associative array is that it enables IDE autocompletion, and allows
static analyzers to catch any mistakes in the parameter names/types
before the code runs in production.

> So there's nothing really "attribute-y" about the CustomError class,

What do you mean by "attribute-y"? The `CustomError` class would be a
normal attribute, and as such when calling `getArguments()` on the
`Limit` ReflectionAttribute it would return a ReflectionAttribute for
`CustomError`, which would allow its arguments to be inspected without
loading the class.

> and any "static object declaration" syntax would do, e.g.
> 
> #[Limit(
> min: 10,
> max: 100,
> error: CustomError{message="...", notify=true, withTrace=false}
> )]

It's not clear what exactly you're proposing here, or how it would work.
E.g. would this still allow inspecting the attribute arguments without
loading the `CustomError` class? Why is a new syntax needed for this?
Would it limit what the `CustomError` constructor is allowed to do? 


> > But if nested attributes are always passed as an array, the `$error`
> > parameter would have to have an `array` type, which provides
> > essentially no context about what is really required. The type system
> > would be perfectly happy to allow an empty array, or an array with
> > multiple values instead of the expected single `CustomError` attribute.
> 
> 
> Again, this is a general problem, not one related to attributes: the
> Symfony validation examples would be best declared as requiring
> "Constraint[] $constraints" rather than "array $constraints".

There is no general problem of not being able to specify that a
parameter takes a single object with a particular type. This would be
an artificial limitation on attributes to patch over the inherent
inconsistency of the grouped syntax for nested attributes.


> > 3. Vote to switch to a less verbose syntax [...]
> > The downside is that...
> 
> Let me stop you there. The downside is that a mob of Internals regulars
> will come to your house and lynch you for asking them to vote on the
> same thing yet again. It just ain't gonna happen.

I doubt anyone is more tired and worn out from discussing and voting
on attributes than I am. But there are serious issues when it comes to
supporting nested attributes with the #[] syntax and attribute grouping.
Shouldn't I bring them up and propose solutions before PHP 8 is released?

Best regards,  
Theodore

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

Re: [PHP-DEV] List of attributes

[Paul Jones]

> On Oct 30, 2020, at 14:28, Rowan Tommins  > wrote:
> 
>> 3. Vote to switch to a less verbose syntax [...]
>>The downside is that...
> 
> Let me stop you there. The downside is that a mob of Internals regulars will 
> come to your house and lynch you for asking them to vote on the same thing 
> yet again. It just ain't gonna happen.

Yes, because we voted repeatedly until the voters "got it right" -- and now 
there cannot ever be another vote on it again.


-- 
Paul M. Jones
pmjo...@pmjones.io 
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php 

Re: [PHP-DEV] List of attributes

[Rowan Tommins]

On 30/10/2020 18:47, Theodore Brown wrote:

While passing all nested attributes as an array would at least enable
consistent semantics, it has the notable disadvantage of preventing
some use cases from being expressed in PHP's type system. Specifically,
how would you express that an attribute parameter must be passed a
single attribute of a particular type?



I acknowledged this use case, but suspected, and still suspect, that 
it's rarer than the list-of-attributes case.




For example, suppose a developer wants to create/use an attribute
like the following:

 #[Limit(
 min: 10,
 max: 100,
 error: #[CustomError(message: "...", notify: true, withTrace: false)]
 )]



What is the fundamental advantage of using an attribute as the argument 
here? The same thing can be expressed with an associative array:


#[Limit(
min: 10,
max: 100,
error: ["message" => "...", "notify" => true, "withTrace" => false]
)]

The only thing that seems to lose is the ability to specify the allowed 
names and types of options - but those won't be checked by the 
ReflectionAttribute anyway, only once the actual constructor is run. So 
there's nothing really "attribute-y" about the CustomError class, and 
any "static object declaration" syntax would do, e.g.


#[Limit(
min: 10,
max: 100,
error: CustomError{message="...", notify=true, withTrace=false}
)]




But if nested attributes are always passed as an array, the `$error`
parameter would have to have an `array` type, which provides
essentially no context about what is really required. The type system
would be perfectly happy to allow an empty array, or an array with
multiple values instead of the expected single `CustomError` attribute.



Again, this is a general problem, not one related to attributes: the 
Symfony validation examples would be best declared as requiring 
"Constraint[] $constraints" rather than "array $constraints".




3. Vote to switch to a less verbose syntax [...]
The downside is that...



Let me stop you there. The downside is that a mob of Internals regulars 
will come to your house and lynch you for asking them to vote on the 
same thing yet again. It just ain't gonna happen.



Regards,

--
Rowan Tommins (né Collins)
[IMSoP]

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

Re: [PHP-DEV] List of attributes

[Theodore Brown]

Hi Rowan and Nicolas (and internals),

On Wed, Oct 28, 2020 at 12:57 PM Rowan Tommins  wrote:

> On 28/10/2020 16:58, Nicolas Grekas wrote:
> > about why we'd need nested attributes, here is a discussion about
> > nested validation constraints:
> > https://github.com/symfony/symfony/issues/38503
> 
> Thanks, it's useful to see some real-world examples. As I suspected,
> nearly all of these explicitly take a *list* of nested attributes,
> not a single attribute.
> 
> > While most of the constraints receive the nested constraints as
> > simple array, |Collection| however requires a mapping (field to
> > constraint) and is usually combined with other composite
> > constraints, which gives us a second nesting level.
> 
> There is much discussion of Collection as an edge case, but although
> the nested attributes are indeed inside a mapping, the *value* of
> that mapping is again a list of attributes. Single items are allowed,
> but this seems to be a special case for convenience. ... This
> reinforces my earlier suggestion (https://externals.io/message/111936#112109)
> that #[Foo] in a nested context can simply imply an array of one
> attribute,


While passing all nested attributes as an array would at least enable
consistent semantics, it has the notable disadvantage of preventing
some use cases from being expressed in PHP's type system. Specifically,
how would you express that an attribute parameter must be passed a
single attribute of a particular type?

For example, suppose a developer wants to create/use an attribute
like the following:

#[Limit(
min: 10,
max: 100,
error: #[CustomError(message: "...", notify: true, withTrace: false)]
)]

I would expect to be able to write the `Limit` attribute class like this:

#[Attribute]
class Limit {
public function __construct(
public int $min,
public int $max,
public CustomError $error,
) {}
}

But if nested attributes are always passed as an array, the `$error`
parameter would have to have an `array` type, which provides
essentially no context about what is really required. The type system
would be perfectly happy to allow an empty array, or an array with
multiple values instead of the expected single `CustomError` attribute.

It may be possible to enable static analysis type checks and IDE
autocompletion by adding extra docblock annotations or attributes
specifying that the array has to contain exactly one `CustomError`
value, but this is a workaround requiring a lot more effort than
should be necessary. Overall the dev experience becomes a lot worse
for these use cases.


On Wed, Oct 28, 2020 at 11:58 AM Nicolas Grekas  
wrote:

> > The problem with this is that it results in inconsistent semantics,
> > where the number of items in an attribute group results in a
> > different type of value being passed. I.e. if you remove two of the
> > three attributes from the list, suddenly the code will break since
> > the `Assert\All` attribute is no longer being passed an array.
> 
> Yes.
> This would be solved by NOT accepting #[] inside attribute declarations.
> Since we borrowed from Rust, let's borrow this again:
> http://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/reference/attributes.html#attributes
> 
> The Rust syntax accepts the #[] only on the outside of the declaration.
> The example above should be written as:
>
> #[Assert\All([
> Assert\Email(),
> Assert\NotBlank(),
> Assert\Length(max: 100)
> ])]
> 
> The closing brackets () would be required to remove any ambiguity
> with constants. Since constant expressions won't ever allow functions
> in them, this isn't ambiguous with function calls.


This does seem more readable and also avoids the grouped syntax
inconsistency. On the other hand, presumably it would prevent ever
supporting function calls in constant expressions. At present I'm not
convinced constant expressions *should* support this, but nevertheless
I have reservations about a syntax choice that would rule it out
completely.

Furthermore, the requirement of parentheses seems inconsistent with
normal attribute declarations and `new ClassName` instantiations,
where parentheses are optional.

As I see it, we have the following other options:

1. Simply ban the grouped syntax in nested context. This has the
downside of extra verbosity and that it may be unexpected/surprising
for developers.

2. Drop support for attribute grouping before PHP 8 is finalized. Rust
(which we borrowed the `#[]` syntax from) itself does not support this
anyway. The primary downside of no attribute grouping is additional
verbosity in some circumstances.

   Note: a number of people who voted for `#[]` expressed disfavor of
attribute grouping, which was originally accepted only for the `<<>>`
syntax with the qualification that it would be superseded by any other
RFC that changes the syntax. [1]

3. Vote to switch to a less verb

Re: [PHP-DEV] List of attributes

[Rowan Tommins]

On 28/10/2020 16:58, Nicolas Grekas wrote:
about why we'd need nested attributes, here is a discussion about 
nested validation constraints:

https://github.com/symfony/symfony/issues/38503



Thanks, it's useful to see some real-world examples. As I suspected, 
nearly all of these explicitly take a *list* of nested attributes, not a 
single attribute.


> While most of the constraints receive the nested constraints as 
simple array, |Collection| however requires a mapping (field to 
constraint) and is usually combined with other composite constraints, 
which gives us a second nesting level.



There is much discussion of Collection as an edge case, but although the 
nested attributes are indeed inside a mapping, the *value* of that 
mapping is again a list of attributes. Single items are allowed, but 
this seems to be a special case for convenience.


In the below example from 
https://symfony.com/doc/current/reference/constraints/Collection.html 
the @Assert\Email could equivalently be written as an array with one item:


    /**
 * @Assert\Collection(
 * fields = {
 * "personal_email" = @Assert\Email,
 * "short_bio" = {
 * @Assert\NotBlank,
 * @Assert\Length(
 * max = 100,
 * maxMessage = "Your short bio is too long!"
 * )
 * }
 * },
 * allowMissingFields = true
 * )
 */


This reinforces my earlier suggestion 
(https://externals.io/message/111936#112109) that #[Foo] in a nested 
context can simply imply an array of one attribute, rendering the above as:


#[Assert\Collection(
    fields: [
 "personal_email" => #[Assert\Email],
 "short_bio" => #[
    Assert\NotBlank,
    Assert\Length(
  max: 100,
  maxMessage: "Your short bio is too long!"
   )
    ]
    ],
    allowMissingFields: true
]


Regards,

--
Rowan Tommins (né Collins)
[IMSoP]

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

Re: [PHP-DEV] List of attributes

[Nicolas Grekas]

Hi all!

about why we'd need nested attributes, here is a discussion about nested
validation constraints:
https://github.com/symfony/symfony/issues/38503

You'll see that we're looking hard into ways around using nested
attributes, but they all fall short. We actually concluded that we should
not create a hacky syntax before php-internals settled on the topic.
I invite you to review the ideas and arguments posted there to see a real
world need on the topic (please don't mind sharing your thoughts about the
topic in the PR of course if you think you can help us move forward.)

See more comments below:

Le ven. 23 oct. 2020 à 17:52, Theodore Brown  a
écrit :

> On Tue, Oct 20, 2020 at 10:13 AM Rowan Tommins 
> wrote:
>
> > On Mon, 19 Oct 2020 at 16:17, Theodore Brown 
> wrote:
> >
> > >
> > > In theory nested attributes could be supported in the same way with
> > > the `#[]` syntax, but it's more verbose and I think less intuitive
> > > (e.g. people may try to use the grouped syntax in this context, but
> > > it wouldn't work). Also the combination of brackets delineating both
> > > arrays and attributes reduces readability:
> > >
> > > #[Assert\All([
> > > #[Assert\Email],
> > > #[Assert\NotBlank],
> > > #[Assert\Length(max: 100)]
> > > ])]
> > >
> >
> > I think you're presupposing how a feature would work that hasn't
> > even been specced out yet. On the face of it, it would seem logical
> > and achievable for the above to be equivalent to this:
> >
> > #[Assert\All(
> >#[
> > Assert\Email,
> > Assert\NotBlank,
> > Assert\Length(max: 100)
> >]
> > )]
> >
> > i.e. for a list of grouped attributes in nested context to be
> > equivalent to an array of nested attributes.
>
> Hi Rowan,
>
> The problem with this is that it results in inconsistent semantics,
> where the number of items in an attribute group results in a
> different type of value being passed. I.e. if you remove two of the
> three attributes from the list, suddenly the code will break since
> the `Assert\All` attribute is no longer being passed an array.
>

Yes.
This would be solved by NOT accepting #[] inside attribute declarations.
Since we borrowed from Rust, let's borrow this again:
http://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/reference/attributes.html#attributes

The Rust syntax accepts the #[] only on the outside of the declaration.
The example above should be written as:
 #[Assert\All([
 Assert\Email(),
 Assert\NotBlank(),
 Assert\Length(max: 100)
 ])]

The closing brackets () would be required to remove any ambiguity with
constants.
Since constant expressions won't ever allow functions in them, this isn't
ambiguous with function calls.



> // error when Assert\All is instantiated since it's no longer being
> // passed an array of attributes:
>
> #[Assert\All(
> #[
> Assert\Email,
> ]
> )]
>
> So when removing nested attributes from a list such that there's only
> one left in the list, you'd have to remember to additionally wrap the
> attribute group in array brackets:
>
> #[Assert\All(
> [#[
> Assert\Email,
> ]]
> )]
>
> And of course when you want to add additional attributes to a group
> that originally had only one attribute, you'd have to remember to
> remove the extra array brackets.
>
> Generally speaking, having the number of items in a list change the
> type of the list is a recipe for confusion and unexpected errors. So
> I think the only sane approach is to ban using the grouped syntax in
> combination with nested attributes.
>
> Unfortunately, no one thought through these issues when proposing to
> change the shorter attribute syntax away from @@ and add the grouped
> syntax, and now it seems like we're stuck with a syntax that is
> unnecessarily complex and confusing to use for nested attributes.
>
> > Unless nested attributes were limited to being direct arguments to
> > another attribute, the *semantics* of nested attributes inside
> > arrays would have to be defined anyway (e.g. how they would look in
> > reflection, whether they would be recursively instantiated by
> > newInstance(), etc).
>
> Yes, the exact semantics of nested attributes need to be defined, but
> this is a completely separate topic from the grouped syntax issue
> described above.
>
> As a user I would expect nested attributes to be reflected the same
> as any other attribute (i.e. as a `ReflectionAttribute` instance).
> Calling `getArguments` on a parent attribute would return an array
> with `ReflectionAttribute` objects for any nested attribute passed
> as a direct argument or array value.
>

I 100% agree with this.


On first thought I think it makes sense to recursively instantiate
> nested attributes when calling `newInstance` on a parent attribute.
> This would allow attribute class constructors to specify the typ

Re: [PHP-DEV] List of attributes

[Rowan Tommins]

On 23/10/2020 16:52, Theodore Brown wrote:

The problem with this is that it results in inconsistent semantics,
where the number of items in an attribute group results in a
different type of value being passed. I.e. if you remove two of the
three attributes from the list, suddenly the code will break since
the `Assert\All` attribute is no longer being passed an array.



That's a fair point. Perhaps nested attributes could *always* be 
considered a list?


That may sound odd, but it's actually quite consistent with "normal" 
attributes:


#[Foo]
class C {}
var_dump( (new ReflectionClass('C'))->getAttributes() );

Gives an array with one item in it, not an object:

array(1) {
    [0]=> object(ReflectionAttribute)#2 (0) { }
}


So similarly:

#[Foo( 42, #[Bar] )
class C {}
var_dump( (new ReflectionClass('C'))->getAttributes()[0]->getArguments() );

Could give an array with one item in it as the argument:

array(2) {
    [0] => int(42),
    [1] => array(1) {
        [0]=> object(ReflectionAttribute)#3 (0) { }
    }
}


This actually feels quite intuitive, because we're used to square 
brackets representing arrays. The only downside is a slight 
inconvenience for cases where you want an argument of exactly one nested 
attribute, but I don't know how common that is.


Regards,

--
Rowan Tommins (né Collins)
[IMSoP]

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

Re: [PHP-DEV] List of attributes

[Benjamin Morel]

On Fri, 23 Oct 2020 at 19:08, Michael Voříšek - ČVUT FEL <
voris...@fel.cvut.cz> wrote:

> We can deprecate comments with "#" in 8.0 or 8.1
>
> ...
>
> Is there anything against excl. BC break of "#" comments? Who is for it?
>
>
I'm all for it, especially considering that it's trivial to fix old
codebases with automated tools with 100% confidence.
But I'm afraid it's too late to do this for 8.0, even though a major
release would have been the perfect time to deprecate such a feature.

— Benjamin

Re: [PHP-DEV] List of attributes

[Michael Voříšek - ČVUT FEL]

Wdyt about dedicating "#" char for attributes? 

We can deprecate comments with "#" in 8.0 or 8.1 


and in later version support attributes with "#" single character -
without [] brackets, ie. support: 


#Attribute1
#Attribte2
public function x()... 

which will be equivalent to: 


#[
#Attribute1
#Attribte2
]
public function x()... 


Is there anything against excl. BC break of "#" comments? Who is for it?


With kind regards / Mit freundlichen Grüßen / S přátelským pozdravem,

Michael Voříšek

On 23 Oct 2020 17:52, Theodore Brown wrote:


On Tue, Oct 20, 2020 at 10:13 AM Rowan Tommins  wrote:

On Mon, 19 Oct 2020 at 16:17, Theodore Brown  wrote:

In theory nested attributes could be supported in the same way with
the `#[]` syntax, but it's more verbose and I think less intuitive
(e.g. people may try to use the grouped syntax in this context, but
it wouldn't work). Also the combination of brackets delineating both
arrays and attributes reduces readability:

#[Assert\All([
#[Assert\Email],
#[Assert\NotBlank],
#[Assert\Length(max: 100)]
])]

I think you're presupposing how a feature would work that hasn't
even been specced out yet. On the face of it, it would seem logical
and achievable for the above to be equivalent to this:

#[Assert\All(
#[
Assert\Email,
Assert\NotBlank,
Assert\Length(max: 100)
]
)]

i.e. for a list of grouped attributes in nested context to be
equivalent to an array of nested attributes.


Hi Rowan,

The problem with this is that it results in inconsistent semantics,
where the number of items in an attribute group results in a
different type of value being passed. I.e. if you remove two of the
three attributes from the list, suddenly the code will break since
the `Assert\All` attribute is no longer being passed an array.

// error when Assert\All is instantiated since it's no longer being  
// passed an array of attributes:


   #[Assert\All(
   #[
   Assert\Email,
   ]
   )]

So when removing nested attributes from a list such that there's only
one left in the list, you'd have to remember to additionally wrap the
attribute group in array brackets:

   #[Assert\All(
   [#[
   Assert\Email,
   ]]
   )]

And of course when you want to add additional attributes to a group
that originally had only one attribute, you'd have to remember to
remove the extra array brackets.

Generally speaking, having the number of items in a list change the
type of the list is a recipe for confusion and unexpected errors. So
I think the only sane approach is to ban using the grouped syntax in
combination with nested attributes.

Unfortunately, no one thought through these issues when proposing to
change the shorter attribute syntax away from @@ and add the grouped
syntax, and now it seems like we're stuck with a syntax that is 
unnecessarily complex and confusing to use for nested attributes.



Unless nested attributes were limited to being direct arguments to
another attribute, the *semantics* of nested attributes inside
arrays would have to be defined anyway (e.g. how they would look in
reflection, whether they would be recursively instantiated by
newInstance(), etc).


Yes, the exact semantics of nested attributes need to be defined, but
this is a completely separate topic from the grouped syntax issue
described above.

As a user I would expect nested attributes to be reflected the same
as any other attribute (i.e. as a `ReflectionAttribute` instance).
Calling `getArguments` on a parent attribute would return an array
with `ReflectionAttribute` objects for any nested attribute passed
as a direct argument or array value.

On first thought I think it makes sense to recursively instantiate
nested attributes when calling `newInstance` on a parent attribute.
This would allow attribute class constructors to specify the type
for each attribute argument. But again, this is a separate discussion
from the issue of nested attribute syntax.

Kind regards,  
Theodore


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

Re: [PHP-DEV] List of attributes

[Theodore Brown]

On Tue, Oct 20, 2020 at 10:13 AM Rowan Tommins  wrote:

> On Mon, 19 Oct 2020 at 16:17, Theodore Brown  wrote:
> 
> >
> > In theory nested attributes could be supported in the same way with
> > the `#[]` syntax, but it's more verbose and I think less intuitive
> > (e.g. people may try to use the grouped syntax in this context, but
> > it wouldn't work). Also the combination of brackets delineating both
> > arrays and attributes reduces readability:
> >
> > #[Assert\All([
> > #[Assert\Email],
> > #[Assert\NotBlank],
> > #[Assert\Length(max: 100)]
> > ])]
> >
> 
> I think you're presupposing how a feature would work that hasn't
> even been specced out yet. On the face of it, it would seem logical
> and achievable for the above to be equivalent to this:
> 
> #[Assert\All(
>#[
> Assert\Email,
> Assert\NotBlank,
> Assert\Length(max: 100)
>]
> )]
> 
> i.e. for a list of grouped attributes in nested context to be
> equivalent to an array of nested attributes.

Hi Rowan,

The problem with this is that it results in inconsistent semantics,
where the number of items in an attribute group results in a
different type of value being passed. I.e. if you remove two of the
three attributes from the list, suddenly the code will break since
the `Assert\All` attribute is no longer being passed an array.

// error when Assert\All is instantiated since it's no longer being  
// passed an array of attributes:

#[Assert\All(
#[
Assert\Email,
]
)]

So when removing nested attributes from a list such that there's only
one left in the list, you'd have to remember to additionally wrap the
attribute group in array brackets:

#[Assert\All(
[#[
Assert\Email,
]]
)]

And of course when you want to add additional attributes to a group
that originally had only one attribute, you'd have to remember to
remove the extra array brackets.

Generally speaking, having the number of items in a list change the
type of the list is a recipe for confusion and unexpected errors. So
I think the only sane approach is to ban using the grouped syntax in
combination with nested attributes.

Unfortunately, no one thought through these issues when proposing to
change the shorter attribute syntax away from @@ and add the grouped
syntax, and now it seems like we're stuck with a syntax that is 
unnecessarily complex and confusing to use for nested attributes.

> Unless nested attributes were limited to being direct arguments to
> another attribute, the *semantics* of nested attributes inside
> arrays would have to be defined anyway (e.g. how they would look in
> reflection, whether they would be recursively instantiated by
> newInstance(), etc).

Yes, the exact semantics of nested attributes need to be defined, but
this is a completely separate topic from the grouped syntax issue
described above.

As a user I would expect nested attributes to be reflected the same
as any other attribute (i.e. as a `ReflectionAttribute` instance).
Calling `getArguments` on a parent attribute would return an array
with `ReflectionAttribute` objects for any nested attribute passed
as a direct argument or array value.

On first thought I think it makes sense to recursively instantiate
nested attributes when calling `newInstance` on a parent attribute.
This would allow attribute class constructors to specify the type
for each attribute argument. But again, this is a separate discussion
from the issue of nested attribute syntax.

Kind regards,  
Theodore

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

Re: [PHP-DEV] List of attributes

[Rowan Tommins]

On Mon, 19 Oct 2020 at 16:17, Theodore Brown  wrote:

>
> In theory nested attributes could be supported in the same way with
> the `#[]` syntax, but it's more verbose and I think less intuitive
> (e.g. people may try to use the grouped syntax in this context, but
> it wouldn't work). Also the combination of brackets delineating both
> arrays and attributes reduces readability:
>
> #[Assert\All([
> #[Assert\Email],
> #[Assert\NotBlank],
> #[Assert\Length(max: 100)]
> ])]
>


I think you're presupposing how a feature would work that hasn't even been
specced out yet. On the face of it, it would seem logical and achievable
for the above to be equivalent to this:

#[Assert\All(
   #[
Assert\Email,
Assert\NotBlank,
Assert\Length(max: 100)
   ]
)]

i.e. for a list of grouped attributes in nested context to be equivalent to
an array of nested attributes.

Unless nested attributes were limited to being direct arguments to another
attribute, the *semantics* of nested attributes inside arrays would have to
be defined anyway (e.g. how they would look in reflection, whether they
would be recursively instantiated by newInstance(), etc).

Regards,
-- 
Rowan Tommins
[IMSoP]

Re: [PHP-DEV] List of attributes

[Benas IML]

On Mon, Oct 19, 2020, 6:17 PM Theodore Brown  wrote:

> On Mon, Oct 5, 2020 at 9:24 AM Nicolas Grekas 
> wrote:
>
> >> I'm wondering if the syntax that allows for several attributes is
> >> really future-proof when considering nested attributes:
> >>
> >>
> >> *1.*
> >> #[foo]
> >> #[bar]
> >>
> >> VS
> >>
> >> *2.*
> >> #[foo, bar]
> >>
> >> Add nested attributes to the mix, here are two possible ways:
> >>
> >> *A.*
> >> #[foo(
> >> #[bar]
> >> )]
> >>
> >> or
> >>
> >>
> >> *B.*
> >> #[foo(
> >> bar
> >> )]
> >>
> >> The A. syntax is consistent with the 1. list. I feel like syntax
> >> B is not desired and could be confusing from a grammar pov.
> >> BUT in syntax 2., we allow an attribute to be unprefixed (bar),
> >> so that syntax B is consistent with 2.
> >>
> >> Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
> >> attributes are introduced?
> >>
> >> I voted yes for syntax 2. when the attributes were using << >>. I would
> >> vote NO now with the new syntax.
> >>
> >> Nicolas
> >
> > As far as my understanding goes, if we introduce "nested" attributes, it
> > will be in the form of relaxing constraints on constant expressions, i.e.
> > by allowing you to write #[Attr(new NestedAttr)].
> >
> > Nikita
>
> Back when the original `<<>>` attribute syntax was being implemented,
> there was an attempt to do just this. But it turned out to be very
> difficult to implement, and it was ultimately given up on since it
> required significant changes to const expressions. Earlier this year
> there was also a poll to gauge interest in supporting function calls
> in constant expressions, but most voters opposed it. [1]
>
> Even if a proposal for relaxing constraints on constant expressions
> gains enough support, it's not clear this is the ideal path forward
> for nested attributes, since as Nicolas pointed out this wouldn't
> have the same lazy-loading semantics as existing attributes.
>
> Straightforward support for nested attributes was one of my main
> motivations for proposing the `@@` syntax in the Shorter Attribute
> Syntax RFC, [2] and I had hoped to collaborate on a follow-up RFC to
> support nested attributes with the AT-AT syntax. This would allow
> existing nested docblock annotations such as Nicolas's example to
> translate intuitively to native attributes:
>
> @@Assert\All([
> @@Assert\Email,
> @@Assert\NotBlank,
> @@Assert\Length(max: 100)
> ])
>
> This would also preserve the lazy-loading feature where these
> attribute classes aren't loaded until code calls `newInstance`
> on one of the `ReflectionAttribute` objects.
>
> But then the Shorter Attribute Syntax Change RFC [3] came along and
> derailed this plan...
>
> In theory nested attributes could be supported in the same way with
> the `#[]` syntax, but it's more verbose and I think less intuitive
> (e.g. people may try to use the grouped syntax in this context, but
> it wouldn't work). Also the combination of brackets delineating both
> arrays and attributes reduces readability:
>

Welp, both variants seem to be readable.


> #[Assert\All([
> #[Assert\Email],
> #[Assert\NotBlank],
> #[Assert\Length(max: 100)]
> ])]
>
> But at this point I assume this is the most viable path forward for
> nested attributes (barring another syntax re-vote and delay of PHP 8).
>

Are you actually kidding me? 4th revote? Please no.

I know Derick and Benjamin have stated they aren't in favor of nested
> attributes and didn't put any thought into the syntax for them, but I
> feel this is unfortunate since nested attributes are an established
> pattern with legitimate use cases in existing libraries.
>
> Best regards,
> Theodore


Best regards,
Benas

>
> [1]: https://wiki.php.net/rfc/calls_in_constant_expressions_poll
> [2]: https://wiki.php.net/rfc/shorter_attribute_syntax
> [3]: https://wiki.php.net/rfc/shorter_attribute_syntax_change
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit:

https://www.php.net/unsub.php
>

Re: [PHP-DEV] List of attributes

[Theodore Brown]

On Mon, Oct 5, 2020 at 9:24 AM Nicolas Grekas  wrote:

>> I'm wondering if the syntax that allows for several attributes is
>> really future-proof when considering nested attributes:
>>
>>
>> *1.*
>> #[foo]
>> #[bar]
>>
>> VS
>>
>> *2.*
>> #[foo, bar]
>>
>> Add nested attributes to the mix, here are two possible ways:
>>
>> *A.*
>> #[foo(
>> #[bar]
>> )]
>>
>> or
>>
>>
>> *B.*
>> #[foo(
>> bar
>> )]
>>
>> The A. syntax is consistent with the 1. list. I feel like syntax
>> B is not desired and could be confusing from a grammar pov.
>> BUT in syntax 2., we allow an attribute to be unprefixed (bar),
>> so that syntax B is consistent with 2.
>>
>> Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
>> attributes are introduced?
>>
>> I voted yes for syntax 2. when the attributes were using << >>. I would
>> vote NO now with the new syntax.
>>
>> Nicolas
>
> As far as my understanding goes, if we introduce "nested" attributes, it
> will be in the form of relaxing constraints on constant expressions, i.e.
> by allowing you to write #[Attr(new NestedAttr)].
>
> Nikita

Back when the original `<<>>` attribute syntax was being implemented,
there was an attempt to do just this. But it turned out to be very
difficult to implement, and it was ultimately given up on since it
required significant changes to const expressions. Earlier this year
there was also a poll to gauge interest in supporting function calls
in constant expressions, but most voters opposed it. [1]

Even if a proposal for relaxing constraints on constant expressions
gains enough support, it's not clear this is the ideal path forward
for nested attributes, since as Nicolas pointed out this wouldn't
have the same lazy-loading semantics as existing attributes.

Straightforward support for nested attributes was one of my main
motivations for proposing the `@@` syntax in the Shorter Attribute
Syntax RFC, [2] and I had hoped to collaborate on a follow-up RFC to
support nested attributes with the AT-AT syntax. This would allow
existing nested docblock annotations such as Nicolas's example to
translate intuitively to native attributes:

@@Assert\All([
@@Assert\Email,
@@Assert\NotBlank,
@@Assert\Length(max: 100)
])

This would also preserve the lazy-loading feature where these
attribute classes aren't loaded until code calls `newInstance`
on one of the `ReflectionAttribute` objects.

But then the Shorter Attribute Syntax Change RFC [3] came along and
derailed this plan...

In theory nested attributes could be supported in the same way with
the `#[]` syntax, but it's more verbose and I think less intuitive
(e.g. people may try to use the grouped syntax in this context, but
it wouldn't work). Also the combination of brackets delineating both
arrays and attributes reduces readability:

#[Assert\All([
#[Assert\Email],
#[Assert\NotBlank],
#[Assert\Length(max: 100)]
])]

But at this point I assume this is the most viable path forward for
nested attributes (barring another syntax re-vote and delay of PHP 8).
I know Derick and Benjamin have stated they aren't in favor of nested
attributes and didn't put any thought into the syntax for them, but I
feel this is unfortunate since nested attributes are an established
pattern with legitimate use cases in existing libraries.

Best regards,  
Theodore

[1]: https://wiki.php.net/rfc/calls_in_constant_expressions_poll
[2]: https://wiki.php.net/rfc/shorter_attribute_syntax
[3]: https://wiki.php.net/rfc/shorter_attribute_syntax_change
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] List of attributes

[Nicolas Grekas]

>
> Hi Benjamin, hi everyone

 I'm wondering if the syntax that allows for several attributes is really
 future-proof when considering nested attributes:


 *1.*
 #[foo]
 #[bar]

 VS


 *2.*
 #[foo, bar]

 Add nested attributes to the mix, here are two possible ways:


 *A.*
 #[foo(
 #[bar]
 )]

 or


 *B.*
 #[foo(
 bar
 )]

 The A. syntax is consistent with the 1. list.
 I feel like syntax B is not desired and could be confusing from a
 grammar
 pov.
 BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
 syntax B is consistent with 2.

 Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
 attributes are introduced?

 I voted yes for syntax 2. when the attributes were using << >>. I would
 vote NO now with the new syntax.

 Nicolas

>>>
>>> As far as my understanding goes, if we introduce "nested" attributes, it
>>> will be in the form of relaxing constraints on constant expressions, i.e.
>>> by allowing you to write #[Attr(new NestedAttr)].
>>>
>>
>> That's what I've read in previous discussions and in other replies in
>> this thread.
>> This would break the possibility to read annotations without failing hard
>> when a class is missing.
>>
>> I would much prefer a solution that preserves this capability (which is
>> the reason why we have ReflectionAttribute::newInstance(): unless this one
>> is called, no autoloading happens.
>>
>> To me, this means that "new Foo()" nested in an attribute can't be a
>> solution.
>> Neither should we be able to nest PHP constants there btw.
>>
>
> Can you clarify your reasoning here to get to this conclusion? because
> with #[Foo(Foo::BAR)] we also not trigger autoloading right now until
> newInstance()
> or getArguments() is called. This is the way it is working right now:
> https://gist.github.com/beberlei/8150f60abaab3b0a70ebb76ce6a379b0
>
> Attributes allow constant expressions as arguments, the same which you can
> do in constant or property declarations for the default value.
>
> An approach with new NestedAttr() would work the same, only triggering
> autoload when argument needs to be evaluated for use.
>

My bad about constants, I was wrong, they already work as part of constant
expressions as you highlighted.

I was propagating this laziness requirement to nested attributes: they
should also be represented as instances of ReflectionAttribute to me at
some point. It would look important to me to be able to inspect a tree of
attributes and decide what to do with each of them before actually
instantiating them (and triggering the autoload cascade).

I'm not convinced that nested attributes should be brought to us via an
improvement of constant-expressions: first because that would break this
laziness requirement, second because I just doubt that constant-expressions
should support objects.

About providing laziness for attributes, this could be done by
ReflectionAttribute::getAttributes():
#[Foo(bar())] could lead to getAttribute returning an array containing a
ReflectionAttribute instance for attribute "bar".

Of course, this has many consequences and also many alternatives that'd
need to be evaluated.

I won't be able to lead an effort towards nested attributes in the short
term, so I'll just let this on the table here :)

Cheers,
Nicolas



>
>> Since we borrowed the syntax to Rust, here is the syntax they use:
>>
>> > #[validate(length(min = 1), custom = "validate_unique_username")]
>>
>> I think this would fit quite nicely for us too, by turning eg length into
>> an instance of ReflectionAttribute.
>>
>> Note that a use case for nested attributes is discussed in
>> https://github.com/symfony/symfony/pull/38309, to replace things like:
>> /**
>> @Assert\All([
>> @Assert\Email,
>> @Assert\NotBlank,
>> @Assert\Length(max=100)
>> ])
>> */
>>
>> We can work around of course, and we will for 8.0, but it would be nice
>> to have a plan forward because similar use cases (grouping attributes in a
>> wrapper attribute) are going to be pretty common IMHO.
>>
>> But we're getting a bit of topic. I'm fine keeping things as is for 8.0.
>> I just wanted to raise a point about the syntax for list of attributes but
>> it didn't get traction apparently.
>>
>> Cheers,
>> Nicolas
>>
>

Re: [PHP-DEV] List of attributes

[Benjamin Eberlei]

On Mon, Sep 28, 2020 at 2:58 PM Nicolas Grekas 
wrote:

>
> Hi Benjamin, hi everyone
>>>
>>> I'm wondering if the syntax that allows for several attributes is really
>>> future-proof when considering nested attributes:
>>>
>>>
>>> *1.*
>>> #[foo]
>>> #[bar]
>>>
>>> VS
>>>
>>>
>>> *2.*
>>> #[foo, bar]
>>>
>>> Add nested attributes to the mix, here are two possible ways:
>>>
>>>
>>> *A.*
>>> #[foo(
>>> #[bar]
>>> )]
>>>
>>> or
>>>
>>>
>>> *B.*
>>> #[foo(
>>> bar
>>> )]
>>>
>>> The A. syntax is consistent with the 1. list.
>>> I feel like syntax B is not desired and could be confusing from a grammar
>>> pov.
>>> BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
>>> syntax B is consistent with 2.
>>>
>>> Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
>>> attributes are introduced?
>>>
>>> I voted yes for syntax 2. when the attributes were using << >>. I would
>>> vote NO now with the new syntax.
>>>
>>> Nicolas
>>>
>>
>> As far as my understanding goes, if we introduce "nested" attributes, it
>> will be in the form of relaxing constraints on constant expressions, i.e.
>> by allowing you to write #[Attr(new NestedAttr)].
>>
>
> That's what I've read in previous discussions and in other replies in this
> thread.
> This would break the possibility to read annotations without failing hard
> when a class is missing.
>
> I would much prefer a solution that preserves this capability (which is
> the reason why we have ReflectionAttribute::newInstance(): unless this one
> is called, no autoloading happens.
>
> To me, this means that "new Foo()" nested in an attribute can't be a
> solution.
> Neither should we be able to nest PHP constants there btw.
>

Can you clarify your reasoning here to get to this conclusion? because with
#[Foo(Foo::BAR)] we also not trigger autoloading right now until
newInstance()
or getArguments() is called. This is the way it is working right now:
https://gist.github.com/beberlei/8150f60abaab3b0a70ebb76ce6a379b0

Attributes allow constant expressions as arguments, the same which you can
do in constant or property declarations for the default value.

An approach with new NestedAttr() would work the same, only triggering
autoload when argument needs to be evaluated for use.

>
> Since we borrowed the syntax to Rust, here is the syntax they use:
>
> > #[validate(length(min = 1), custom = "validate_unique_username")]
>
> I think this would fit quite nicely for us too, by turning eg length into
> an instance of ReflectionAttribute.
>
> Note that a use case for nested attributes is discussed in
> https://github.com/symfony/symfony/pull/38309, to replace things like:
> /**
> @Assert\All([
> @Assert\Email,
> @Assert\NotBlank,
> @Assert\Length(max=100)
> ])
> */
>
> We can work around of course, and we will for 8.0, but it would be nice to
> have a plan forward because similar use cases (grouping attributes in a
> wrapper attribute) are going to be pretty common IMHO.
>
> But we're getting a bit of topic. I'm fine keeping things as is for 8.0. I
> just wanted to raise a point about the syntax for list of attributes but it
> didn't get traction apparently.
>
> Cheers,
> Nicolas
>

Re: [PHP-DEV] List of attributes

[Nicolas Grekas]

> Hi Benjamin, hi everyone
>>
>> I'm wondering if the syntax that allows for several attributes is really
>> future-proof when considering nested attributes:
>>
>>
>> *1.*
>> #[foo]
>> #[bar]
>>
>> VS
>>
>>
>> *2.*
>> #[foo, bar]
>>
>> Add nested attributes to the mix, here are two possible ways:
>>
>>
>> *A.*
>> #[foo(
>> #[bar]
>> )]
>>
>> or
>>
>>
>> *B.*
>> #[foo(
>> bar
>> )]
>>
>> The A. syntax is consistent with the 1. list.
>> I feel like syntax B is not desired and could be confusing from a grammar
>> pov.
>> BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
>> syntax B is consistent with 2.
>>
>> Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
>> attributes are introduced?
>>
>> I voted yes for syntax 2. when the attributes were using << >>. I would
>> vote NO now with the new syntax.
>>
>> Nicolas
>>
>
> As far as my understanding goes, if we introduce "nested" attributes, it
> will be in the form of relaxing constraints on constant expressions, i.e.
> by allowing you to write #[Attr(new NestedAttr)].
>

That's what I've read in previous discussions and in other replies in this
thread.
This would break the possibility to read annotations without failing hard
when a class is missing.

I would much prefer a solution that preserves this capability (which is the
reason why we have ReflectionAttribute::newInstance(): unless this one is
called, no autoloading happens.

To me, this means that "new Foo()" nested in an attribute can't be a
solution.
Neither should we be able to nest PHP constants there btw.

Since we borrowed the syntax to Rust, here is the syntax they use:

> #[validate(length(min = 1), custom = "validate_unique_username")]

I think this would fit quite nicely for us too, by turning eg length into
an instance of ReflectionAttribute.

Note that a use case for nested attributes is discussed in
https://github.com/symfony/symfony/pull/38309, to replace things like:
/**
@Assert\All([
@Assert\Email,
@Assert\NotBlank,
@Assert\Length(max=100)
])
*/

We can work around of course, and we will for 8.0, but it would be nice to
have a plan forward because similar use cases (grouping attributes in a
wrapper attribute) are going to be pretty common IMHO.

But we're getting a bit of topic. I'm fine keeping things as is for 8.0. I
just wanted to raise a point about the syntax for list of attributes but it
didn't get traction apparently.

Cheers,
Nicolas

Re: [PHP-DEV] List of attributes

[Nikita Popov]

On Sun, Sep 27, 2020 at 10:23 AM Nicolas Grekas 
wrote:

> Hi Benjamin, hi everyone
>
> I'm wondering if the syntax that allows for several attributes is really
> future-proof when considering nested attributes:
>
>
> *1.*
> #[foo]
> #[bar]
>
> VS
>
>
> *2.*
> #[foo, bar]
>
> Add nested attributes to the mix, here are two possible ways:
>
>
> *A.*
> #[foo(
> #[bar]
> )]
>
> or
>
>
> *B.*
> #[foo(
> bar
> )]
>
> The A. syntax is consistent with the 1. list.
> I feel like syntax B is not desired and could be confusing from a grammar
> pov.
> BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
> syntax B is consistent with 2.
>
> Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
> attributes are introduced?
>
> I voted yes for syntax 2. when the attributes were using << >>. I would
> vote NO now with the new syntax.
>
> Nicolas
>

As far as my understanding goes, if we introduce "nested" attributes, it
will be in the form of relaxing constraints on constant expressions, i.e.
by allowing you to write #[Attr(new NestedAttr)].

Nikita

Re: [PHP-DEV] List of attributes

[Derick Rethans]

On 27 September 2020 09:22:48 BST, Nicolas Grekas  
wrote:
>Hi Benjamin, hi everyone
>
>I'm wondering if the syntax that allows for several attributes is
>really
>future-proof when considering nested attributes:
>
>
>*1.*
>#[foo]
>#[bar]
>
>VS
>
>
>*2.*
>#[foo, bar]



>Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
>attributes are introduced?

No. You're about 2 months too late for that.

I'm also not in favour of nested attributes. 

cheers,
Derick 

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

Re: [PHP-DEV] List of attributes

[Rowan Tommins]

On 27 September 2020 09:22:48 BST, Nicolas Grekas  
wrote:
>The A. syntax is consistent with the 1. list.
>I feel like syntax B is not desired and could be confusing from a
>grammar
>pov.
>BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
>syntax B is consistent with 2.


I think it's fairly straightforward to see #[...] as meaning "list of 
attributes", rather than as "change meaning of all identifiers recursively". 
So, this will look for a constant "MAGIC", not a nested attribute:

#[Foo(true, 42), Bar(MAGIC)] class X ...


Nested attributes are a slightly confusing concept anyway, because "normal" 
attributes are always attached to something, but nested attributes occur in 
place of a normal value. I can see their usefulness, but having the extra 
#[...] to say "not a normal value, extra magic happening here" seems sensible.


Presumably we could also allow grouping of nested attributes such that this:

#[Foo(#[Bar, Baz]) class X ...

Would just be equivalent to this:

#[Foo([ #[Bar], #[Baz] ]) class X ...

Regards,

-- 
Rowan Tommins
[IMSoP]

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

[PHP-DEV] List of attributes

[Nicolas Grekas]

Hi Benjamin, hi everyone

I'm wondering if the syntax that allows for several attributes is really
future-proof when considering nested attributes:


*1.*
#[foo]
#[bar]

VS


*2.*
#[foo, bar]

Add nested attributes to the mix, here are two possible ways:


*A.*
#[foo(
#[bar]
)]

or


*B.*
#[foo(
bar
)]

The A. syntax is consistent with the 1. list.
I feel like syntax B is not desired and could be confusing from a grammar
pov.
BUT in syntax 2., we allow an attribute to be unprefixed (bar), so that
syntax B is consistent with 2.

Shouldn't we remove syntax 2. in 8.0 and consider it again when nested
attributes are introduced?

I voted yes for syntax 2. when the attributes were using << >>. I would
vote NO now with the new syntax.

Nicolas