Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123083
Precedence: bulk
MIME-Version: 1.0
References: <CAPyj-LBMQEJ=2BF=xk_on4O2xTU+MM2XRAZ5g=uPwVOf=bh+sw@mail.gmail.com>
 <66154AA0.1040905@adviesenzo.nl> <66160A1D.4060409@adviesenzo.nl>
In-Reply-To: <66160A1D.4060409@adviesenzo.nl>
Date: Wed, 10 Apr 2024 10:25:01 +0200
Message-ID: <CANS-=pcG+ZwyzLaadCTdxd3+JUgyGmw086U33N8yFUjsieJptw@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC][Vote announcement] Property hooks
To: Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="000000000000e4e90e0615b9c7db"
From: kjarli@gmail.com (Lynn)

--000000000000e4e90e0615b9c7db
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Apr 10, 2024 at 5:47=E2=80=AFAM Juliette Reinders Folmer <
php-internals_nospam@adviesenzo.nl> wrote:

> On 9-4-2024 16:03, Juliette Reinders Folmer wrote:
>
> On 8-4-2024 23:39, Ilija Tovilo wrote:
>
> Hi everyone
>
> Heads-up: Larry and I would like to start the vote of the property
> hooks RFC tomorrow:https://wiki.php.net/rfc/property-hooks
>
> We have worked long and hard on this RFC, and hope that we have found
> some middle-ground that works for the majority. One last concern we
> have not officially clarified on the list:
> https://externals.io/message/122445#122667
>
> I personally do not feel strongly about whether asymmetric types make it =
into the initial implementation. Larry does, however, and I think it is not=
 fair to exclude them without providing any concrete reasons not to. [snip]
