[RFC] Constructor Property Promotion

Paul M. Jones

6 years ago

> On Mar 26, 2020, at 08:30, Nikita Popov <nikita.ppv@gmail.com> wrote:
>
> Hi internals,
>
> I would like to submit the following RFC for your consideration:
> https://wiki.php.net/rfc/constructor_promotion

Big fan. Thanks for this.

--
Paul M. Jones
pmjones@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

Chase Peeler

6 years ago

On Thu, Mar 26, 2020 at 9:33 AM Paul M. Jones <pmjones@pmjones.io> wrote:

>
>
> > On Mar 26, 2020, at 08:30, Nikita Popov <nikita.ppv@gmail.com> wrote:
> >
> > Hi internals,
> >
> > I would like to submit the following RFC for your consideration:
> > https://wiki.php.net/rfc/constructor_promotion
>
> Big fan. Thanks for this.
>
>

I like this too. One question (and apologies if I overlooked this in the
RFC), but is type-hinting required, or would the following be accepted:

class bar {
public function __construct(public $foo){}
}

>
> --
> Paul M. Jones
> pmjones@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
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

--
Chase Peeler
chasepeeler@gmail.com

Sebastian Bergmann

6 years ago

Am 26.03.2020 um 14:30 schrieb Nikita Popov:

> I would like to submit the following RFC for your consideration:
> https://wiki.php.net/rfc/constructor_promotion

Looks good to me! Thanks.

David Rodrigues

6 years ago

Hello!

With all respect, seems a bit confuses to me, without in fact offer a new
feature, just a shortcut. I hope it's me seeing it the wrong way.

What happen if I rename the constructor parameters on a child class
constructor?

