[RFC] [DISCUSSION] Make constructors and destructors return void

  110612
June 16, 2020 23:10 benas.molis.iml@gmail.com (Benas IML)
Hey internals,

This is a completely refined, follow-up RFC to my original RFC. Based on the
feedback I have received, this PR implements full validation and implicitly
enforces `void` rules on constructors/destructors while also allowing to
declare an **optional** explicit `void` return type. Note, that there is a
small but justifiable BC break (as stated by the RFC).

RFC: https://wiki.php.net/rfc/make_ctor_ret_void

Best regards,
Benas Seliuginas
  110613
June 17, 2020 00:44 drealecs@gmail.com (=?UTF-8?Q?Alexandru_P=C4=83tr=C4=83nescu?=)
Hi Benas,

This looks good to me.

Can you do a research on top 1k-2k most used composer packages and verify
how small the BC break it is?

Also, a suggestion on how to fix the BC issue if it exists could be
mentioned I guess.

Thank you,
Aled

On Wed, Jun 17, 2020, 02:11 Benas IML iml@gmail.com> wrote:

> Hey internals, > > This is a completely refined, follow-up RFC to my original RFC. Based on > the > feedback I have received, this PR implements full validation and implicitly > enforces `void` rules on constructors/destructors while also allowing to > declare an **optional** explicit `void` return type. Note, that there is a > small but justifiable BC break (as stated by the RFC). > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void > > Best regards, > Benas Seliuginas > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
  110667
June 18, 2020 16:43 benas.molis.iml@gmail.com (Benas IML)
Hey Alexandru,

Your email was marked as spam for me, thus this is a late response,
sorry about that!

Nikita analyzed top 2,000 Composer packages and found that 95 of them will
have a BC break. As a solution, to minimize the amount of BC breaks, I'm
proposing to have a deprecation warning in PHP 8.0 and subsequently enforce
`void` rules in later versions.

Best regards,
Benas

On Wed, Jun 17, 2020, 3:44 AM Alexandru Pătrănescu <drealecs@gmail.com>
wrote:

> Hi Benas, > > This looks good to me. > > Can you do a research on top 1k-2k most used composer packages and verify > how small the BC break it is? > > Also, a suggestion on how to fix the BC issue if it exists could be > mentioned I guess. > > Thank you, > Aled > > On Wed, Jun 17, 2020, 02:11 Benas IML iml@gmail.com> wrote: > >> Hey internals, >> >> This is a completely refined, follow-up RFC to my original RFC. Based on >> the >> feedback I have received, this PR implements full validation and >> implicitly >> enforces `void` rules on constructors/destructors while also allowing to >> declare an **optional** explicit `void` return type. Note, that there is a >> small but justifiable BC break (as stated by the RFC). >> >> RFC: https://wiki.php.net/rfc/make_ctor_ret_void >> >> Best regards, >> Benas Seliuginas >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: https://www.php.net/unsub.php >> >>
  110650
June 18, 2020 11:47 benas.molis.iml@gmail.com (Benas IML)
Hey internals,

1. Since there is confusion around multiple RFCs, I would like to emphasize
that this RFC (Make constructors and destructors return void) superseded
the original RFC (Allow void return type on constructors/destructors).
Thus, the latter RFC is now abandoned.

Link to the new RFC: https://wiki.php.net/rfc/make_ctor_ret_void
Link to the old RFC: https://wiki.php.net/rfc/constructor_return_type

I would also like to inform you that the new RFC is now complete. As such,
I have put the RFC into "Under Discussion" status.

2. Since this RFC is proposing a rather small BC break (given that there
will be a deprecation warning in PHP 8.0), should we start enforcing `void`
rules on constructors and destructors in PHP 8.1 or PHP 9.0?

Best regards,
Benas

On Wed, Jun 17, 2020, 2:10 AM Benas IML iml@gmail.com> wrote:

> Hey internals, > > This is a completely refined, follow-up RFC to my original RFC. Based on > the > feedback I have received, this PR implements full validation and implicitly > enforces `void` rules on constructors/destructors while also allowing to > declare an **optional** explicit `void` return type. Note, that there is a > small but justifiable BC break (as stated by the RFC). > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void > > Best regards, > Benas Seliuginas >
  110655
June 18, 2020 13:06 bobwei9@hotmail.com (Bob Weinand)
> Am 17.06.2020 um 01:10 schrieb Benas IML iml@gmail.com>: > > Hey internals, > > This is a completely refined, follow-up RFC to my original RFC. Based on the > feedback I have received, this PR implements full validation and implicitly > enforces `void` rules on constructors/destructors while also allowing to > declare an **optional** explicit `void` return type. Note, that there is a > small but justifiable BC break (as stated by the RFC). > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void > > Best regards, > Benas Seliuginas
Hey Benas, I do not see any particular benefit from that RFC. Regarding what the manual states - the manual is wrong there and thus should be fixed in the manual. This is not an argument for changing engine behaviour. Sometimes a constructor (esp. of a parent class) or destructor may be called manually. Sometimes you have valid information to pass from the parent class. With your RFC an arbitrary restriction is introduced necessitating an extra method instead. In general that RFC feels like "uh, __construct and __destruct are mostly void, so let's enforce it … because we can"? On these grounds and it being an additional (albeit mostly small) unnecessary BC break, I'm not in favor of that RFC. Bob
  110663
June 18, 2020 15:18 benas.molis.iml@gmail.com (Benas IML)
Hey Bob,

Magic methods are **never** supposed to be called directly (even more if
that method is a constructor or a destructor). If that's not the case, it's
just plain bad code. But by enforcing these rules, we make sure that less
of that (bad code) is written and as a result, we make PHP code less
bug-prone and easier to debug. That's also most likely the reason why
"ensure magic methods' signature" RFC opted in to validate `__clone`
method's signature and ensure that it has `void` return type.

Just for the sake of making sure that you understand what I mean, here are
a couple of examples that show that no magic method is ever supposed to be
called directly:
```php
// __toString
(string) $object;

// __invoke
$object();

// __serialize
serialize($object);
```

Moreover, by validating constructors/destructors and allowing an explicit
`void` return type declaration, we are becoming much more consistent
(something that PHP is striving for) with other magic methods (e. g.
`__clone`).