>
> My concern is more about the external impact of what is effectively a cha=
nge to the type system of the language: [snip] will tools like PhpStan and =
Psalm require complex changes to analyse code using such properties?
>
> In particular, this paragraph is referencing the ability to widen the
> accepted $value parameter type of the set hook, described at the
> bottom of https://wiki.php.net/rfc/property-hooks#set. I have talked
> to Ond=C5=99ej Mirtes, the maintainer of PHPStan, and he confirmed that
> this should not be complex to implement in PHPStan. In fact, PHPStan
> already offers the @property-read and @property-write class
> annotations, which can be used to describe "virtual" properties
> handled within __get/__set, already providing asymmetric types of
> sorts. Hence, this concern should be a non-issue.
>
> Thank you to everybody who has contributed to the discussion!
>
> Ilija
>
>
>
> Ilija,
>
> Heads-up: I'm still writing up an opinion and intend to send it to the
> list before end of day (CET). I know I'm late to the party, but I've been
> having trouble finding the words to express myself properly regarding thi=
s
> RFC (and have been struggling to find the right words for months).
>
> Smile,
> Juliette
>
>
> Later than intended, but here goes....
>
> If there is one RFC which has been giving me nightmares since I first
> heard of it, it's this one.
>
> I realize it is late in the discussion period to speak up, but for months
> I've been trying to find the words to express my concerns in a polite and
> constructive way and have failed.
>
> I am going to try now anyway (before it is too late), so please bear with
> me. Also, as I'm not a C-developer, please forgive me if I get the
> internals wrong. I'm writing this from a PHP-dev/user perspective, with m=
y
> perspective being heavily influenced by my role as maintainer of
> PHP_CodeSniffer.
>
> ---
> TL;DR: this RFC tries to do too much in one go and introduces a huge
> amount of cognitive complexity with all the exceptions and the difference=
s
> in behaviour between virtual and backed properties. This cognitive
> complexity is so high that I expect that the feature will catch most
> developers out a lot of the time.
> ---
>
> I can definitely see the use case and desirability of the property hooks
> functionality proposed in the RFC and compared to the initial RFC I read
> last year, the current RFC is, IMO, much improved.
> Huge kudos to Ilija and Larry for all the work they have put in to this!
>
> I applaud the intention of this RFC to make it easier to avoid the magic
> __get()/__set() et al methods. What I have a problem with is the
> implementation details.
>
> Over the last few years, we've seen a movement to get rid of more and mor=
e
> of the _surprising_ behaviour of PHP, with subtle exceptions being
> deprecated and slated for removal and (most) new syntaxes trying to use t=
he
> principle of least surprise by design.
>
> This RFC, in my view, is in stark contrast to this as it introduces a
> plethora of exceptions and subtle different behaviour in a way that will
> catch developers out for years to come.
>
> At this moment (excluding property hooks), from a user perspective, there
> are three different function syntaxes in PHP: named functions (and
> methods), anonymous functions and arrow functions.
>
> The semantics of these are very similar with subtle differences:
> * Can be static or non-static.
> * Take parameters, which can be typed, references, variadic, optional etc=
.
> * Can have a return type.
> * Can return by reference.
> * Have a function "body".
>
> The differences between the current syntaxes - from a user perspective -
> are as follows:
> =3D Named functions:
> * When declared in a class, can have visibility attached, can be abstract=
,
> can be final.
> * When declared in an interface or declared as abstract, will not have a
> function "body".
>
> =3D Anonymous functions:
> * Can import plain variables from outside its scope with a `use()` clause=
.
> * Are declared as an expression (can be assigned to a variable etc).
>
> =3D Arrow functions:
> * Have access to variables in the same scope.
> * Are declared as an expression.
> * Body of the function starts with a =3D> instead of being enclosed in
> curlies and can end on a range of characters.
> * Can only take one statement in the body.
> * Automagically returns.
>
> The property hooks RFC introduces a fourth flavour of function syntax. An=
d
> not just one syntax, but five and the differences in the semantics of the
> function syntaxes are significant.
>
> The differences in semantics I see for "full" property hook functions
> compared to other function syntaxes are as follows:
> * `get` cannot take parameters (and doesn't have the parentheses typicall=
y
> expected for a function declaration).
> * `get` cannot take return type (inherits this from the property, which i=
s
> logically sound, but does create a syntactic difference).
> * `set` can take one (typed or untyped) parameter.
> * `set` cannot take return type (silently set to void, which is logically
> sound, but does create a syntactic difference).
> * `set` magically creates a $value variable for the "new" value, though
> that variable _may_ have an arbitrary different name depending on whether
> or not the parameter was explicitly specified.
> * Has a function body.
>
> Then there are multiple short hand syntaxes, which each have yet more
> syntactic differences:
> * The arrow function variant for `get` with all the above differences +
> the differences inherent to arrow functions, combined, with the exception
> of the access to variables in the same scope and a more clearly defined e=
nd
> of the expression.
> * The implicit "set" parameter, which does not have a explicit parameter,
> does not have parentheses and has the magically created $value variable.
> * The arrow function variant for `set` with all the above differences +
> the differences inherent to arrow functions with the above mentioned
> exceptions + the implicit assignment, which breaks the expected behaviour
> of arrow functions by assigning the result of the expression instead of
> returning it (totally understandable, but still a difference).
> * The abstract/interface variants, which don't have a function body.
>
> Next there are the differences in semantics which are now being introduce=
d
> for properties:
> * Aside from the hooks themselves....
> * Properties _may_ or _may not_ have a default value anymore, depending o=
n
> whether the hooks cause it to be a backed or a virtual property.
> * Properties with hooks can _be_ readonly, but cannot be declared as
> readonly.
>
> And then there are the new features for properties (not just hooked ones)=
:
> * Properties can now be declared as final.
> * Properties can now be declared as abstract, but only with explicit hook
> requirements, otherwise the abstract keyword is not allowed.
> * Properties can now be declared on interfaces, but only with explicit
> hook requirements, otherwise they are not allowed.
> * Abstract/Interface properties don't comply with the same
> covariance/contravariance rules as other properties
>

This complexity describes what I've been unconsciously worried about though
I really like having the proposed feature in PHP. I'm not sure if there's a
solution to reduce the complexity to a point where glancing at something
will tell you how it works when you don't know exactly how all variants
work. There are already a lot of gotches with something as simple as adding
a `string|null` to a property without setting its default value to `null`.
It's one of those things you won't find out until you run that code because
technically speaking it's not incorrect. I worry that a bunch of those
gotchas will bite me in the behind because I don't fully understand how it
works and mistakes are easily made. It also adds a lot of complexity when
reviewing someone else's code where bugs might slip through.


