Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110024
User-Agent: Cyrus-JMAP/3.3.0-dev0-351-g9981f4f-fmstable-20200421v1
Mime-Version: 1.0
Message-ID: <8dc53b46-8156-4b62-a521-d6d6fd379281@www.fastmail.com>
In-Reply-To: 
 <CAF+90c-vC4h034Gwr2pjbOgtkMiSkMJ-=jVs_6hEodjWZRrvYQ@mail.gmail.com>
References: 
 <CAF+90c8tW27-rhrk8pLieMsEWa4WHEBciPX1zThME68zf6s2QQ@mail.gmail.com>
 <CAF+90c_8CEBr90C_Y5Ze9uPL=aMis8Lzh2vKX4=Ac_y7ioQMcw@mail.gmail.com>
 <CAOWwgpntdhGd-avcJrO-X3Yiq=WG5uf5jrpH-VXQda_fGip05w@mail.gmail.com>
 <CAPyj-LBPsXzRzWp3Mn26pTJc=XJyWsnD_TK1J5h8mnu-b6TPSw@mail.gmail.com>
 <CAOWwgpndYuRYs8EgBxWUwktmCrdSVuHxBiT4-i+vYUCdKGUdPw@mail.gmail.com>
 <76b72148-4b3a-4a63-ab4c-245439a94823@www.fastmail.com>
 <CAF+90c9ikBbVT2AfwmnhsRF3wWegk8j8O11WUxRv0o2F3SoPSQ@mail.gmail.com>
 <CAOWwgpnj022Ukg5nFgAu=9PupKaqwykvDLttr+6KXAn1-8o6QA@mail.gmail.com>
 <CAEPJdumGSjeLKeq-wk8o6OUFd6e6jbPGeDSf4Hit6tssKQpSkw@mail.gmail.com>
 <CAF+90c9wUhMLFjo8EO1pOZvOUqO+aM1KsK4FdBFLg9NfbGzQrQ@mail.gmail.com>
 <CAEPJdumiLURwhAe6eVeXTYeo+u6hbd=tKeS8z9Dndu6C8vFTPg@mail.gmail.com>
 <CAF+90c-vC4h034Gwr2pjbOgtkMiSkMJ-=jVs_6hEodjWZRrvYQ@mail.gmail.com>
Date: Tue, 05 May 2020 15:27:09 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain
Subject: Re: [PHP-DEV] Re: [RFC] Constructor Property Promotion
From: larry@garfieldtech.com ("Larry Garfield")

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(
    <<Positive>>
    protected int $x, 

    <<Positive>>
    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(
    <<Positive>>
    int $x {
      public get,
      private set,
    }, 

    <<Positive>>
    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(
    <<Positive>>
    int $x {
      public get () {
        return $this->x;
      },
      private set($new) {
        if ($new < 0) {
          throw new Exception();
        }
        $this->x = $new;
      },
    }, 

    <<Positive>>
    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 {
  <<Positive>>
  int $x {
    public get () {
      return $this->x;
    },
    private set($new) {
      if ($new < 0) {
        throw new Exception();
      }
      $this->x = $new;
    },
  }, 

  <<Positive>>
  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 Garfield