Also, saying that "sometimes you have valid information to pass from the
parent class" is quite an overstatement. After analyzing most of the 95
Composer packages that had a potential BC break, I found out that either
they wanted to return early (that is still possible to do using `return;`)
or they added a `return something;` for no reason. Thus, no libraries
actually returned something useful and valid from a constructor (as they
shouldn't).

Last but certainly not least, constructors have one and only one
responsibility - to initialize an object. Whether you read Wikipedia's or
PHP manual's definition, a constructor does just that. It initializes. So,
the PHP manual is perfectly correct and documents the correct return type
that a constructor should have.

Best regards,
Benas

On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobwei9@hotmail.com> wrote:

> > Am 17.06.2020 um 01:10 schrieb Benas IML iml@gmail.com>: > > > > Hey internals, > > > > This is a completely refined, follow-up RFC to my original RFC. Based on > the > > feedback I have received, this PR implements full validation and > implicitly > > enforces `void` rules on constructors/destructors while also allowing to > > declare an **optional** explicit `void` return type. Note, that there is > a > > small but justifiable BC break (as stated by the RFC). > > > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void > > > > Best regards, > > Benas Seliuginas > > Hey Benas, > > I do not see any particular benefit from that RFC. > > Regarding what the manual states - the manual is wrong there and thus > should be fixed in the manual. This is not an argument for changing engine > behaviour. > > Sometimes a constructor (esp. of a parent class) or destructor may be > called manually. Sometimes you have valid information to pass from the > parent class. > With your RFC an arbitrary restriction is introduced necessitating an > extra method instead. > > In general that RFC feels like "uh, __construct and __destruct are mostly > void, so let's enforce it … because we can"? > > On these grounds and it being an additional (albeit mostly small) > unnecessary BC break, I'm not in favor of that RFC. > > Bob
  110664
June 18, 2020 16:02 andreas@dqxtech.net (Andreas Hennings)
Hi

The RFC introduces what I call a "meaningless choice", by making something
possible that is currently illegal, but which does not change behavior.
https://3v4l.org/daeEm

It forces organisations, frameworks or the php-fig group to introduce yet
another coding convention to decide whether or not there should be a ":
void" declaration on constructors.

I am ok with restricting the use of "return *;" inside a constructor.
But I don't like allowing the ": void" declaration.

Greetings

On Thu, 18 Jun 2020 at 17:18, Benas IML iml@gmail.com> wrote:

> Hey Bob, > > Magic methods are **never** supposed to be called directly (even more if > that method is a constructor or a destructor). If that's not the case, it's > just plain bad code. But by enforcing these rules, we make sure that less > of that (bad code) is written and as a result, we make PHP code less > bug-prone and easier to debug. That's also most likely the reason why > "ensure magic methods' signature" RFC opted in to validate `__clone` > method's signature and ensure that it has `void` return type. > > Just for the sake of making sure that you understand what I mean, here are > a couple of examples that show that no magic method is ever supposed to be > called directly: > ```php > // __toString > (string) $object; > > // __invoke > $object(); > > // __serialize > serialize($object); > ``` > > Moreover, by validating constructors/destructors and allowing an explicit > `void` return type declaration, we are becoming much more consistent > (something that PHP is striving for) with other magic methods (e. g. > `__clone`). > > Also, saying that "sometimes you have valid information to pass from the > parent class" is quite an overstatement. After analyzing most of the 95 > Composer packages that had a potential BC break, I found out that either > they wanted to return early (that is still possible to do using `return;`) > or they added a `return something;` for no reason. Thus, no libraries > actually returned something useful and valid from a constructor (as they > shouldn't). > > Last but certainly not least, constructors have one and only one > responsibility - to initialize an object. Whether you read Wikipedia's or > PHP manual's definition, a constructor does just that. It initializes. So, > the PHP manual is perfectly correct and documents the correct return type > that a constructor should have. > > Best regards, > Benas > > On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobwei9@hotmail.com> wrote: > > > > Am 17.06.2020 um 01:10 schrieb Benas IML iml@gmail.com>: > > > > > > Hey internals, > > > > > > This is a completely refined, follow-up RFC to my original RFC. Based > on > > the > > > feedback I have received, this PR implements full validation and > > implicitly > > > enforces `void` rules on constructors/destructors while also allowing > to > > > declare an **optional** explicit `void` return type. Note, that there > is > > a > > > small but justifiable BC break (as stated by the RFC). > > > > > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void > > > > > > Best regards, > > > Benas Seliuginas > > > > Hey Benas, > > > > I do not see any particular benefit from that RFC. > > > > Regarding what the manual states - the manual is wrong there and thus > > should be fixed in the manual. This is not an argument for changing > engine > > behaviour. > > > > Sometimes a constructor (esp. of a parent class) or destructor may be > > called manually. Sometimes you have valid information to pass from the > > parent class. > > With your RFC an arbitrary restriction is introduced necessitating an > > extra method instead. > > > > In general that RFC feels like "uh, __construct and __destruct are mostly > > void, so let's enforce it … because we can"? > > > > On these grounds and it being an additional (albeit mostly small) > > unnecessary BC break, I'm not in favor of that RFC. > > > > Bob >
  110665
June 18, 2020 16:33 benas.molis.iml@gmail.com (Benas IML)
Hey Andreas,

That is incorrect. Adding an explicit `: void` does change behavior since
only then the check (whether something is being returned) is enforced. This
allows PHP 8 projects to take advantage of this enforcement while being
respective to older codebases.

And well, the explicit `: void` declaration also helps your code to be more
consistent with other methods ;)

Without an explicit `: void` return type declaration, `void` rules are not
enforced on constructors/destructors and will not be until PHP 9.0 (which
will probably release in 5 years).

Moreover, saying "it forces organisations, frameworks or the php-fig group
to introduce yet another coding convention" is a complete exaggeration. In
fact, even now there are no PSR conventions that specify how and when to
write parameter/return/property type hints.

Also, it's important to understand that PHP libraries are really really
slow at starting to **depend** on new PHP versions. It will probably take a
few good years (if not more) until first few libraries start requiring PHP
8.0. As a matter of fact, even Symfony framework is still requiring PHP
7.2.5 and cannot take advantage of its newer features (e. g. typed
properties).

Last but not least, just to reiterate, the `: void` return type is optional
and you don't need to specify it.

Best regards,
Benas

On Thu, Jun 18, 2020, 7:02 PM Andreas Hennings <andreas@dqxtech.net> wrote:

> Hi > > The RFC introduces what I call a "meaningless choice", by making something > possible that is currently illegal, but which does not change behavior. > https://3v4l.org/daeEm > > It forces organisations, frameworks or the php-fig group to introduce yet > another coding convention to decide whether or not there should be a ": > void" declaration on constructors. > > I am ok with restricting the use of "return *;" inside a constructor. > But I don't like allowing the ": void" declaration. > > Greetings > > On Thu, 18 Jun 2020 at 17:18, Benas IML iml@gmail.com> wrote: > >> Hey Bob, >> >> Magic methods are **never** supposed to be called directly (even more if >> that method is a constructor or a destructor). If that's not the case, >> it's >> just plain bad code. But by enforcing these rules, we make sure that less >> of that (bad code) is written and as a result, we make PHP code less >> bug-prone and easier to debug. That's also most likely the reason why >> "ensure magic methods' signature" RFC opted in to validate `__clone` >> method's signature and ensure that it has `void` return type. >> >> Just for the sake of making sure that you understand what I mean, here are >> a couple of examples that show that no magic method is ever supposed to be >> called directly: >> ```php >> // __toString >> (string) $object; >> >> // __invoke >> $object(); >> >> // __serialize >> serialize($object); >> ``` >> >> Moreover, by validating constructors/destructors and allowing an explicit >> `void` return type declaration, we are becoming much more consistent >> (something that PHP is striving for) with other magic methods (e. g. >> `__clone`). >> >> Also, saying that "sometimes you have valid information to pass from the >> parent class" is quite an overstatement. After analyzing most of the 95 >> Composer packages that had a potential BC break, I found out that either >> they wanted to return early (that is still possible to do using `return;`) >> or they added a `return something;` for no reason. Thus, no libraries >> actually returned something useful and valid from a constructor (as they >> shouldn't). >> >> Last but certainly not least, constructors have one and only one >> responsibility - to initialize an object. Whether you read Wikipedia's or >> PHP manual's definition, a constructor does just that. It initializes. So, >> the PHP manual is perfectly correct and documents the correct return type >> that a constructor should have. >> >> Best regards, >> Benas >> >> On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobwei9@hotmail.com> wrote: >> >> > > Am 17.06.2020 um 01:10 schrieb Benas IML iml@gmail.com>: >> > > >> > > Hey internals, >> > > >> > > This is a completely refined, follow-up RFC to my original RFC. Based >> on >> > the >> > > feedback I have received, this PR implements full validation and >> > implicitly >> > > enforces `void` rules on constructors/destructors while also allowing >> to >> > > declare an **optional** explicit `void` return type. Note, that there >> is >> > a >> > > small but justifiable BC break (as stated by the RFC). >> > > >> > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void >> > > >> > > Best regards, >> > > Benas Seliuginas >> > >> > Hey Benas, >> > >> > I do not see any particular benefit from that RFC. >> > >> > Regarding what the manual states - the manual is wrong there and thus >> > should be fixed in the manual. This is not an argument for changing >> engine >> > behaviour. >> > >> > Sometimes a constructor (esp. of a parent class) or destructor may be >> > called manually. Sometimes you have valid information to pass from the >> > parent class. >> > With your RFC an arbitrary restriction is introduced necessitating an >> > extra method instead. >> > >> > In general that RFC feels like "uh, __construct and __destruct are >> mostly >> > void, so let's enforce it … because we can"? >> > >> > On these grounds and it being an additional (albeit mostly small) >> > unnecessary BC break, I'm not in favor of that RFC. >> > >> > Bob >> >
  110666
June 18, 2020 16:43 bobwei9@hotmail.com (Bob Weinand)
Hey,