Example (https://3v4l.org/VqQde):

class A {
public function __construct(public int $x) { ... }
}

class B extends A {
public function __construct(public int $y) { ...;
parent::__construct($y); }
}


Atenciosamente,
David Rodrigues


Em qui., 26 de mar. de 2020 às 12:45, Sebastian Bergmann <sebastian@php.net>
escreveu:

Nikita Popov

6 years ago

On Thu, Mar 26, 2020 at 5:02 PM David Rodrigues <david.proweb@gmail.com>
wrote:

> Hello!
>
> With all respect, seems a bit confuses to me, without in fact offer a new
> feature, just a shortcut. I hope it's me seeing it the wrong way.
>
> What happen if I rename the constructor parameters on a child class
> constructor?
>
> Example (https://3v4l.org/VqQde):
>
> class A {
>     public function __construct(public int $x) { ... }
> }
>
> class B extends A {
>     public function __construct(public int $y) { ...;
> parent::__construct($y); }
> }
>

Constructors (in general, independent of this feature) are per-class and do
not participate in classical signature-based inheritance (the exception
being abstract constructors, but those aren't relevant here). The notion of
"renaming" a constructor parameter doesn't make sense in that regard,
because the parameters of the child constructor have absolutely no relation
to the parameters of the parent constructor.

The way to write your example would be:

class A {
public function __construct(public int $x) {}
}

class B extends A {
public function __construct(int $x) {
parent::__construct($x);
// Your extra logic here.
}
}

Regards,
Nikita

Sara Golemon

6 years ago

On Thu, Mar 26, 2020 at 8:31 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> 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.
>
> Similar to an RFC you submitted six years ago:

https://wiki.php.net/rfc/automatic_property_initialization
I think the availability of typed properties makes the syntax for this much
more sensible and I look forward to +1ing it.

-Sara

Michał Brzuchalski

6 years ago

czw., 26 mar 2020 o 14:31 Nikita Popov <nikita.ppv@gmail.com> napisał(a):

> 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.
>
>

As I already said in linked topic. IMHO this is wrong and can be easily
overused.

A list of properties is changed into a list of constructor method arguments
which
can be easily overused on long argument lists and when the time comes to
have attributes then the list of promoted arguments with their annotations
and some extra blank lines then to improve readability could lead to
long constructor signature list on a different level of indentation which
also then always has to be un-collapsed in IDE and on the top of a class to
keep the track of properties where usually list of properties exists with
additional named constructors (always at the top).

Not even mentioning that mix of promoted and un-promoted argument list
would just feel awkward and that kind of mixed constructor where some of
arguments are assigned magically and some of them not can make it harder to
read.

And I'm not saying this cause I'm preparing another proposal for filling
the gap
between the use of ad-hoc array structures, and the use of well-defined and
type-safe value types.
I'm saying this because PHP was always verbose and easy to read/decipher
this solution IMHO has more drawbacks regarding readability than benefits.

Cheers,

--
Michał Brzuchalski

Ben Ramsey

6 years ago

> On Mar 26, 2020, at 08:30, Nikita Popov <nikita.ppv@gmail.com> 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.

Will this cause any problems with ReflectionClass? (I’m assuming not, but wanted to ask anyway.)

Cheers,
Ben

Stas Malyshev

6 years ago

Hi!

> I would like to submit the following RFC for your consideration:
> https://wiki.php.net/rfc/constructor_promotion

I like this shortcut. I am not sure though I understand why callable is
not allowed? Can't it just create a non-typed property?

--
Stas Malyshev
smalyshev@gmail.com

Markus Fischer

6 years ago

Hi,
On 2020-03-26 14:30, Nikita Popov wrote:

> 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.

The amount of boilerplate this would cut down for e.g. projects using DI
and DTOs is unfathomable.

How does this work together with reflection? Will reflection see the
properties _and_ the extended constructor definitions?

thanks,
- Markus

Nikita Popov

6 years ago

On Thu, Apr 2, 2020 at 1:35 AM Markus Fischer <markus@fischer.name> wrote:

> Hi,
> On 2020-03-26 14:30, Nikita Popov wrote:
> > 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.
>
> The amount of boilerplate this would cut down for e.g. projects using DI
> and DTOs is unfathomable.
>
> How does this work together with reflection? Will reflection see the
> properties _and_ the extended constructor definitions?
>

Reflection will see the state after desugaring. That is, it just sees
normal properties and normal constructor args. I've made this more explicit
in the RFC now: https://wiki.php.net/rfc/constructor_promotion#reflection

Nikita

Markus Fischer

6 years ago

Him

On 02.04.20 12:07, Nikita Popov wrote:

> Reflection will see the state after desugaring. That is, it just sees
> normal properties and normal constructor args. I've made this more explicit
> in the RFC now: https://wiki.php.net/rfc/constructor_promotion#reflection

Thanks!

Another one, also related a bit to your PHP-Parser (?): will the
tokenization process "see" the code before desugaring?

I'm wondering how automated translation tool can detect that a certain
code is not "sugared" and promote this?

thanks!
- Markus

Nikita Popov

6 years ago

On Thu, Apr 2, 2020 at 3:47 PM Markus Fischer <markus@fischer.name> wrote:

> Him
>
> On 02.04.20 12:07, Nikita Popov wrote:
> > Reflection will see the state after desugaring. That is, it just sees
> > normal properties and normal constructor args. I've made this more
> explicit
> > in the RFC now:
> https://wiki.php.net/rfc/constructor_promotion#reflection
>
> Thanks!
>
> Another one, also related a bit to your PHP-Parser (?): will the
> tokenization process "see" the code before desugaring?
>
> I'm wondering how automated translation tool can detect that a certain
> code is not "sugared" and promote this?
>

Tokenization and parsing both happen before the desugaring, they see the
code in its original form. They would detect promoted properties by
checking whether a constructor parameter has a visibility modifier.  Of
course, tooling might want to perform the desugaring transformation
internally, so it sees a uniform representation of everything.

Regards,
Nikita

Mike Schinkel

6 years ago

Question: Will parse errors in the __construct() be reported as all being on the same line, or will the parser report each as a distinct line of code when reporting parse errors?

-Mike

Nicolas Grekas

6 years ago

> I would like to submit the following RFC for your consideration:
> https://wiki.php.net/rfc/constructor_promotion
>

I love it :)

Just one question:
Shouldn't default values be copied on the signature? That would be expected
for me.
(and that would make the properties still initialized when child classes
don't call the parent constructor for whatever reasons.)

class Point {
public function __construct(
public float $x = 0.0,

<==>

class Point {
public float $x = 0.0;

public function __construct(
float $x = 0.0,

Nicolas

Nikita Popov

6 years ago

On Wed, Apr 8, 2020 at 1:56 PM Nicolas Grekas <nicolas.grekas+php@gmail.com>
wrote:

>
> I would like to submit the following RFC for your consideration:
>> https://wiki.php.net/rfc/constructor_promotion
>>
>
> I love it :)
>
> Just one question:
> Shouldn't default values be copied on the signature? That would be
> expected for me.
> (and that would make the properties still initialized when child classes
> don't call the parent constructor for whatever reasons.)
>
> class Point {
>     public function __construct(
>         public float $x = 0.0,
>
> <==>
>
> class Point {
>     public float $x = 0.0;
>
>     public function __construct(
>         float $x = 0.0,
>
> Nicolas
>

Some reasons why the default value is not duplicated into the property are
discussed in https://wiki.php.net/rfc/constructor_promotion#desugaring.
It's primarily about forward-compatibility concerns.

But apart from that, I strongly believe that you should not, ever, specify
a default value on a property that is initialized in the constructor. You
can have a default, or you can have explicit initialization in the
constructor -- both together do not make sense.

Regards,
Nikita

Nikita Popov

6 years ago

On Thu, Mar 26, 2020 at 2:30 PM Nikita Popov <nikita.ppv@gmail.com> 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

Nikita Popov

6 years ago

On Thu, Mar 26, 2020 at 2:30 PM Nikita Popov <nikita.ppv@gmail.com> 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...

Does anyone have thoughts on this?

Regards,
Nikita

Nicolas Grekas

6 years ago

> On Thu, Mar 26, 2020 at 2:30 PM Nikita Popov <nikita.ppv@gmail.com> 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

Ilija Tovilo

6 years ago

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

Nicolas Grekas

6 years ago

>
> > 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

Larry Garfield

6 years ago

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

Nikita Popov

6 years ago

On Wed, Apr 29, 2020 at 2:05 AM Larry Garfield <larry@garfieldtech.com>
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

Nicolas Grekas

6 years ago

> 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

Benjamin Eberlei

6 years ago

On Wed, Apr 29, 2020 at 9:47 AM Nicolas Grekas <nicolas.grekas+php@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.

Nikita Popov

6 years ago

On Wed, Apr 29, 2020 at 10:56 AM Benjamin Eberlei <kontakt@beberlei.de>
wrote:

>
>
> On Wed, Apr 29, 2020 at 9:47 AM Nicolas Grekas <
> nicolas.grekas+php@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

Andrew Faulds

6 years ago

Hi Nikita,

Nikita Popov wrote:

> On Thu, Mar 26, 2020 at 2:30 PM Nikita Popov <nikita.ppv@gmail.com> wrote:
> > 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...

I think that is a great starting point because it is a safe decision: if
we later decide there is a better behaviour, we can change it without
causing any problems for existing code. So, if we don't come up with a
better idea, I would strongly recommend that one. I also agree with your
other thoughts on it. :)

Thanks,
Andrea

Mike Schinkel

6 years ago

> On Mar 26, 2020, at 9:30 AM, Nikita Popov <nikita.ppv@gmail.com> 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.
>
> Regards,
> Nikita

I am rather concerned about constructor promotion will result in some complex "single" lines of code that all must be syntactically correct for any of it to be syntactically correct.  It will make it more challenging to read and write code as we add attributes and PHP doc to properties.  And if we still have to deal with concern that parameter names that were previously never indented to become part of the API will be exposed as part of the API, with potential breakage when name changes.

Pondering these issues I think I have a very simple proposal that would address most (all?) of the concerns I am aware for both Constructor Property Promotion and Named Parameters.

Consider simply what we might call "Parameter Blocks."  Since PHP always expects a parentheses to follow the function or method name it should be possible to opt-in replace it with a brace-enclosed block of parameters instead since it would be unambiguous and this no conflict, and no need for new keywords.

The following illustrates how it my be used by repurposing an example from the RFC:

abstract class Node {
public function __construct{
/**
* @param Location|null — The starting location for this abstract node
*/
Location $startLoc = null;

protected Location $startLoc = null;

/**
* @param Location|null — The ending location for this abstract node
*/
protected Location $endLoc = null;
} {}
}

class ParamNode extends Node {
public function __construct{
/**
* @param string $name — Names this Node.
*/
public string $name;

/**
* @param ExprNode|null $default — Contains the expression this node represents
*/
<<ORM\Entity("exprNode")>>
public ExprNode $default = null;

/**
* @param TypeNode|null $type — Contains the type expected for this node's expression
*/
<<ORM\Entity("typeNode")>>
public TypeNode $type = null;

/**
* @param bool $byRef — True if this node is a reference, false otherwise
*/
public bool $byRef = false;

/**
* @param bool $byRef — True if this node is variadic, false otherwise
*/
public bool $variadic = false;

/**
* @param Location|null — The starting location for this node
*/
Location $startLoc = null;

/**
* @param Location|null — The ending location for this node
*/
Location $endLoc = null;
} {
parent::__construct($startLoc, $endLoc);
}
}

Envision how that same code would have to be written if all as part of the single "line" contained within the parenthesis of a programming language?

As far as I can tell this is a clean and elegant approach with many fewer downsides than those that have been discussed thus far.  I hope that each of you reading this will seriously consider this is viable alternative option.

-Mike

Rowan Collins

6 years ago

Hi Mike,

On 06/05/2020 20:48, Mike Schinkel wrote:

> Consider simply what we might call "Parameter Blocks."  Since PHP always expects a parentheses to follow the function or method name it should be possible to opt-in replace it with a brace-enclosed block of parameters instead since it would be unambiguous and this no conflict, and no need for new keywords.
>
> The following illustrates how it my be used by repurposing an example from the RFC:
> ...

I'm not really clear what this example is showing. It seems to be the
same as the one in Nikita's RFC, but with the punctuation changed
slightly from "(public $foo, public $bar)" to "{public $foo; public $bar;}"

I'm not sure how this changes anything, or how it relates to named
parameters. Could you expand on the problems you see this solving?

Regards,

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

Mike Schinkel

6 years ago

> On May 6, 2020, at 4:14 PM, Rowan Tommins <rowan.collins@gmail.com> wrote:
>
> Hi Mike,
>
> On 06/05/2020 20:48, Mike Schinkel wrote:
>> Consider simply what we might call "Parameter Blocks."  Since PHP always expects a parentheses to follow the function or method name it should be possible to opt-in replace it with a brace-enclosed block of parameters instead since it would be unambiguous and this no conflict, and no need for new keywords.
>>
>> The following illustrates how it my be used by repurposing an example from the RFC:
>> ...
>
>
> I'm not really clear what this example is showing. It seems to be the same as the one in Nikita's RFC, but with the punctuation changed slightly from "(public $foo, public $bar)" to "{public $foo; public $bar;}"

That is exactly the case.

> I'm not sure how this changes anything, or how it relates to named parameters. Could you expand on the problems you see this solving?

Let's look at it like this, with line numbers that would be reported in the case of syntax error:

1:  function foo(public $foo, public $bar) {}

vs.

1: function foo {
2:	public $foo;
3: 	public $bar;
4: } {}

--------------

Now, consider the following two examples. On which line would PHP report the error?

function foo() (
public $foo,
public $ bar
) {}

vs.

function foo {
public $foo;
public $ bar;
} {}

With two parameters, no attributes and no PHP doc the error is pretty easy to spot.

-----------------

Now consider a constructor w/20 properties/parameters, 5 attributes across those properties, and a total of 100 lines of PHP Doc interspersed and it becomes a lot more difficult to deal with if it is one virtual line — as PHP sees it — than spread across 125 lines.

Much harder to diagnose a syntax error.

-----------------

It is for the same reason I avoid code that instantiates a single array literal that spans hundreds of line of code and instead create methods to append small array literals to the array.  If you have one really long virtual line it makes it much harder to identify where syntax errors occurred.

Having one long virtual line also limits what features can be added in future versions of PHP.

If you need help visualizing it I mocked up an example from actual function that is in product that was written by someone else before I joined the client's current team:

https://gist.github.com/mikeschinkel/faaee3fc8ccc2371c7200f3d634289c4 <https://gist.github.com/mikeschinkel/faaee3fc8ccc2371c7200f3d634289c4>

-Mike

Rowan Collins

6 years ago

On Wed, 6 May 2020 at 22:48, Mike Schinkel <mike@newclarity.net> wrote:

> Now, consider the following two examples. On which line would PHP report
> the error?
>
> function foo() (
>
> public $foo,
>
> public $ bar
>
> ) {}
>
>
> vs.
>
> function foo {
> public $foo;
>   public $ bar;
> } {}
>
>

This feels like an implementation detail of the parser, rather than an
automatic consequence of the syntax, and the current parser seems to handle
it just fine.

<?php
function foo(
$foo,
$ bar
) {}
# unexpected '$', expecting variable... on line 4
# https://3v4l.org/AC9kh

The same is true of arrays: both syntax and run-time errors are reported
with correct line numbers, e.g. https://3v4l.org/fMp4t and
https://3v4l.org/BXO8q

> If you need help visualizing it I mocked up an example from actual
> function that is in product that was written by someone else before I
> joined the client's current team:
>
> https://gist.github.com/mikeschinkel/faaee3fc8ccc2371c7200f3d634289c4
>

Here's the single-declaration version of that with a syntax error on line
163: https://3v4l.org/mGq2f

I think you're solving a non-existent problem here.

Regards,

--
Rowan Tommins
[IMSoP]

Mike Schinkel

6 years ago

> On May 7, 2020, at 4:47 AM, Rowan Tommins <rowan.collins@gmail.com> wrote:
> the current parser seems to handle it just fine.

Interesting. You are correct on this point. I was going by memory that we had lots of problems with nested arrays, I should have tested this one assertion before posting.

> I think you're solving a non-existent problem here.

True, but that was only one of the several reasons I proposed the atlernate syntax, if you'll reread my original post you'll see the next point:

1. The proposal on the table for Named Parameters effectively converts all parameters names into public aspects of the their function's API.  Some suggested allowing aliases, allowing OUT-OUT, but AFAIK there have been no suggestion for how to OPT-OUT of named parameters entirely.

IOW, with the proposed syntax Named Parameters would infect every function/method across all of userland PHP code, whether the developer wants to maintain it as part of their API or not.  This would be like PHP 8.0 making all private variable publicly accessible.

This alternate syntax would make Named Parameters OPT-IN allowing developers to actively CHOOSE them — or not — as part of their public API.

The rest were not part of my first email as I was trying to keep it shorter, but I now see my brevity was not effective as I had hoped, so...

2. Adding the ability to fully parse all features of properties and parameters into two different contexts each with their own syntax variants will add ongoing maintenance burden of core (or at least it seems apparent that it would.)

3. The alternate block syntax would simplify userland refactoring to move properties to parameters and back when compared to the RFC for CPP. Mostly (all?) it would require would be copying and pasting of lines. The RFC syntax would require at least a minimum amount of refactoring (semicolons to commas and back.)

4. Who knows what syntax contortions will be required for parameters as we try to apply new language features that affect properties?  IOW, the current RFC is a less safe for future compatibility than the alternate block syntax.

5. New features for properties will likely take more discussion time to determine acceptable alternate syntaxes for using inside the parenthesis and then more time to implement, possibly delaying universally desired features or even nipping them in the bud.

6. Userland tooling would need to support both syntaxes, requiring more work for those project's maintainers, probably delay updates, and certainly requiring more ongoing maintenance.

7. Userland developers will be confused by and have to learn two slightly different syntaxes for essentially the same (type of) thing.

8. I believe this alternate syntax addresses Michal Bruzuchalski's and Larry Garfield's concerns, at least partially:

https://externals.io/message/109335#109343
https://externals.io/message/109335#110024

9. This alternate syntax would reduce the desugaring required; only the value assignment would be needed:

https://wiki.php.net/rfc/constructor_promotion#desugaring
https://externals.io/message/109335#109515

10. Nikita claims there is a "line" to be crossed when property declarations have their own blocks and would disallow their use with CPP.  And with the RFC's syntax that seems indeed an appropriate limitation. But with the alternate proposed syntax it would not need to be a limitation and I would argue that fewer limitations on future potential would be better.

https://externals.io/message/109335#110034

And finally:

11. The current Constructor Property Promotion adds complexity to the language where there is no analogue elsewhere PHP, and having things "be like existing PHP" is a frequent argument related to new features on this list.

PHP is primarily a line and block oriented language, and CPP purports to add Javascript-like complexity into a single statement. Frankly that is one of the reasons I so dislike programming in Javascript; I find the smaller distinct line-oriented statements of PHP much easier to reason about and would think many userland developers would also.

The alternate syntax fits in with PHP's existing line- and block-oriented flavor much better the RFC's syntax. Simplicity (should) win(s).

-Mike

Rowan Collins

6 years ago

On 08/05/2020 02:32, Mike Schinkel wrote:

> 1. The proposal on the table for Named Parameters effectively converts
> all parameters names into public aspects of the their function's API.

Yes, an opt-in version of named parameters might be useful. However, I'm
not immediately convinced it should share any syntax with property
promotion; the only real connection is that we're discussing them at the
same time.

> 2. Adding the ability to fully parse all features of properties and parameters into two different contexts each with their own syntax variants will add ongoing maintenance burden of core (or at least it seems apparent that it would.)
> 4. Who knows what syntax contortions will be required ...
> 5. New features for properties will likely take more discussion time to determine acceptable alternate syntaxes ...

The only reason I can think there would need to be variations in the
syntax would be if there was some new syntax for properties which used
commas in a way that would be ambiguous in parameter lists. Otherwise,
parsing a promoted parameter is exactly the same as parsing a
non-promoted one, but you stop at "," rather than ";".

> 3. The alternate block syntax would simplify userland refactoring to move properties to parameters and back...
> 11. The current Constructor Property Promotion adds complexity to the language where there is no analogue elsewhere PHP

By its nature, this proposed feature is merging two existing syntaxes.
The current syntax proposal looks mostly like the constructor's existing
parameter list; yours looks mostly like the existing property
declarations; I'm not sure either is automatically better.

Switching to or from promoted properties will always require changing
one or both parts of the class - the largest edit will be deleting or
re-creating the body of the constructor.

> 6. Userland tooling would need to support both syntaxes...
> 7. Userland developers will be confused by and have to learn two slightly different syntaxes for essentially the same (type of) thing.

These points are both true no matter what the syntax looks like: it will
be a new syntax, for a new feature, and tools and programmers will have
to adapt to that. Glancing at the implementation on the RFC, the changes
to the actual PHP parser actually seem fairly small.

> 8. I believe this alternate syntax addresses Michal Bruzuchalski's and Larry Garfield's concerns...
> 10. Nikita claims there is a "line" to be crossed when property declarations have their own blocks and would disallow their use with CPP...

I hesitate to speak for them, but I think their concerns are more
conceptual: that this requires nesting a large portion of the class
definition inside the constructor definition. Changing the syntax
doesn't really change that - the "parameter block" would still be an
extra level of nesting, attached to the constructor definition, rather
than at the top level of the class, where it lives today.

> 9. This alternate syntax would reduce the desugaring required

The compiler won't need to transform the actual source code; the
"de-sugaring" is just a representation of the _equivalent_ code if
written manually.

> PHP is primarily a line and block oriented language, and CPP purports to add Javascript-like complexity into a single statement.

I'm not sure what is "Javascript-like" about the proposed syntax, or why
using semicolons rather than commas makes it less so.


Regards,

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

Larry Garfield

6 years ago

On Fri, May 8, 2020, at 1:04 PM, Rowan Tommins wrote:

> On 08/05/2020 02:32, Mike Schinkel wrote:

> > 8. I believe this alternate syntax addresses Michal Bruzuchalski's and Larry Garfield's concerns...
> > 10. Nikita claims there is a "line" to be crossed when property declarations have their own blocks and would disallow their use with CPP...
>
>
> I hesitate to speak for them, but I think their concerns are more
> conceptual: that this requires nesting a large portion of the class
> definition inside the constructor definition. Changing the syntax
> doesn't really change that - the "parameter block" would still be an
> extra level of nesting, attached to the constructor definition, rather
> than at the top level of the class, where it lives today.

Correct.  My concern isn't with comma vs semicolon.  My concern is that the property definition is then living between `public function __construct(` and `) {`.

When the property definition is basic (visibility, type, and something like readonly) that's still reasonable enough for the benefit it brings.

When the property definition becomes involved enough that it is preferable to have it span lines (docblock, attributes, or something like property accessors or potentially even just asymmetric visibility), that's when I get fidgety, because then you're technically writing

public function __construct(/** @see blah.html */ <<Positive>> protected int $count, /** @see blah.html */ <<Positive>> <<LessThan(99)>> protected int $age,) {

}

Just with convenience newlines.  Semi-colons and curly braces wouldn't in themselves address that.

What *might* would be putting promotable properties in an entirely separate construct from the constructor.

An off the cuff riff from that might be something like:

class Stuff {

promoted {

/**
* @see blah.html
*/
<<Positive>>
protected int $count;

/**
* @see blah.html
*/
<<Positive>>
<<LessThan(99)>>
protected int $age;
}
}

That moves them out of the constructor entirely, and might overlap with the desire for named parameters.

$p = new Stuff(5, 30);
// or
$p = new Stuff{count: 5, age: 30};

However, that has the major drawback of then not being in the constructor, so promoting only some constructor arguments becomes impossible.  It's also worse on the BC front.

I don't know if that's a good idea or not; as I said, just an off-the-cuff riff on what would address the "complex properties" question better.  For the simple case, the current RFC syntax seems fine.  (Maybe someone can riff off of this to something better.)

--Larry Garfield

Dan Ackroyd

6 years ago

On Wed, 6 May 2020 at 20:49, Mike Schinkel <mike@newclarity.net> wrote:

>
> I am rather concerned about

Mike,

It's nice that you want to contribute to PHP. But trying to brainstorm
ideas over email is not productive.

Spending your time helping get the manual more up-to-date would be far
more beneficial than throwing out "Hey why don't you try this"
comments.

> a standard library of PHP-written functionality bundled in a PHAR file that would be included with future implementations of PHP and automatically included by PHP.

Anyone who has worked with Java or Go would know that having core
functionality separate from the executable would bring in a whole new
set of error conditions related to classpath or gopath. And
additionally version mismatch problems between the library found, and
the language executable being invoked.

Additionally, while both strings and arrays in PHP work, how they are
implemented leaves something to be desired. For strings, it's the
behaviour around character sets/unicode. For arrays, it's a 'having a
single data structure for all possible needs' problem.

Spending time on a 'standard library' before there have been
improvements to strings and/or arrays is unlikely to produce anything
of great value.

As I said, if you have energy that can be used to make PHP better, I
think helping with the manual would be useful
https://github.com/php/doc-en Or if you can, getting up to speed with
C to work on bugs would also be good https://bugs.php.net/ But just
throwing out ideas is not a productive use of your time and distracts
people from more useful conversations.

cheers
Dan
Ack

Rowan Collins

6 years ago

On 08/05/2020 14:44, Dan Ackroyd wrote:

> Mike,
> It's nice that you want to contribute to PHP. But trying to brainstorm
> ideas over email is not productive.

Hi Dan,

I appreciate that you, too, want to make PHP better, as a language and
as a project; but just about everything in this e-mail is wrong.

Firstly, you've managed to reply to the wrong thread.

Secondly, you've once again taken it on yourself to proclaim when people
should and shouldn't use this e-mail list, without actually asking
anyone else's opinion.

Thirdly, Mike is not the only one "brainstorming" this particular idea,
as your comments about "spending your time better elsewhere" imply. Ben
Ramsey, Larry Garfield, and Dik Takken have all expressed support for
the general idea, and it has come up in other discussions as a potential
benefit of preloading and JIT.

I certainly don't agree with all Mike's ideas, nor do I always feel the
mailing list is as productive as it could be, but I think this message
was just way off base.

Regards,

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

Nikita Popov

6 years ago

On Thu, Mar 26, 2020 at 2:30 PM Nikita Popov <nikita.ppv@gmail.com> 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.
>
> Regards,
> Nikita
>

I know I said it a few times already, but ... I plan to start voting on
this soon. There has been some additional discussion recently on
alternative syntax/approaches, such as Mike's parameter blocks, and Larry's
"promoted" keyword. I wasn't really convinced by either of those, so I plan
to go forward with the proposal as is.

Nikita

Dan Ackroyd

6 years ago

On Mon, 11 May 2020 at 10:52, Nikita Popov <nikita.ppv@gmail.com> 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

Nikita Popov

6 years ago

On Mon, May 11, 2020 at 5:26 PM Dan Ackroyd <Danack@basereality.com> wrote:

> On Mon, 11 May 2020 at 10:52, Nikita Popov <nikita.ppv@gmail.com> 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