[RFC] [DISCUSSION] Make constructors and destructors return void View as Markdown

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 benas.molis.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:

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

Am 17.06.2020 um 01:10 schrieb Benas IML <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

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

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

Hey,

Am 18.06.2020 um 17:18 schrieb Benas IML benas.molis.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:

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

Am 17.06.2020 um 01:10 schrieb Benas IML 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

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