> Am 18.06.2020 um 17:18 schrieb Benas IML iml@gmail.com>: > > Hey Bob, > > Magic methods are **never** supposed to be called directly (even more if that method is a constructor or a destructor). If that's not the case, it's just plain bad code. But by enforcing these rules, we make sure that less of that (bad code) is written and as a result, we make PHP code less bug-prone and easier to debug. That's also most likely the reason why
__construct() is invoked directly on parent calls, sometimes to reinitialize an object or after ReflectionClass::newInstanceWithoutConstructor. I invoke __destruct() directly when needing an early freeing of existing resources.
> "ensure magic methods' signature" RFC opted in to validate `__clone` method's signature and ensure that it has `void` return type. > > Just for the sake of making sure that you understand what I mean, here are a couple of examples that show that no magic method is ever supposed to be called directly: > ```php > // __toString > (string) $object;
I like using ->__toString() in favor of (string) casts when the variable is guaranteed to be an object to highlight that and avoid magic-ness.
> // __invoke > $object();
Same here, unless the object is a closure.
> // __serialize > serialize($object); > ```
Can't argue much about that one, I never use serialize().
> Moreover, by validating constructors/destructors and allowing an explicit `void` return type declaration, we are becoming much more consistent (something that PHP is striving for) with other magic methods (e. g. `__clone`).
Yeah, __clone() is odd. No idea why.
> Also, saying that "sometimes you have valid information to pass from the parent class" is quite an overstatement. After analyzing most of the 95 Composer packages that had a potential BC break, I found out that either they wanted to return early (that is still possible to do using `return;`) or they added a `return something;` for no reason. Thus, no libraries actually returned something useful and valid from a constructor (as they shouldn't). > > Last but certainly not least, constructors have one and only one responsibility - to initialize an object. Whether you read Wikipedia's or PHP manual's definition, a constructor does just that. It initializes. So, the PHP manual is perfectly correct and documents the correct return type that a constructor should have.
It also is generally a bad idea to have side effects in constructors, but _sometimes_ it is justified. Only because something mostly is a bad idea, it is not always. Also note that other languages completely forbid manual ctor calls. But PHP doesn't (and for good reason, like after using ReflectionClass::newInstanceWithoutConstructor). Bob
> Best regards, > Benas > > On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobwei9@hotmail.com <mailto:bobwei9@hotmail.com>> wrote: > > Am 17.06.2020 um 01:10 schrieb Benas IML iml@gmail.com <mailto:benas.molis.iml@gmail.com>>: > > > > Hey internals, > > > > This is a completely refined, follow-up RFC to my original RFC. Based on the > > feedback I have received, this PR implements full validation and implicitly > > enforces `void` rules on constructors/destructors while also allowing to > > declare an **optional** explicit `void` return type. Note, that there is a > > small but justifiable BC break (as stated by the RFC). > > > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void <https://wiki.php.net/rfc/make_ctor_ret_void> > > > > Best regards, > > Benas Seliuginas > > Hey Benas, > > I do not see any particular benefit from that RFC. > > Regarding what the manual states - the manual is wrong there and thus should be fixed in the manual. This is not an argument for changing engine behaviour. > > Sometimes a constructor (esp. of a parent class) or destructor may be called manually. Sometimes you have valid information to pass from the parent class. > With your RFC an arbitrary restriction is introduced necessitating an extra method instead. > > In general that RFC feels like "uh, __construct and __destruct are mostly void, so let's enforce it … because we can"? > > On these grounds and it being an additional (albeit mostly small) unnecessary BC break, I'm not in favor of that RFC. > > Bob
  110668
June 18, 2020 17:04 benas.molis.iml@gmail.com (Benas IML)
Hey Bob,

Your examples (regarding constructors/destructors) do make sense since you
are using these methods for what they should be used: initialization and
freeing resources. But, this is off-topic to the RFC.

This RFC only proposes to enforce no-return rule on
constructors/destructors. And based on my previous reply, top 2,000
Composer packages that have a BC break don't actually return anything
(early returns and returns that don't change code behavior don't count).

Your examples regarding `__toString`, `__invoke` and `__serialize` are also
not relevant to this RFC. Although, I do understand that you were
responding to email and I'm thankful for that :)


Best regards,
Benas

On Thu, Jun 18, 2020, 7:43 PM Bob Weinand <bobwei9@hotmail.com> wrote:

> Hey, > > Am 18.06.2020 um 17:18 schrieb Benas IML iml@gmail.com>: > > Hey Bob, > > Magic methods are **never** supposed to be called directly (even more if > that method is a constructor or a destructor). If that's not the case, it's > just plain bad code. But by enforcing these rules, we make sure that less > of that (bad code) is written and as a result, we make PHP code less > bug-prone and easier to debug. That's also most likely the reason why > > > __construct() is invoked directly on parent calls, sometimes to > reinitialize an object or > after ReflectionClass::newInstanceWithoutConstructor. > > I invoke __destruct() directly when needing an early freeing of existing > resources. > > "ensure magic methods' signature" RFC opted in to validate `__clone` > method's signature and ensure that it has `void` return type. > > Just for the sake of making sure that you understand what I mean, here are > a couple of examples that show that no magic method is ever supposed to be > called directly: > ```php > // __toString > (string) $object; > > > I like using ->__toString() in favor of (string) casts when the variable > is guaranteed to be an object to highlight that and avoid magic-ness. > > // __invoke > $object(); > > > Same here, unless the object is a closure. > > // __serialize > serialize($object); > ``` > > > Can't argue much about that one, I never use serialize(). > > Moreover, by validating constructors/destructors and allowing an explicit > `void` return type declaration, we are becoming much more consistent > (something that PHP is striving for) with other magic methods (e. g. > `__clone`). > > > Yeah, __clone() is odd. No idea why. > > Also, saying that "sometimes you have valid information to pass from the > parent class" is quite an overstatement. After analyzing most of the 95 > Composer packages that had a potential BC break, I found out that either > they wanted to return early (that is still possible to do using `return;`) > or they added a `return something;` for no reason. Thus, no libraries > actually returned something useful and valid from a constructor (as they > shouldn't). > > Last but certainly not least, constructors have one and only one > responsibility - to initialize an object. Whether you read Wikipedia's or > PHP manual's definition, a constructor does just that. It initializes. So, > the PHP manual is perfectly correct and documents the correct return type > that a constructor should have. > > > It also is generally a bad idea to have side effects in constructors, but > _sometimes_ it is justified. Only because something mostly is a bad idea, > it is not always. > Also note that other languages completely forbid manual ctor calls. But > PHP doesn't (and for good reason, like after > using ReflectionClass::newInstanceWithoutConstructor). > > Bob > > Best regards, > Benas > > On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobwei9@hotmail.com> wrote: > >> > Am 17.06.2020 um 01:10 schrieb Benas IML iml@gmail.com>: >> > >> > Hey internals, >> > >> > This is a completely refined, follow-up RFC to my original RFC. Based >> on the >> > feedback I have received, this PR implements full validation and >> implicitly >> > enforces `void` rules on constructors/destructors while also allowing to >> > declare an **optional** explicit `void` return type. Note, that there >> is a >> > small but justifiable BC break (as stated by the RFC). >> > >> > RFC: https://wiki.php.net/rfc/make_ctor_ret_void >> > >> > Best regards, >> > Benas Seliuginas >> >> Hey Benas, >> >> I do not see any particular benefit from that RFC. >> >> Regarding what the manual states - the manual is wrong there and thus >> should be fixed in the manual. This is not an argument for changing engine >> behaviour. >> >> Sometimes a constructor (esp. of a parent class) or destructor may be >> called manually. Sometimes you have valid information to pass from the >> parent class. >> With your RFC an arbitrary restriction is introduced necessitating an >> extra method instead. >> >> In general that RFC feels like "uh, __construct and __destruct are mostly >> void, so let's enforce it … because we can"? >> >> On these grounds and it being an additional (albeit mostly small) >> unnecessary BC break, I'm not in favor of that RFC. >> >> Bob > > >
  110672
June 18, 2020 20:15 andreas@dqxtech.net (Andreas Hennings)
On Thu, 18 Jun 2020 at 18:33, Benas IML iml@gmail.com> wrote:

> Hey Andreas, > > That is incorrect. Adding an explicit `: void` does change behavior since > only then the check (whether something is being returned) is enforced. This > allows PHP 8 projects to take advantage of this enforcement while being > respective to older codebases. > > Ok. I read more carefully now.
But this distinction would only apply in PHP 8. And the only difference here is whether returning a value is just deprecated or fully illegal. In PHP 9, constructors with ": void" would be the same as without ": void". So long term it will become a "meaningless choice". Or what am I missing?
> And well, the explicit `: void` declaration also helps your code to be > more consistent with other methods ;) >
Except consistency with existing constructors in other packages which choose to not add ": void" to constructors.
> > Without an explicit `: void` return type declaration, `void` rules are not > enforced on constructors/destructors and will not be until PHP 9.0 (which > will probably release in 5 years). > > Moreover, saying "it forces organisations, frameworks or the php-fig group > to introduce yet another coding convention" is a complete exaggeration. In > fact, even now there are no PSR conventions that specify how and when to > write parameter/return/property type hints. >
Either it is enforced in a "code convention", or it is up to every single developer and team, and we get silly arguments between developers in code review whether or not this should be added. Or we get git noise because one developer adds those declarations, and another removes them.
> > Also, it's important to understand that PHP libraries are really really > slow at starting to **depend** on new PHP versions. It will probably take a > few good years (if not more) until first few libraries start requiring PHP > 8.0. As a matter of fact, even Symfony framework is still requiring PHP > 7.2.5 and cannot take advantage of its newer features (e. g. typed > properties). >
So if you want to support PHP 7, you cannot add ": void". If you only care about PHP 9, you don't need to add ": void" because it is pointless. Any convention would probably discourage it to be more universally usable.
> > Last but not least, just to reiterate, the `: void` return type is > optional and you don't need to specify it. >
Exactly my point. "Optional" means people will make arbitrary choices and argue about it, or look for a convention. Greetings Andreas
> > Best regards, > Benas > > On Thu, Jun 18, 2020, 7:02 PM Andreas Hennings <andreas@dqxtech.net> > wrote: > >> Hi >> >> The RFC introduces what I call a "meaningless choice", by making >> something possible that is currently illegal, but which does not change >> behavior. >> https://3v4l.org/daeEm >> >> It forces organisations, frameworks or the php-fig group to introduce yet >> another coding convention to decide whether or not there should be a ": >> void" declaration on constructors. >> >> I am ok with restricting the use of "return *;" inside a constructor. >> But I don't like allowing the ": void" declaration. >> >> Greetings >> >> On Thu, 18 Jun 2020 at 17:18, Benas IML iml@gmail.com> >> wrote: >> >>> Hey Bob, >>> >>> Magic methods are **never** supposed to be called directly (even more if >>> that method is a constructor or a destructor). If that's not the case, >>> it's >>> just plain bad code. But by enforcing these rules, we make sure that less >>> of that (bad code) is written and as a result, we make PHP code less >>> bug-prone and easier to debug. That's also most likely the reason why >>> "ensure magic methods' signature" RFC opted in to validate `__clone` >>> method's signature and ensure that it has `void` return type. >>> >>> Just for the sake of making sure that you understand what I mean, here >>> are >>> a couple of examples that show that no magic method is ever supposed to >>> be >>> called directly: >>> ```php >>> // __toString >>> (string) $object; >>> >>> // __invoke >>> $object(); >>> >>> // __serialize >>> serialize($object); >>> ``` >>> >>> Moreover, by validating constructors/destructors and allowing an explicit >>> `void` return type declaration, we are becoming much more consistent >>> (something that PHP is striving for) with other magic methods (e. g. >>> `__clone`). >>> >>> Also, saying that "sometimes you have valid information to pass from the >>> parent class" is quite an overstatement. After analyzing most of the 95 >>> Composer packages that had a potential BC break, I found out that either >>> they wanted to return early (that is still possible to do using >>> `return;`) >>> or they added a `return something;` for no reason. Thus, no libraries >>> actually returned something useful and valid from a constructor (as they >>> shouldn't). >>> >>> Last but certainly not least, constructors have one and only one >>> responsibility - to initialize an object. Whether you read Wikipedia's or >>> PHP manual's definition, a constructor does just that. It initializes. >>> So, >>> the PHP manual is perfectly correct and documents the correct return type >>> that a constructor should have. >>> >>> Best regards, >>> Benas >>> >>> On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobwei9@hotmail.com> wrote: >>> >>> > > Am 17.06.2020 um 01:10 schrieb Benas IML iml@gmail.com >>> >: >>> > > >>> > > Hey internals, >>> > > >>> > > This is a completely refined, follow-up RFC to my original RFC. >>> Based on >>> > the >>> > > feedback I have received, this PR implements full validation and >>> > implicitly >>> > > enforces `void` rules on constructors/destructors while also >>> allowing to >>> > > declare an **optional** explicit `void` return type. Note, that >>> there is >>> > a >>> > > small but justifiable BC break (as stated by the RFC). >>> > > >>> > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void >>> > > >>> > > Best regards, >>> > > Benas Seliuginas >>> > >>> > Hey Benas, >>> > >>> > I do not see any particular benefit from that RFC. >>> > >>> > Regarding what the manual states - the manual is wrong there and thus >>> > should be fixed in the manual. This is not an argument for changing >>> engine >>> > behaviour. >>> > >>> > Sometimes a constructor (esp. of a parent class) or destructor may be >>> > called manually. Sometimes you have valid information to pass from the >>> > parent class. >>> > With your RFC an arbitrary restriction is introduced necessitating an >>> > extra method instead. >>> > >>> > In general that RFC feels like "uh, __construct and __destruct are >>> mostly >>> > void, so let's enforce it … because we can"? >>> > >>> > On these grounds and it being an additional (albeit mostly small) >>> > unnecessary BC break, I'm not in favor of that RFC. >>> > >>> > Bob >> >> On Thu, 18 Jun 2020 at 19:05, Benas IML iml@gmail.com> wrote:
> Hey Bob, > > Your examples (regarding constructors/destructors) do make sense since you > are using these methods for what they should be used: initialization and > freeing resources. But, this is off-topic to the RFC. > > This RFC only proposes to enforce no-return rule on > constructors/destructors. And based on my previous reply, top 2,000 > Composer packages that have a BC break don't actually return anything > (early returns and returns that don't change code behavior don't count). > > Your examples regarding `__toString`, `__invoke` and `__serialize` are also > not relevant to this RFC. Although, I do understand that you were > responding to email and I'm thankful for that :) > > > Best regards, > Benas > > On Thu, Jun 18, 2020, 7:43 PM Bob Weinand <bobwei9@hotmail.com> wrote: > > > Hey, > > > > Am 18.06.2020 um 17:18 schrieb Benas IML iml@gmail.com>: > > > > Hey Bob, > > > > Magic methods are **never** supposed to be called directly (even more if > > that method is a constructor or a destructor). If that's not the case, > it's > > just plain bad code. But by enforcing these rules, we make sure that less > > of that (bad code) is written and as a result, we make PHP code less > > bug-prone and easier to debug. That's also most likely the reason why > > > > > > __construct() is invoked directly on parent calls, sometimes to > > reinitialize an object or > > after ReflectionClass::newInstanceWithoutConstructor. > > > > I invoke __destruct() directly when needing an early freeing of existing > > resources. > > > > "ensure magic methods' signature" RFC opted in to validate `__clone` > > method's signature and ensure that it has `void` return type. > > > > Just for the sake of making sure that you understand what I mean, here > are > > a couple of examples that show that no magic method is ever supposed to > be > > called directly: > > ```php > > // __toString > > (string) $object; > > > > > > I like using ->__toString() in favor of (string) casts when the variable > > is guaranteed to be an object to highlight that and avoid magic-ness. > > > > // __invoke > > $object(); > > > > > > Same here, unless the object is a closure. > > > > // __serialize > > serialize($object); > > ``` > > > > > > Can't argue much about that one, I never use serialize(). > > > > Moreover, by validating constructors/destructors and allowing an explicit > > `void` return type declaration, we are becoming much more consistent > > (something that PHP is striving for) with other magic methods (e. g. > > `__clone`). > > > > > > Yeah, __clone() is odd. No idea why. > > > > Also, saying that "sometimes you have valid information to pass from the > > parent class" is quite an overstatement. After analyzing most of the 95 > > Composer packages that had a potential BC break, I found out that either > > they wanted to return early (that is still possible to do using > `return;`) > > or they added a `return something;` for no reason. Thus, no libraries > > actually returned something useful and valid from a constructor (as they > > shouldn't). > > > > Last but certainly not least, constructors have one and only one > > responsibility - to initialize an object. Whether you read Wikipedia's or > > PHP manual's definition, a constructor does just that. It initializes. > So, > > the PHP manual is perfectly correct and documents the correct return type > > that a constructor should have. > > > > > > It also is generally a bad idea to have side effects in constructors, but > > _sometimes_ it is justified. Only because something mostly is a bad idea, > > it is not always. > > Also note that other languages completely forbid manual ctor calls. But > > PHP doesn't (and for good reason, like after > > using ReflectionClass::newInstanceWithoutConstructor). > > > > Bob > > > > Best regards, > > Benas > > > > On Thu, Jun 18, 2020, 4:06 PM Bob Weinand <bobwei9@hotmail.com> wrote: > > > >> > Am 17.06.2020 um 01:10 schrieb Benas IML iml@gmail.com>: > >> > > >> > Hey internals, > >> > > >> > This is a completely refined, follow-up RFC to my original RFC. Based > >> on the > >> > feedback I have received, this PR implements full validation and > >> implicitly > >> > enforces `void` rules on constructors/destructors while also allowing > to > >> > declare an **optional** explicit `void` return type. Note, that there > >> is a > >> > small but justifiable BC break (as stated by the RFC). > >> > > >> > RFC: https://wiki.php.net/rfc/make_ctor_ret_void > >> > > >> > Best regards, > >> > Benas Seliuginas > >> > >> Hey Benas, > >> > >> I do not see any particular benefit from that RFC. > >> > >> Regarding what the manual states - the manual is wrong there and thus > >> should be fixed in the manual. This is not an argument for changing > engine > >> behaviour. > >> > >> Sometimes a constructor (esp. of a parent class) or destructor may be > >> called manually. Sometimes you have valid information to pass from the > >> parent class. > >> With your RFC an arbitrary restriction is introduced necessitating an > >> extra method instead. > >> > >> In general that RFC feels like "uh, __construct and __destruct are > mostly > >> void, so let's enforce it … because we can"? > >> > >> On these grounds and it being an additional (albeit mostly small) > >> unnecessary BC break, I'm not in favor of that RFC. > >> > >> Bob > > > > > > >
  110673
