Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Mon, May 11, 2020 at 5:26 PM Dan Ackroyd wrote:
> On Mon, 11 May 2020 at 10:52, Nikita Popov wrote:
> >
> > Please tell me if you have further concerns
>
> Question - as this is just sugar, presumably it doesn't add much
> complexity to the PHP engine?
>
Yes, the implementation complexity for this feature is minimal, as these
things go.
> Vague concern - because it's a novel syntax, I find it hard to read,
> particularly when there are comments* for the properties. Do you think
> I'll just get used to it, or is it likely to be an ongoing issue?
>
Well, I can't really answer this with confidence, but given the number of
people who told me how much they love this feature in TypeScript, I would
hazard that you would indeed get used to it.
> Also, in five years time, if we have structs**, people will be
> wondering why we bothered with this. But I think it does make life
> easier for developers in the meantime.
>
When we discussed this topic in chat last time, I got the feeling that the
rough consensus was that introducing "structs" as a feature distinct from
classes is a bad idea, and we should instead try to make relevant features
available for classes. Constructor promotion is one of those :)
> class UrlToPdfProcessor
> {
> public function __construct(
> // The full url to reach the instance of chrome running in
> headless mode.
> public string $chromeUrl,
>
> // A path to a directory where the output PDF will be written to.
> // It must already exist and be writeable by the process running
> // the PDF processor.
> public string $tmpPath,
>
> // The internal domain name for the site to be rendered.
> // The domain name should not contain either protocol or path
> // components
> string $internalDomain
> ) {
> }
> }
>
As suggested in
https://wiki.php.net/rfc/constructor_promotion#coding_style_considerations,
my recommendation is to use normal @param annotations for constructor
arguments, as we do now. While interleaving parameters and comments is
certainly possible (already), it's certainly not common style, and I don't
think it will become common style with constructor promotion.
Regards,
Nikita
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Mon, 11 May 2020 at 10:52, Nikita Popov wrote:
>
> Please tell me if you have further concerns
Question - as this is just sugar, presumably it doesn't add much
complexity to the PHP engine?
Vague concern - because it's a novel syntax, I find it hard to read,
particularly when there are comments* for the properties. Do you think
I'll just get used to it, or is it likely to be an ongoing issue?
Also, in five years time, if we have structs**, people will be
wondering why we bothered with this. But I think it does make life
easier for developers in the meantime.
cheers
Dan
Ack
* Example of comments.
class UrlToPdfProcessor
{
public function __construct(
// The full url to reach the instance of chrome running in
headless mode.
public string $chromeUrl,
// A path to a directory where the output PDF will be written to.
// It must already exist and be writeable by the process running
// the PDF processor.
public string $tmpPath,
// The internal domain name for the site to be rendered.
// The domain name should not contain either protocol or path
// components
string $internalDomain
) {
}
}
** structs - https://github.com/Danack/RfcCodex/blob/master/structs.md
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Wed, May 6, 2020, at 3:31 AM, Nikita Popov wrote:
> On Tue, May 5, 2020 at 10:27 PM Larry Garfield
> wrote:
>
> > On Tue, May 5, 2020, at 7:35 AM, Nikita Popov wrote:
> >
> >
> > > Performing validation when the getAttributes() call is performed does
> > sound
> > > reasonable to me. We can also add a class flag to perform this validation
> > > only once (if it is successful), so the cost doesn't have to be paid by
> > > every single getAttributes() consumer.
> > >
> > > For the purpose of this RFC, I've now updated it to say that attributes
> > > will be applied to both properties and parameters (
> > > https://wiki.php.net/rfc/constructor_promotion#attributes), but with an
> > > explicit note that we should change the behavior prior to the PHP 8
> > release
> > > if it turns out to be problematic, e.g. with regards to an attribute
> > > validation feature. I think this is something of a detail, and we'll be
> > > mostly fine whichever way we chose, but it's hard to say right now which
> > > option is best, in particular with regard to future attributes
> > improvements.
> > >
> > > Regards,
> > > Nikita
> >
> > Thinking on it further, there's something sitting in the back of my head
> > that's bothering me. This helped me flesh it out.
> >
> > I like promotion, and think it will be great for simplifying code.
> > However, as attributes show as we load more bells and whistles onto
> > properties (even if they are desirable and useful bells and whistles) the
> > level of conflict is going to get larger.
> >
> > Consider, today we have:
> >
> > class Point {
> > protected int $x;
> > protected int $y;
> >
> > public function __construct(int $x, int $y) {
> > $this->x = $x;
> > $this->y = $y;
> > }
> > }
> >
> > And this is needlessly verbose. With promotion, that simplifies to:
> >
> > class Point {
> > public function __construct(protected int $x, protected int $y) {}
> > }
> >
> > And this is good. There seems little dispute about that.
> >
> > But then we add attributes, and it starts to get a little odd:
> >
> > class Point {
> > public function __construct(
> > <>
> > protected int $x,
> >
> > <>
> > protected int $y,
> > ) {}
> > }
> >
> > And has the possible confusion between parameter attributes and property
> > attributes.
> >
> > But... there's plenty of other things that may get added to properties.
> > We've discussed the "readonly" property and the more robust assymetric
> > visibility. What would that look like?
> >
> > class Point {
> > public function __construct(
> > <>
> > int $x {
> > public get,
> > private set,
> > },
> >
> > <>
> > int $y {
> > public get,
> > private set,
> > },
> > ) {}
> > }
> >
> > And that's now an awful lot of metadata to shove into, technically, a
> > function signature. And if accessor methods ever happen:
> >
> > class Point {
> > public function __construct(
> > <>
> > int $x {
> > public get () {
> > return $this->x;
> > },
> > private set($new) {
> > if ($new < 0) {
> > throw new Exception();
> > }
> > $this->x = $new;
> > },
> > },
> >
> > <>
> > int $y {
> > public get () {
> > return $this->y;
> > },
> > private set($new) {
> > if ($new < 0) {
> > throw new Exception();
> > }
> > $this->y = $new;
> > },
> > },
> > ) {}
> > }
> >
> > And now we have methods defined inside a function signature, and that's
> > just bizzarro. It also means at least 4 indent levels before you even get
> > to the body of the methods.
> >
> > And there's probably other things that could get attached to properties at
> > some point I'm not thinking of that someone will suggest.
> >
> > So I have to wonder, are we painting ourselves into a hole? Most of the
> > uses I can think of for extra-metadata-properties are... overlapping with
> > the case where you want to have constructor promotion. That means "well if
> > you need to do fancy stuff just don't use promotion" becomes a
> > non-sustainable answer, because the cases where you want both will increase
> > rather than be an edge case.
> >
> > Is there a way we can build in an escape hatch for ourselves to avoid
> > these issues, both attributes and for future extension?
> >
> > First idea off my head, which may or may not be good:
> >
> > Allow an alternate keyword to fill in the body of the constructor, but NOT
> > fill in the property. You still define the property yourself and put all
> > the extra metadata there. To wit:
> >
> > class Point {
> > <>
> > int $x {
> > public get () {
> > return $this->x;
> > },
> > private set($new) {
> > if ($new < 0) {
> > throw new Exception();
> > }
> > $this->x = $new;
> > },
> > },
> >
> > <>
> > int $y {
> > public get () {
> > return $this->y;
> > },
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Tue, May 5, 2020 at 10:27 PM Larry Garfield
wrote:
> On Tue, May 5, 2020, at 7:35 AM, Nikita Popov wrote:
>
>
> > Performing validation when the getAttributes() call is performed does
> sound
> > reasonable to me. We can also add a class flag to perform this validation
> > only once (if it is successful), so the cost doesn't have to be paid by
> > every single getAttributes() consumer.
> >
> > For the purpose of this RFC, I've now updated it to say that attributes
> > will be applied to both properties and parameters (
> > https://wiki.php.net/rfc/constructor_promotion#attributes), but with an
> > explicit note that we should change the behavior prior to the PHP 8
> release
> > if it turns out to be problematic, e.g. with regards to an attribute
> > validation feature. I think this is something of a detail, and we'll be
> > mostly fine whichever way we chose, but it's hard to say right now which
> > option is best, in particular with regard to future attributes
> improvements.
> >
> > Regards,
> > Nikita
>
> Thinking on it further, there's something sitting in the back of my head
> that's bothering me. This helped me flesh it out.
>
> I like promotion, and think it will be great for simplifying code.
> However, as attributes show as we load more bells and whistles onto
> properties (even if they are desirable and useful bells and whistles) the
> level of conflict is going to get larger.
>
> Consider, today we have:
>
> class Point {
> protected int $x;
> protected int $y;
>
> public function __construct(int $x, int $y) {
> $this->x = $x;
> $this->y = $y;
> }
> }
>
> And this is needlessly verbose. With promotion, that simplifies to:
>
> class Point {
> public function __construct(protected int $x, protected int $y) {}
> }
>
> And this is good. There seems little dispute about that.
>
> But then we add attributes, and it starts to get a little odd:
>
> class Point {
> public function __construct(
> <>
> protected int $x,
>
> <>
> protected int $y,
> ) {}
> }
>
> And has the possible confusion between parameter attributes and property
> attributes.
>
> But... there's plenty of other things that may get added to properties.
> We've discussed the "readonly" property and the more robust assymetric
> visibility. What would that look like?
>
> class Point {
> public function __construct(
> <>
> int $x {
> public get,
> private set,
> },
>
> <>
> int $y {
> public get,
> private set,
> },
> ) {}
> }
>
> And that's now an awful lot of metadata to shove into, technically, a
> function signature. And if accessor methods ever happen:
>
> class Point {
> public function __construct(
> <>
> int $x {
> public get () {
> return $this->x;
> },
> private set($new) {
> if ($new < 0) {
> throw new Exception();
> }
> $this->x = $new;
> },
> },
>
> <>
> int $y {
> public get () {
> return $this->y;
> },
> private set($new) {
> if ($new < 0) {
> throw new Exception();
> }
> $this->y = $new;
> },
> },
> ) {}
> }
>
> And now we have methods defined inside a function signature, and that's
> just bizzarro. It also means at least 4 indent levels before you even get
> to the body of the methods.
>
> And there's probably other things that could get attached to properties at
> some point I'm not thinking of that someone will suggest.
>
> So I have to wonder, are we painting ourselves into a hole? Most of the
> uses I can think of for extra-metadata-properties are... overlapping with
> the case where you want to have constructor promotion. That means "well if
> you need to do fancy stuff just don't use promotion" becomes a
> non-sustainable answer, because the cases where you want both will increase
> rather than be an edge case.
>
> Is there a way we can build in an escape hatch for ourselves to avoid
> these issues, both attributes and for future extension?
>
> First idea off my head, which may or may not be good:
>
> Allow an alternate keyword to fill in the body of the constructor, but NOT
> fill in the property. You still define the property yourself and put all
> the extra metadata there. To wit:
>
> class Point {
> <>
> int $x {
> public get () {
> return $this->x;
> },
> private set($new) {
> if ($new < 0) {
> throw new Exception();
> }
> $this->x = $new;
> },
> },
>
> <>
> int $y {
> public get () {
> return $this->y;
> },
> private set($new) {
> if ($new < 0) {
> throw new Exception();
> }
> $this->y = $new;
> },
> },
>
> public function __construct(promote int $x, promote int $y) {}
> }
>
> That eliminates 2 of the 4 places you'd have to repeat the name instead of
> 3, but means the constructor isn't burdened with having to hold a
> potentially non-trivial
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Tue, May 5, 2020, at 7:35 AM, Nikita Popov wrote:
> Performing validation when the getAttributes() call is performed does sound
> reasonable to me. We can also add a class flag to perform this validation
> only once (if it is successful), so the cost doesn't have to be paid by
> every single getAttributes() consumer.
>
> For the purpose of this RFC, I've now updated it to say that attributes
> will be applied to both properties and parameters (
> https://wiki.php.net/rfc/constructor_promotion#attributes), but with an
> explicit note that we should change the behavior prior to the PHP 8 release
> if it turns out to be problematic, e.g. with regards to an attribute
> validation feature. I think this is something of a detail, and we'll be
> mostly fine whichever way we chose, but it's hard to say right now which
> option is best, in particular with regard to future attributes improvements.
>
> Regards,
> Nikita
Thinking on it further, there's something sitting in the back of my head that's
bothering me. This helped me flesh it out.
I like promotion, and think it will be great for simplifying code. However, as
attributes show as we load more bells and whistles onto properties (even if
they are desirable and useful bells and whistles) the level of conflict is
going to get larger.
Consider, today we have:
class Point {
protected int $x;
protected int $y;
public function __construct(int $x, int $y) {
$this->x = $x;
$this->y = $y;
}
}
And this is needlessly verbose. With promotion, that simplifies to:
class Point {
public function __construct(protected int $x, protected int $y) {}
}
And this is good. There seems little dispute about that.
But then we add attributes, and it starts to get a little odd:
class Point {
public function __construct(
<>
protected int $x,
<>
protected int $y,
) {}
}
And has the possible confusion between parameter attributes and property
attributes.
But... there's plenty of other things that may get added to properties. We've
discussed the "readonly" property and the more robust assymetric visibility.
What would that look like?
class Point {
public function __construct(
<>
int $x {
public get,
private set,
},
<>
int $y {
public get,
private set,
},
) {}
}
And that's now an awful lot of metadata to shove into, technically, a function
signature. And if accessor methods ever happen:
class Point {
public function __construct(
<>
int $x {
public get () {
return $this->x;
},
private set($new) {
if ($new < 0) {
throw new Exception();
}
$this->x = $new;
},
},
<>
int $y {
public get () {
return $this->y;
},
private set($new) {
if ($new < 0) {
throw new Exception();
}
$this->y = $new;
},
},
) {}
}
And now we have methods defined inside a function signature, and that's just
bizzarro. It also means at least 4 indent levels before you even get to the
body of the methods.
And there's probably other things that could get attached to properties at some
point I'm not thinking of that someone will suggest.
So I have to wonder, are we painting ourselves into a hole? Most of the uses I
can think of for extra-metadata-properties are... overlapping with the case
where you want to have constructor promotion. That means "well if you need to
do fancy stuff just don't use promotion" becomes a non-sustainable answer,
because the cases where you want both will increase rather than be an edge case.
Is there a way we can build in an escape hatch for ourselves to avoid these
issues, both attributes and for future extension?
First idea off my head, which may or may not be good:
Allow an alternate keyword to fill in the body of the constructor, but NOT fill
in the property. You still define the property yourself and put all the extra
metadata there. To wit:
class Point {
<>
int $x {
public get () {
return $this->x;
},
private set($new) {
if ($new < 0) {
throw new Exception();
}
$this->x = $new;
},
},
<>
int $y {
public get () {
return $this->y;
},
private set($new) {
if ($new < 0) {
throw new Exception();
}
$this->y = $new;
},
},
public function __construct(promote int $x, promote int $y) {}
}
That eliminates 2 of the 4 places you'd have to repeat the name instead of 3,
but means the constructor isn't burdened with having to hold a potentially
non-trivial amount of metadata. It also resolves the attribute question as
splitting it up like this would let you do "one or the other" (property vs
parameter).
That may not be the best way, but I'm slightly uneasy with a path that could
lead to 30 lines of property metadata squeezed into a constructor signature
definition. :-)
Thoughts?
--Larry
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Wed, Apr 29, 2020 at 11:27 AM Benjamin Eberlei wrote: > > > On Wed, Apr 29, 2020 at 11:07 AM Nikita Popov > wrote: > >> On Wed, Apr 29, 2020 at 10:56 AM Benjamin Eberlei >> wrote: >> >>> >>> >>> On Wed, Apr 29, 2020 at 9:47 AM Nicolas Grekas < >>> nicolas.grekas+...@gmail.com> wrote: >>> > I think it might be best to apply to "both" and provide an isPromoted() > method on both ReflectionParameter and ReflectionProperty. Any code that > wishes to validate the allowed positions of an attribute can then skip > properties/parameters that report isPromoted() as true, to avoid reporting > false positives. > That sounds good. Deal on my side. >>> >>> Just to mention, any approach here potentially conflicts with anything >>> we consider for potential target validation on attributes, i.e. declaring >>> for an attribute that it is only allowed on a property OR an argument. >>> >>> At the point constructor promotion happens, we can also not look into >>> the attribute to see if its target=property or target=parameter, because >>> this would require triggering autoloader. >>> >> >> Does it really conflict though? Can't the target validation just ignore >> invalid attributes when promotion is involved (or rather, only check that >> the attribute is valid for either property or parameter, but not >> necessarily both of them)? >> > > Since target validation would happen on ReflectionAttribute::newInstance > alongside other validation already proposed, this means there is already > the ReflectionAttribute instantiated, so we could only avoid throwing an > exception, but the code might still get an attribute returned that doesn't > belong there. The technical problem here is that we defer the validation to > newInstance(), and not already during getAttributes(), to avoid > autoloading. It looks like we can either have target validation and have to > move validation to Reflection*::getAttributes(), or we can't have the > validation. > > The same problem would essentially appear with a "repeatable=true/false" > feature that prevents the same attribute from being declared multiple > times. Its validation should also better be done at > Reflection*::getAttributes(). > > Maybe to allow for access to attributes without validation we should > instead have > `Reflection*::getAttributes(ReflectionAttribute::FLAGS_NO_VALIDATION)` and > don't defer validation to newInstance() by default? > Performing validation when the getAttributes() call is performed does sound reasonable to me. We can also add a class flag to perform this validation only once (if it is successful), so the cost doesn't have to be paid by every single getAttributes() consumer. For the purpose of this RFC, I've now updated it to say that attributes will be applied to both properties and parameters ( https://wiki.php.net/rfc/constructor_promotion#attributes), but with an explicit note that we should change the behavior prior to the PHP 8 release if it turns out to be problematic, e.g. with regards to an attribute validation feature. I think this is something of a detail, and we'll be mostly fine whichever way we chose, but it's hard to say right now which option is best, in particular with regard to future attributes improvements. Regards, Nikita
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Wed, Apr 29, 2020 at 11:07 AM Nikita Popov wrote: > On Wed, Apr 29, 2020 at 10:56 AM Benjamin Eberlei > wrote: > >> >> >> On Wed, Apr 29, 2020 at 9:47 AM Nicolas Grekas < >> nicolas.grekas+...@gmail.com> wrote: >> >>> > I think it might be best to apply to "both" and provide an isPromoted() >>> > method on both ReflectionParameter and ReflectionProperty. Any code >>> that >>> > wishes to validate the allowed positions of an attribute can then skip >>> > properties/parameters that report isPromoted() as true, to avoid >>> reporting >>> > false positives. >>> > >>> >>> That sounds good. Deal on my side. >>> >> >> Just to mention, any approach here potentially conflicts with anything we >> consider for potential target validation on attributes, i.e. declaring for >> an attribute that it is only allowed on a property OR an argument. >> >> At the point constructor promotion happens, we can also not look into the >> attribute to see if its target=property or target=parameter, because this >> would require triggering autoloader. >> > > Does it really conflict though? Can't the target validation just ignore > invalid attributes when promotion is involved (or rather, only check that > the attribute is valid for either property or parameter, but not > necessarily both of them)? > Since target validation would happen on ReflectionAttribute::newInstance alongside other validation already proposed, this means there is already the ReflectionAttribute instantiated, so we could only avoid throwing an exception, but the code might still get an attribute returned that doesn't belong there. The technical problem here is that we defer the validation to newInstance(), and not already during getAttributes(), to avoid autoloading. It looks like we can either have target validation and have to move validation to Reflection*::getAttributes(), or we can't have the validation. The same problem would essentially appear with a "repeatable=true/false" feature that prevents the same attribute from being declared multiple times. Its validation should also better be done at Reflection*::getAttributes(). Maybe to allow for access to attributes without validation we should instead have `Reflection*::getAttributes(ReflectionAttribute::FLAGS_NO_VALIDATION)` and don't defer validation to newInstance() by default? > > Nikita >
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Wed, Apr 29, 2020 at 10:56 AM Benjamin Eberlei wrote: > > > On Wed, Apr 29, 2020 at 9:47 AM Nicolas Grekas < > nicolas.grekas+...@gmail.com> wrote: > >> > I think it might be best to apply to "both" and provide an isPromoted() >> > method on both ReflectionParameter and ReflectionProperty. Any code that >> > wishes to validate the allowed positions of an attribute can then skip >> > properties/parameters that report isPromoted() as true, to avoid >> reporting >> > false positives. >> > >> >> That sounds good. Deal on my side. >> > > Just to mention, any approach here potentially conflicts with anything we > consider for potential target validation on attributes, i.e. declaring for > an attribute that it is only allowed on a property OR an argument. > > At the point constructor promotion happens, we can also not look into the > attribute to see if its target=property or target=parameter, because this > would require triggering autoloader. > Does it really conflict though? Can't the target validation just ignore invalid attributes when promotion is involved (or rather, only check that the attribute is valid for either property or parameter, but not necessarily both of them)? Nikita
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Wed, Apr 29, 2020 at 9:47 AM Nicolas Grekas wrote: > > I think it might be best to apply to "both" and provide an isPromoted() > > method on both ReflectionParameter and ReflectionProperty. Any code that > > wishes to validate the allowed positions of an attribute can then skip > > properties/parameters that report isPromoted() as true, to avoid > reporting > > false positives. > > > > That sounds good. Deal on my side. > Just to mention, any approach here potentially conflicts with anything we consider for potential target validation on attributes, i.e. declaring for an attribute that it is only allowed on a property OR an argument. At the point constructor promotion happens, we can also not look into the attribute to see if its target=property or target=parameter, because this would require triggering autoloader. > Nicolas >
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
> I think it might be best to apply to "both" and provide an isPromoted() > method on both ReflectionParameter and ReflectionProperty. Any code that > wishes to validate the allowed positions of an attribute can then skip > properties/parameters that report isPromoted() as true, to avoid reporting > false positives. > That sounds good. Deal on my side. Nicolas
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Wed, Apr 29, 2020 at 2:05 AM Larry Garfield wrote: > On Tue, Apr 28, 2020, at 10:56 AM, Nicolas Grekas wrote: > > > > > > > I would expect attributes to apply only to the properties on my side. > > > > > > Parameter annotations could be interesting for dependency injection. > > > Symfony currently has some DI magic through parameter names among > > > other things. It might be nice to control these through annotations > > > instead. This would be impossible or involve a lot of hackery and > > > assumptions if the annotations aren't available on the parameters. > > > Alternatively we might make this information available through > > > reflection (like ReflectionParameter::getGeneratedProperty()) so you > > > can look for the annotation there. > > > > > > > Sure, this could be considered. > > > > But this doesn't contradict what I wrote: > > if one uses constructor promotion, the then attributes should only go to > > the properties to me. > > If one wants to add attributes to parameters, then one would need to > > opt-out from constructor promotion. > > > > Nicolas > > I'm inclined to agree with Nicolas. My gut feeling (and I don't have more > data to back it up than that, I admit) is that property annotations will be > more prevalent than parameter annotations, so those should take precedence. > > That said... how many will conflict? In practice, the most common use > case I can see for parameter attributes would be fancy DI configuration for > services... and I don't know that I'd care about attributes on the > properties in that case. Conversely, for properties that have attributes > that will usually be for extended type information, ORM mapping, and stuff > like that. For which... I can't think of what I'd want attributes I'd want > on the parameter itself. > > Perhaps I'm just not creative enough at this hour, but I'm not sure that > "both" is such a problem. It only seems like it would be an issue for > attributes that user-space wanted to enforce on certain cases only; since > attributes are new, a viable answer there is "btw, if you're restricting > your annotation be aware of this double-case and either allow it or don't, > but it's on you to decide what if anything to do." > That's basically my line of thinking as well. I do agree that as things are right now, most likely a property annotation is intended. But that's potentially only an artifact of phpdoc annotations not supporting parameter annotations, so there is no existing ecosystem around them. I think it might be best to apply to "both" and provide an isPromoted() method on both ReflectionParameter and ReflectionProperty. Any code that wishes to validate the allowed positions of an attribute can then skip properties/parameters that report isPromoted() as true, to avoid reporting false positives. Nikita
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Tue, Apr 28, 2020, at 10:56 AM, Nicolas Grekas wrote: > > > > > I would expect attributes to apply only to the properties on my side. > > > > Parameter annotations could be interesting for dependency injection. > > Symfony currently has some DI magic through parameter names among > > other things. It might be nice to control these through annotations > > instead. This would be impossible or involve a lot of hackery and > > assumptions if the annotations aren't available on the parameters. > > Alternatively we might make this information available through > > reflection (like ReflectionParameter::getGeneratedProperty()) so you > > can look for the annotation there. > > > > Sure, this could be considered. > > But this doesn't contradict what I wrote: > if one uses constructor promotion, the then attributes should only go to > the properties to me. > If one wants to add attributes to parameters, then one would need to > opt-out from constructor promotion. > > Nicolas I'm inclined to agree with Nicolas. My gut feeling (and I don't have more data to back it up than that, I admit) is that property annotations will be more prevalent than parameter annotations, so those should take precedence. That said... how many will conflict? In practice, the most common use case I can see for parameter attributes would be fancy DI configuration for services... and I don't know that I'd care about attributes on the properties in that case. Conversely, for properties that have attributes that will usually be for extended type information, ORM mapping, and stuff like that. For which... I can't think of what I'd want attributes I'd want on the parameter itself. Perhaps I'm just not creative enough at this hour, but I'm not sure that "both" is such a problem. It only seems like it would be an issue for attributes that user-space wanted to enforce on certain cases only; since attributes are new, a viable answer there is "btw, if you're restricting your annotation be aware of this double-case and either allow it or don't, but it's on you to decide what if anything to do." --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
> > > I would expect attributes to apply only to the properties on my side. > > Parameter annotations could be interesting for dependency injection. > Symfony currently has some DI magic through parameter names among > other things. It might be nice to control these through annotations > instead. This would be impossible or involve a lot of hackery and > assumptions if the annotations aren't available on the parameters. > Alternatively we might make this information available through > reflection (like ReflectionParameter::getGeneratedProperty()) so you > can look for the annotation there. > Sure, this could be considered. But this doesn't contradict what I wrote: if one uses constructor promotion, the then attributes should only go to the properties to me. If one wants to add attributes to parameters, then one would need to opt-out from constructor promotion. Nicolas
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
Hi Nicolas > I would expect attributes to apply only to the properties on my side. Parameter annotations could be interesting for dependency injection. Symfony currently has some DI magic through parameter names among other things. It might be nice to control these through annotations instead. This would be impossible or involve a lot of hackery and assumptions if the annotations aren't available on the parameters. Alternatively we might make this information available through reflection (like ReflectionParameter::getGeneratedProperty()) so you can look for the annotation there. I don't think this is a huge problem, just wanted to mention it. Ilija -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
> On Thu, Mar 26, 2020 at 2:30 PM Nikita Popov wrote: > > > Hi internals, > > > > I would like to submit the following RFC for your consideration: > > https://wiki.php.net/rfc/constructor_promotion > > > > This is based on one off the suggestions made in > > https://externals.io/message/109220, and some existing discussion on the > > topic can be found in that thread. > > > > The primary motivation for this feature is to close the currently rather > > wide gap between the use of ad-hoc array structures, and the use of > > well-defined and type-safe value objects. The latter is what we want > people > > to use, but the former is, unfortunately, what is *easy* to use. > > > > As it looks like the Attributes RFC is going to be accepted, we need to > consider how it will interact with this proposal. I've added an extra > section for this: > https://wiki.php.net/rfc/constructor_promotion#attributes > > The problem is that attributes are allowed both on properties and on > parameters. For promoted properties, what does the attribute apply to? My > original suggestion was to make it apply to both the parameter and the > generated property. However, Benjamin pointed out that an attribute on a > promoted property is almost certainly intended to apply to the property > only, especially because parameter attributes have not been supported > historically (in phpdoc). Applying it to both the property and the > parameter may cause issues for attributes that want to strictly validate > where they are used. > > I think the best answer to this question may be to forbid the use of > attributes on promoted properties entirely, because there is no unambiguous > interpretation for them. I also think that using attributes pushes this > "syntax sugar" to its limit, as you can easily end up with a 50 line > constructor signature that way, at least if I'm going by some of the more > involved uses of annotations... > This last proposal would kinda defeat constructor promotion and all the reasons why it was introduced in the first place. I would expect attributes to apply only to the properties on my side. If people want different or duplicate attributes on parameters, then it's normal to not be able to use constructor promotion. My 2cts, Nicolas
Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
On Tue, Apr 14, 2020 at 9:17 PM Dmitry Stogov wrote: > Hi Nikita, > > Personally, I don't see a big reason in introduction this syntax sugar. > It allows to make class declaration more compact but less readable. > > Also, this feature doesn't work well with inheritance. RFC doesn't provide > any examples with inherited classes. > The implementation doesn't allow initialization of parent class > properties, and doesn't allow to call parent constructor before > initialization of child class properties (like C++ allows). > I have now added an example involving inheritance to the RFC: https://wiki.php.net/rfc/constructor_promotion#inheritance Constructor promotion does work with inheritance. As you pointed out, the only oddity is that properties are assigned before the parent constructor call, rather than after it. I don't think this makes a difference in practice though. > In my opinion, the existing PHP syntax is already clear and good enough. > Fair enough :) Nikita -- > *From:* Nikita Popov > *Sent:* Tuesday, April 14, 2020 16:02 > *To:* PHP internals > *Subject:* [PHP-DEV] Re: [RFC] Constructor Property Promotion > > On Thu, Mar 26, 2020 at 2:30 PM Nikita Popov wrote: > > > Hi internals, > > > > I would like to submit the following RFC for your consideration: > > https://wiki.php.net/rfc/constructor_promotion > > > > This is based on one off the suggestions made in > > https://externals.io/message/109220, and some existing discussion on the > > topic can be found in that thread. > > > > The primary motivation for this feature is to close the currently rather > > wide gap between the use of ad-hoc array structures, and the use of > > well-defined and type-safe value objects. The latter is what we want > people > > to use, but the former is, unfortunately, what is *easy* to use. > > > > As the reception has been positive, I plan to open voting on this proposal > soon. Please tell me if you have further concerns or the RFC needs > additional clarification. > > Regards, > Nikita > > > CAUTION: This email originated from outside of the organization. Do not > click on links or open attachments unless you recognize the sender and know > the content is safe. > > This e-mail may contain information that is privileged or confidential. If > you are not the intended recipient, please delete the e-mail and any > attachments and notify us immediately. > >