> And last but not least, there is the ability to declare property hooks in
> constructor property promotion ... where we can now basically have a
> function declaration within the signature of a method declaration.... wit=
h
> all five possible syntax types.
>

This is what I've been afraid of. Constructor method signatures are already
exploding and I hope that one day we'll revise this notation and head
towards `__construct($this->theProperty)` instead. One could argue that
property promotion is fully optional, except that in its most basic design
it does save a lot of lines of code and is a net-gain in terms of
maintainability and readability in its minimum form for properly written
code. The benefits fall apart the moment you don't have static code
analysis available, or if the class is big/old and it just takes time for
the tool to be done. If I forget to add the visibility keyword in the
constructor I have to rely on my IDE telling me that the argument is
unused, which results in extra overhead of trying to find out why it's
telling me it's unused, just to realize I forgot to add `public` or
`private`.

Some of my worries would be removed if hooks are not allowed in property
promotion scenarios, but this doesn't solve the complexity mentioned for
the hooks themselves.

--000000000000e4e90e0615b9c7db
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Wed, Apr 10, 2024 at 5:47=E2=80=AF=
AM Juliette Reinders Folmer &lt;<a href=3D"mailto:php-internals_nospam@advi=
esenzo.nl">php-internals_nospam@adviesenzo.nl</a>&gt; wrote:<br></div><bloc=
kquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:=
1px solid rgb(204,204,204);padding-left:1ex">
 =20
   =20
 =20
  <div bgcolor=3D"#FFFFFF">
    On 9-4-2024 16:03, Juliette Reinders Folmer wrote:<br>
    <blockquote type=3D"cite">
     =20
      <div>On 8-4-2024 23:39, Ilija Tovilo
        wrote:<br>
      </div>
      <blockquote type=3D"cite">
        <pre>Hi everyone

Heads-up: Larry and I would like to start the vote of the property
hooks RFC tomorrow:
<a href=3D"https://wiki.php.net/rfc/property-hooks" target=3D"_blank">https=
://wiki.php.net/rfc/property-hooks</a>

We have worked long and hard on this RFC, and hope that we have found
some middle-ground that works for the majority. One last concern we
have not officially clarified on the list:

<a href=3D"https://externals.io/message/122445#122667" target=3D"_blank">ht=
tps://externals.io/message/122445#122667</a>

</pre>
        <blockquote type=3D"cite">
          <blockquote type=3D"cite">
            <pre>I personally do not feel strongly about whether asymmetric=
 types make it into the initial implementation. Larry does, however, and I =
think it is not fair to exclude them without providing any concrete reasons=
 not to. [snip]
</pre>
          </blockquote>
          <pre>My concern is more about the external impact of what is effe=
ctively a change to the type system of the language: [snip] will tools like=
 PhpStan and Psalm require complex changes to analyse code using such prope=
rties?
</pre>
        </blockquote>
        <pre>In particular, this paragraph is referencing the ability to wi=
den the
accepted $value parameter type of the set hook, described at the
bottom of <a href=3D"https://wiki.php.net/rfc/property-hooks#set" target=3D=
"_blank">https://wiki.php.net/rfc/property-hooks#set</a>. I have talked
to Ond=C5=99ej Mirtes, the maintainer of PHPStan, and he confirmed that
this should not be complex to implement in PHPStan. In fact, PHPStan
already offers the @property-read and @property-write class
annotations, which can be used to describe &quot;virtual&quot; properties
handled within __get/__set, already providing asymmetric types of
sorts. Hence, this concern should be a non-issue.

Thank you to everybody who has contributed to the discussion!

Ilija

</pre>
      </blockquote>
      <br>
      Ilija,<br>
      <br>
      Heads-up: I&#39;m still writing up an opinion and intend to send it t=