June 18, 2020 20:29 benas.molis.iml@gmail.com (Benas IML)
Hey Andreas,

It seems that you actually haven't read my reply carefully enough.

> But this distinction would only apply in PHP 8. And the only difference here
is whether returning a value is just deprecated or fully illegal.
> In PHP 9, constructors with ": void" would be the same as without ": void".
So long term it will become a "meaningless choice".
> > Or what am I missing?
The "allowance" of `void` return type will help: - to be more explicit. - to enforce `void` rules on constructors/destructors (in PHP 8). - to be more consistent with other methods (which is what people like the most about allowing `void` on ctor/dtor based on r/php). - to be more consistent with the documentation.
> Except consistency with existing constructors in other packages which choose
to not add ": void" to constructors.
> > Either it is enforced in a "code convention", or it is up to every single developer and team, and we get silly arguments between developers in code
review whether or not this should be added. Or we get git noise because one developer adds those declarations, and another removes them. I find these comments of yours to make completely no sense since most of the additions to PHP will create some sort of silly arguments between developers. A best example would be the `mixed` type. Based on what you have said, this pseudo type will create inconsistencies because some libraries will have parameters explicitly declared as `mixed` while others - won't. It will also create arguments between developers on whether they should use it or not. Moving on, let's take the `void` type (implemented in PHP 7.1) as another great example! Laravel and Symfony (both depend on PHP 7.2) don't use it while other libraries - do. So, as I understand based on your comments, this is creating inconsistencies and arguments between developers. Some say `void` should be added, some say not. Some libraries declare it, some don't. Moving on, attributes. If you go onto Reddit, you can see that the crowd is divided 50/50. Some believe that attributes are bad and say that they will use functions (for "adding" metadata) instead. Others prefer attributes and will use those. You can literally find people bashing each other for their preference. So that is creating heated arguments AND possibly future inconsistencies between different people/libraries. It's not possible to be "politically" neutral. Every single feature/code style is used by some group of people and isn't used by another. So next time please be more pragmatic.
> So if you want to support PHP 7, you cannot add ": void". > If you only care about PHP 9, you don't need to add ": void" because it is pointless.
> > Any convention would probably discourage it to be more universally usable.
This is a completely contradicting statement. Just a few sentences ago you said that there will be silly arguments between people. But now as I understand, most conventions will probably discourage the explicit `: void` return type on constructors/destructors. Thus, since most people follow conventions, there will be little-to-no arguments.
> Exactly my point. "Optional" means people will make arbitrary choices and argue about it, or look for a convention.
Already addressed this. Best regards, Benas On Thu, Jun 18, 2020, 11:15 PM Andreas Hennings <andreas@dqxtech.net> wrote:
> > On Thu, 18 Jun 2020 at 18:33, Benas IML iml@gmail.com> wrote: > >> Hey Andreas, >> >> That is incorrect. Adding an explicit `: void` does change behavior since >> only then the check (whether something is being returned) is enforced. This >> allows PHP 8 projects to take advantage of this enforcement while being >> respective to older codebases. >> >> Ok. I read more carefully now. > > But this distinction would only apply in PHP 8. And the only difference > here is whether returning a value is just deprecated or fully illegal. > In PHP 9, constructors with ": void" would be the same as without ": void". > So long term it will become a "meaningless choice". > > Or what am I missing? > > > >> And well, the explicit `: void` declaration also helps your code to be >> more consistent with other methods ;) >> > > Except consistency with existing constructors in other packages which > choose to not add ": void" to constructors. > > >> >> Without an explicit `: void` return type declaration, `void` rules are >> not enforced on constructors/destructors and will not be until PHP 9.0 >> (which will probably release in 5 years). >> >> Moreover, saying "it forces organisations, frameworks or the php-fig >> group to introduce yet another coding convention" is a complete >> exaggeration. In fact, even now there are no PSR conventions that specify >> how and when to write parameter/return/property type hints. >> > > Either it is enforced in a "code convention", or it is up to every single > developer and team, and we get silly arguments between developers in code > review whether or not this should be added. Or we get git noise because one > developer adds those declarations, and another removes them. > > >> >> Also, it's important to understand that PHP libraries are really really >> slow at starting to **depend** on new PHP versions. It will probably take a >> few good years (if not more) until first few libraries start requiring PHP >> 8.0. As a matter of fact, even Symfony framework is still requiring PHP >> 7.2.5 and cannot take advantage of its newer features (e. g. typed >> properties). >> > > So if you want to support PHP 7, you cannot add ": void". > If you only care about PHP 9, you don't need to add ": void" because it is > pointless. > > Any convention would probably discourage it to be more universally usable. > > > >> >> Last but not least, just to reiterate, the `: void` return type is >> optional and you don't need to specify it. >> > > Exactly my point. "Optional" means people will make arbitrary choices and > argue about it, or look for a convention. > > Greetings > Andreas > > > >> >> Best regards, >> Benas >> >
  110674
June 18, 2020 20:49 andreas@dqxtech.net (Andreas Hennings)
On Thu, 18 Jun 2020 at 22:29, Benas IML iml@gmail.com> wrote:

> Hey Andreas, > > It seems that you actually haven't read my reply carefully enough. > > > But this distinction would only apply in PHP 8. And the only difference > here > is whether returning a value is just deprecated or fully illegal. > > In PHP 9, constructors with ": void" would be the same as without ": > void". > So long term it will become a "meaningless choice". > > > > Or what am I missing? > > The "allowance" of `void` return type will help: > - to be more explicit. > - to enforce `void` rules on constructors/destructors (in PHP 8). > - to be more consistent with other methods (which is what people like the > most > about allowing `void` on ctor/dtor based on r/php). > - to be more consistent with the documentation. > > > Except consistency with existing constructors in other packages which > choose > to not add ": void" to constructors. > > > > Either it is enforced in a "code convention", or it is up to every single > developer and team, and we get silly arguments between developers in code > review whether or not this should be added. Or we get git noise because one > developer adds those declarations, and another removes them. > > I find these comments of yours to make completely no sense since most of > the > additions to PHP will create some sort of silly arguments between > developers. > > A best example would be the `mixed` type. Based on what you have said, this > pseudo type will create inconsistencies because some libraries will have > parameters explicitly declared as `mixed` while others - won't. It will > also > create arguments between developers on whether they should use it or not. > > Moving on, let's take the `void` type (implemented in PHP 7.1) as another > great > example! Laravel and Symfony (both depend on PHP 7.2) don't use it while > other > libraries - do. So, as I understand based on your comments, this is > creating > inconsistencies and arguments between developers. Some say `void` should be > added, some say not. Some libraries declare it, some don't. >
Another example would be the "public" access specifier, which can be omitted and the method will still be public. There is a major difference though: If you look at a method without "public", or a parameter without "mixed", there is a chance that the developer actually should have put something else here, e.g. "private" or "string", and was simply too lazy, was not aware of that possibility, or wrote the code with a prior PHP version in mind. Having the explicit keyword documents an intentional choice to make the method public, to make the parameter mixed, etc. On the other hand, for a constructor the ": void" is just stating the obvious. Even if you see a constructor without ": void", you still know that this is not meant to return anything. Either because it is generally agreed to be bad (PHP7) or because it is deprecated (PHP8) or because it is illegal (PHP9)
> > Moving on, attributes. If you go onto Reddit, you can see that the crowd is > divided 50/50. Some believe that attributes are bad and say that they will > use > functions (for "adding" metadata) instead. Others prefer attributes and > will > use those. You can literally find people bashing each other for their > preference. So that is creating heated arguments AND possibly future > inconsistencies between different people/libraries. >
But here there are actual technical arguments why someone might prefer one or the other solution.
> > It's not possible to be "politically" neutral. Every single feature/code > style > is used by some group of people and isn't used by another. So next time > please > be more pragmatic. > > > So if you want to support PHP 7, you cannot add ": void". > > If you only care about PHP 9, you don't need to add ": void" because it > is > pointless. > > > > Any convention would probably discourage it to be more universally > usable. > > This is a completely contradicting statement. Just a few sentences ago you > said > that there will be silly arguments between people. But now as I understand, > most conventions will probably discourage the explicit `: void` return > type on > constructors/destructors. Thus, since most people follow conventions, there > will be little-to-no arguments. >
There are conventions or popular preference, and there are people or projects which want to do things their own way. And of course there is the time before agreements are reached in specific conventions, where people produce code which could be one way or the other. I don't see a contradiction here. Greetings Andreas
> > Exactly my point. "Optional" means people will make arbitrary choices and > argue about it, or look for a convention. > > Already addressed this. > > Best regards, > Benas > > On Thu, Jun 18, 2020, 11:15 PM Andreas Hennings <andreas@dqxtech.net> > wrote: > >> >> On Thu, 18 Jun 2020 at 18:33, Benas IML iml@gmail.com> >> wrote: >> >>> Hey Andreas, >>> >>> That is incorrect. Adding an explicit `: void` does change behavior >>> since only then the check (whether something is being returned) is >>> enforced. This allows PHP 8 projects to take advantage of this enforcement >>> while being respective to older codebases. >>> >>> Ok. I read more carefully now. >> >> But this distinction would only apply in PHP 8. And the only difference >> here is whether returning a value is just deprecated or fully illegal. >> In PHP 9, constructors with ": void" would be the same as without ": >> void". >> So long term it will become a "meaningless choice". >> >> Or what am I missing? >> >> >> >>> And well, the explicit `: void` declaration also helps your code to be >>> more consistent with other methods ;) >>> >> >> Except consistency with existing constructors in other packages which >> choose to not add ": void" to constructors. >> >> >>> >>> Without an explicit `: void` return type declaration, `void` rules are >>> not enforced on constructors/destructors and will not be until PHP 9.0 >>> (which will probably release in 5 years). >>> >>> Moreover, saying "it forces organisations, frameworks or the php-fig >>> group to introduce yet another coding convention" is a complete >>> exaggeration. In fact, even now there are no PSR conventions that specify >>> how and when to write parameter/return/property type hints. >>> >> >> Either it is enforced in a "code convention", or it is up to every single >> developer and team, and we get silly arguments between developers in code >> review whether or not this should be added. Or we get git noise because one >> developer adds those declarations, and another removes them. >> >> >>> >>> Also, it's important to understand that PHP libraries are really really >>> slow at starting to **depend** on new PHP versions. It will probably take a >>> few good years (if not more) until first few libraries start requiring PHP >>> 8.0. As a matter of fact, even Symfony framework is still requiring PHP >>> 7.2.5 and cannot take advantage of its newer features (e. g. typed >>> properties). >>> >> >> So if you want to support PHP 7, you cannot add ": void". >> If you only care about PHP 9, you don't need to add ": void" because it >> is pointless. >> >> Any convention would probably discourage it to be more universally usable. >> >> >> >>> >>> Last but not least, just to reiterate, the `: void` return type is >>> optional and you don't need to specify it. >>> >> >> Exactly my point. "Optional" means people will make arbitrary choices and >> argue about it, or look for a convention. >> >> Greetings >> Andreas >> >> >> >>> >>> Best regards, >>> Benas >>> >>
  110676