o
      the list before end of day (CET). I know I&#39;m late to the party,
      but I&#39;ve been having trouble finding the words to express myself
      properly regarding this RFC (and have been struggling to find the
      right words for months).<br>
      <br>
      Smile,<br>
      Juliette<br>
    </blockquote>
    <br>
    Later than intended, but here goes.... <br>
    <br>
    If there is one RFC which has been giving me nightmares since I
    first heard of it, it&#39;s this one.<br>
    <br>
    I realize it is late in the discussion period to speak up, but for
    months I&#39;ve been trying to find the words to express my concerns in
    a polite and constructive way and have failed.<br>
    <br>
    I am going to try now anyway (before it is too late), so please bear
    with me. Also, as I&#39;m not a C-developer, please forgive me if I get
    the internals wrong. I&#39;m writing this from a PHP-dev/user
    perspective, with my perspective being heavily influenced by my role
    as maintainer of PHP_CodeSniffer.<br>
    <br>
    ---<br>
    TL;DR: this RFC tries to do too much in one go and introduces a huge
    amount of cognitive complexity with all the exceptions and the
    differences in behaviour between virtual and backed properties. This
    cognitive complexity is so high that I expect that the feature will
    catch most developers out a lot of the time.<br>
    ---<br>
    <br>
    I can definitely see the use case and desirability of the property
    hooks functionality proposed in the RFC and compared to the initial
    RFC I read last year, the current RFC is, IMO, much improved.<br>
    Huge kudos to Ilija and Larry for all the work they have put in to
    this!<br>
    <br>
    I applaud the intention of this RFC to make it easier to avoid the
    magic __get()/__set() et al methods. What I have a problem with is
    the implementation details.<br>
    <br>
    Over the last few years, we&#39;ve seen a movement to get rid of more
    and more of the _surprising_ behaviour of PHP, with subtle
    exceptions being deprecated and slated for removal and (most) new
    syntaxes trying to use the principle of least surprise by design.<br>
    <br>
    This RFC, in my view, is in stark contrast to this as it introduces
    a plethora of exceptions and subtle different behaviour in a way
    that will catch developers out for years to come.<br>
    <br>
    At this moment (excluding property hooks), from a user perspective,
    there are three different function syntaxes in PHP: named functions
    (and methods), anonymous functions and arrow functions.<br>
    <br>
    The semantics of these are very similar with subtle differences:<br>
    * Can be static or non-static.<br>
    * Take parameters, which can be typed, references, variadic,
    optional etc.<br>
    * Can have a return type.<br>
    * Can return by reference.<br>
    * Have a function &quot;body&quot;.<br>
    <br>
    The differences between the current syntaxes - from a user
    perspective - are as follows:<br>
    =3D Named functions:<br>
    * When declared in a class, can have visibility attached, can be
    abstract, can be final.<br>
    * When declared in an interface or declared as abstract, will not
    have a function &quot;body&quot;.<br>
    <br>
    =3D Anonymous functions:<br>
    * Can import plain variables from outside its scope with a `use()`
    clause.<br>
    * Are declared as an expression (can be assigned to a variable etc).<br=
>
    <br>
    =3D Arrow functions:<br>
    * Have access to variables in the same scope.<br>
    * Are declared as an expression.<br>
    * Body of the function starts with a =3D&gt; instead of being enclosed
    in curlies and can end on a range of characters.<br>
    * Can only take one statement in the body.<br>
    * Automagically returns.<br>
    <br>
    The property hooks RFC introduces a fourth flavour of function
    syntax. And not just one syntax, but five and the differences in the
    semantics of the function syntaxes are significant.<br>
    <br>
    The differences in semantics I see for &quot;full&quot; property hook
    functions compared to other function syntaxes are as follows:<br>
    * `get` cannot take parameters (and doesn&#39;t have the parentheses
    typically expected for a function declaration).<br>
    * `get` cannot take return type (inherits this from the property,
    which is logically sound, but does create a syntactic difference).<br>
    * `set` can take one (typed or untyped) parameter.<br>
    * `set` cannot take return type (silently set to void, which is
    logically sound, but does create a syntactic difference).<br>
    * `set` magically creates a $value variable for the &quot;new&quot; val=
ue,
    though that variable _may_ have an arbitrary different name
    depending on whether or not the parameter was explicitly specified.<br>
    * Has a function body.<br>
    <br>
    Then there are multiple short hand syntaxes, which each have yet
    more syntactic differences:<br>
    * The arrow function variant for `get` with all the above
    differences + the differences inherent to arrow functions, combined,
    with the exception of the access to variables in the same scope and
    a more clearly defined end of the expression.<br>
    * The implicit &quot;set&quot; parameter, which does not have a explici=
t
    parameter, does not have parentheses and has the magically created
    $value variable.<br>
    * The arrow function variant for `set` with all the above
    differences + the differences inherent to arrow functions with the
    above mentioned exceptions + the implicit assignment, which breaks
    the expected behaviour of arrow functions by assigning the result of
    the expression instead of returning it (totally understandable, but
    still a difference).<br>
    * The abstract/interface variants, which don&#39;t have a function body=
.<br>
    <br>
    Next there are the differences in semantics which are now being
    introduced for properties:<br>
    * Aside from the hooks themselves....<br>
    * Properties _may_ or _may not_ have a default value anymore,
    depending on whether the hooks cause it to be a backed or a virtual
    property.<br>
    * Properties with hooks can _be_ readonly, but cannot be declared as
    readonly.<br>
    <br>
    And then there are the new features for properties (not just hooked
    ones):<br>
    * Properties can now be declared as final.<br>
    * Properties can now be declared as abstract, but only with explicit
    hook requirements, otherwise the abstract keyword is not allowed.<br>
    * Properties can now be declared on interfaces, but only with
    explicit hook requirements, otherwise they are not allowed.<br>
    * Abstract/Interface properties don&#39;t comply with the same
    covariance/contravariance rules as other properties<br></div></blockquo=
te><div><br></div><div>This complexity describes what I&#39;ve been unconsc=
iously worried about though I really like having the proposed feature in PH=
P. I&#39;m not sure if there&#39;s a solution to reduce the complexity to a=
 point where glancing at something will tell you how it works when you don&=
#39;t know exactly how all variants work. There are already a lot of gotche=
s with something as simple as adding a `string|null` to a property without =
setting its default value to `null`. It&#39;s one of those things you won&#=
39;t find out until you run that code because technically speaking it&#39;s=
 not incorrect. I worry that a bunch of those gotchas will bite me in the b=
ehind because I don&#39;t fully understand how it works and mistakes are ea=
sily=C2=A0made. It also adds a lot of complexity when reviewing someone els=
e&#39;s code where bugs=C2=A0might slip through.</div><div>=C2=A0</div><blo=
ckquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left=
:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor=3D"#FFFFFF">
    And last but not least, there is the ability to declare property
    hooks in constructor property promotion ... where we can now
    basically have a function declaration within the signature of a
    method declaration.... with all five possible syntax types.<br></div></=
blockquote><div><br></div><div>This is what I&#39;ve been afraid of. Constr=
uctor method signatures are already exploding and I hope that one day we=
9;ll revise this notation and head towards `__construct($this-&gt;theProper=
ty)` instead. One could argue that property promotion is fully optional, ex=
cept that in its most basic design it does save a lot of lines of code and =
is a net-gain in terms of maintainability and readability in its=C2=A0minim=
um form for properly=C2=A0written code. The benefits fall apart the=C2=A0mo=
ment you don&#39;t have static code analysis available,=C2=A0or if the clas=
s is big/old and it just takes time for the tool to be done. If I forget to=
 add the visibility keyword in the constructor I have to rely on my IDE tel=
ling me that the argument is unused, which results in extra overhead of try=
ing to find out why it&#39;s telling me it&#39;s unused, just to realize I =
forgot to add `public` or `private`.</div><div><br></div><div>Some of my wo=
rries would be removed if hooks are not allowed in property promotion scena=
rios, but this doesn&#39;t solve the complexity mentioned for the hooks the=
mselves.</div></div></div>

--000000000000e4e90e0615b9c7db--