June 18, 2020 21:16 larry@garfieldtech.com ("Larry Garfield")
On Thu, Jun 18, 2020, at 3:49 PM, Andreas Hennings wrote:

> On the other hand, for a constructor the ": void" is just stating the > obvious. > Even if you see a constructor without ": void", you still know that this is > not meant to return anything. > Either because it is generally agreed to be bad (PHP7) or because it is > deprecated (PHP8) or because it is illegal (PHP9)
I see this in the same category as __toString(). Adding `: string` to that method provides exactly zero additional information. You know it's going to return a string. That's it's whole purpose. On the off chance someone is returning a non-string right now, they're very clearly Doing It Wrong(tm). However, the stringable RFC added the ability to put `: string` on the method in order to be clearer, more explicit, and to not annoy type fans who are like "Ah, I've typed every one of my methods... except this one, because I can't, raaaah!" I see this as the same for constructors. Any constructor returning non-void right now is Doing It Wrong(tm), and you know that going in. But being able to make the obvious explicit still has its advantages, both for documentation and for consistency. I am in favor of this RFC for that reason. --Larry Garfield
  110677
June 18, 2020 21:29 benas.molis.iml@gmail.com (Benas IML)
Hey Andreas,

> Another example would be the "public" access specifier, which can be omitted and the method will still be public.
> > There is a major difference though: > > If you look at a method without "public", or a parameter without "mixed", there is a chance that the developer actually should have put something
else here, e.g. "private" or "string", and was simply too lazy, was not aware of that possibility, or wrote the code with a prior PHP version in mind.
> Having the explicit keyword documents an intentional choice to make the method public, to make the parameter mixed, etc.
`: void` on constructor/destructor also helps to explicitly show the developer's intention to not return a value.
> There are conventions or popular preference, and there are people or projects which want to do things their own way.
And here's another reason why letting `: void` is a good idea. PHP should not enforce conventions but at most recommend them and allow the community to have the freedom of choice.
> On the other hand, for a constructor the ": void" is just stating the obvious.
> Even if you see a constructor without ": void", you still know that this is not meant to return anything.
> Either because it is generally agreed to be bad (PHP7) or because it is deprecated (PHP8) or because it is illegal (PHP9)
And why that matters? Everyone knows that the `__toString` method returns a `string` and everyone knows that the `__clone` method returns a `void`. In both those cases, adding an explicit return type is allowed.
> But here there are actual technical arguments why someone might prefer one or the other solution
I never said that I was referring to people having technical arguments? Anyways, this is not relevant to the RFC
> And of course there is the time before agreements are reached in specific conventions, where people produce code which could be one way or the other.
> I don't see a contradiction here.
Again, I'm not sure what you're referring to. Best regards, Benas On Thu, Jun 18, 2020, 11:49 PM Andreas Hennings <andreas@dqxtech.net> wrote:
> > > On Thu, 18 Jun 2020 at 22:29, Benas IML iml@gmail.com> wrote: > >> Hey Andreas, >> >> It seems that you actually haven't read my reply carefully enough. >> >> > But this distinction would only apply in PHP 8. And the only difference >> here >> is whether returning a value is just deprecated or fully illegal. >> > In PHP 9, constructors with ": void" would be the same as without ": >> void". >> So long term it will become a "meaningless choice". >> > >> > Or what am I missing? >> >> The "allowance" of `void` return type will help: >> - to be more explicit. >> - to enforce `void` rules on constructors/destructors (in PHP 8). >> - to be more consistent with other methods (which is what people like the >> most >> about allowing `void` on ctor/dtor based on r/php). >> - to be more consistent with the documentation. >> >> > Except consistency with existing constructors in other packages which >> choose >> to not add ": void" to constructors. >> > >> > Either it is enforced in a "code convention", or it is up to every >> single >> developer and team, and we get silly arguments between developers in code >> review whether or not this should be added. Or we get git noise because >> one >> developer adds those declarations, and another removes them. >> >> I find these comments of yours to make completely no sense since most of >> the >> additions to PHP will create some sort of silly arguments between >> developers. >> >> A best example would be the `mixed` type. Based on what you have said, >> this >> pseudo type will create inconsistencies because some libraries will have >> parameters explicitly declared as `mixed` while others - won't. It will >> also >> create arguments between developers on whether they should use it or not. >> >> Moving on, let's take the `void` type (implemented in PHP 7.1) as another >> great >> example! Laravel and Symfony (both depend on PHP 7.2) don't use it while >> other >> libraries - do. So, as I understand based on your comments, this is >> creating >> inconsistencies and arguments between developers. Some say `void` should >> be >> added, some say not. Some libraries declare it, some don't. >> > > Another example would be the "public" access specifier, which can be > omitted and the method will still be public. > > There is a major difference though: > > If you look at a method without "public", or a parameter without "mixed", > there is a chance that the developer actually should have put something > else here, e.g. "private" or "string", and was simply too lazy, was not > aware of that possibility, or wrote the code with a prior PHP version in > mind. > Having the explicit keyword documents an intentional choice to make the > method public, to make the parameter mixed, etc. > > On the other hand, for a constructor the ": void" is just stating the > obvious. > Even if you see a constructor without ": void", you still know that this > is not meant to return anything. > Either because it is generally agreed to be bad (PHP7) or because it is > deprecated (PHP8) or because it is illegal (PHP9) > > > >> >> Moving on, attributes. If you go onto Reddit, you can see that the crowd >> is >> divided 50/50. Some believe that attributes are bad and say that they >> will use >> functions (for "adding" metadata) instead. Others prefer attributes and >> will >> use those. You can literally find people bashing each other for their >> preference. So that is creating heated arguments AND possibly future >> inconsistencies between different people/libraries. >> > > But here there are actual technical arguments why someone might prefer one > or the other solution. > > >> >> It's not possible to be "politically" neutral. Every single feature/code >> style >> is used by some group of people and isn't used by another. So next time >> please >> be more pragmatic. >> >> > So if you want to support PHP 7, you cannot add ": void". >> > If you only care about PHP 9, you don't need to add ": void" because it >> is >> pointless. >> > >> > Any convention would probably discourage it to be more universally >> usable. >> >> This is a completely contradicting statement. Just a few sentences ago >> you said >> that there will be silly arguments between people. But now as I >> understand, >> most conventions will probably discourage the explicit `: void` return >> type on >> constructors/destructors. Thus, since most people follow conventions, >> there >> will be little-to-no arguments. >> > > > And of course there is the time before agreements are reached in specific > conventions, where people produce code which could be one way or the other. > I don't see a contradiction here. > > Greetings > Andreas > >>
  110692
June 21, 2020 14:12 rowan.collins@gmail.com (Rowan Tommins)
Hi Andreas,

On 18/06/2020 17:02, Andreas Hennings wrote:
> I am ok with restricting the use of "return *;" inside a constructor. > But I don't like allowing the ": void" declaration.
The way I look at it, constructors are mostly declared like a normal method - they use the keyword "function"; can be marked public, private, protected, abstract, and final; and can have a parameter list, with types and defaults - so the surprising thing is that there is a special rule *forbidding* them from having a return specifier. If we were designing the language from scratch, would there be any particular reason to *add* that restriction? On 18/06/2020 21:49, Andreas Hennings wrote:
> On the other hand, for a constructor the ": void" is just stating the > obvious. > Even if you see a constructor without ": void", you still know that this is > not meant to return anything. > Either because it is generally agreed to be bad (PHP7) or because it is > deprecated (PHP8) or because it is illegal (PHP9)
As long as it's possible to return things from constructors, then it is "obvious" that a given constructor is void only in the same way that it is "obvious" that a method called "convertToArray" returns an array. I would be surprised if any style guide would forbid writing "public function convertToArray(): array", so would be equally surprised to see one forbid writing "public function __construct(): void". In both cases, the extra marker takes the reader from 99% certain to 100%. If in future the rules are tightened so that constructors implicitly apply the same restriction as if they were labelled ": void", then it is instead "meaningless" in the same sense that the ": Traversable" is meaningless in this code: class Foo implements IteratorAggregate {     // ...     public function getIterator(): Traversable {         // ...     } } For historical reasons, the IteratorAggregate interface doesn't require the method to declare its return type, but the code using it performs the same check as if it did. Enforcing "voidness" on constructors feels similar - if there was no concern for backwards compatibility, we would probably enforce that getIterator() was marked with ": Traversable" (or an appropriate sub-type), and __construct() was marked ": void", and leave the rest of the logic to the generic code for handling those declarations. Again, I would be surprised if any style guide would forbid writing "getIterator(): Traversable" just because the check is already enforced implicitly by a different mechanism. Regards, -- Rowan Tommins (né Collins) [IMSoP]
  110795
June 30, 2020 22:02 benas.molis.iml@gmail.com (Benas IML)
Since 2 weeks have passed and there wasn't much discussion, I would like to
remind that I'm opening the vote on Friday (July 3rd).

Best regards,
Benas Seliuginas

On Wed, Jun 17, 2020, 2:10 AM Benas IML iml@gmail.com> wrote:

> Hey internals, > > This is a completely refined, follow-up RFC to my original RFC. Based on > the > feedback I have received, this PR implements full validation and implicitly > enforces `void` rules on constructors/destructors while also allowing to > declare an **optional** explicit `void` return type. Note, that there is a > small but justifiable BC break (as stated by the RFC). > > RFC: https://wiki.php.net/rfc/make_ctor_ret_void > > Best regards, > Benas Seliuginas >