Skip to content

[RFC] Allow void return type on constructors/destructors #5717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[RFC] Allow void return type on constructors/destructors #5717

wants to merge 1 commit into from

Conversation

moliata
Copy link
Contributor

@moliata moliata commented Jun 14, 2020

TODO list:

@moliata
Copy link
Contributor Author

moliata commented Jun 14, 2020

Let me know if additional tests are necessary (e. g. type widening).

@nikic
Copy link
Member

nikic commented Jun 14, 2020

As I've said before, I would prefer to just implicitly enforce void rules for constructors and destructors. There's no point in making people write that : void.

@moliata
Copy link
Contributor Author

moliata commented Jun 14, 2020

As I've said before, I would prefer to just implicitly enforce void rules for constructors and destructors. There's no point in making people write that : void.

@nikic There is a separate pull request for patching that bug and enforcing void rules on constructors/destructors. This RFC's goal is to allow using void on constructors/destructors (e. g. for consistency with other methods, alternative code style, etc.). In a way, it's like a trailing comma: some might use it, some might not, but both cases are allowed.

@theodorejb
Copy link
Contributor

@moliata A trailing comma is helpful to avoid modifying multiple lines when adding/removing items. I'm not sure what the benefit of explicitly writing void on constructors is, though. It's needless boilerplate that will just lead to code style arguments. I'm not even sure it makes sense, since constructors implicitly return a new instance of the class.

@moliata
Copy link
Contributor Author

moliata commented Jun 15, 2020

@moliata A trailing comma is helpful to avoid modifying multiple lines when adding/removing items. I'm not sure what the benefit of explicitly writing void on constructors is, though. It's needless boilerplate that will just lead to code style arguments. I'm not even sure it makes sense, since constructors implicitly return a new instance of the class.

Thank you @theodorejb for your opinion. Here is my take on it and my reasons as to why we should allow void return type on constructors/destructors.

  1. Explicit declaration (only matters after the other PR for enforcing void rules passes)
<?php
class Test {
    public function __construct() {}
}

class Test2 extends Test {
    public function __construct() {
        // WTF? Why can't I do this? No return type means mixed|void, right?
        $parent = parent::__construct();
    }
}
  1. Documented as void
<?php
...

// PHP manual explicitly states (in multiple pages) void return type
public function __construct(): void;
  1. __clone allows void return type (only matters after Gabriel's PR passes)
<?php
...

$object = new Test();
// Also implicitly returns a cloned object yet after Gabriel's PR
// we will be able to use void return type on __clone
$object_2 = clone $object;
  1. Consistency with other methods
    This one doesn't need a code example but given that every other method (including magic methods) can have a return type specified, I would also like to (for consistency) add return types to constructors/destructors.

@theodorejb
Copy link
Contributor

@moliata Most other magic methods that allow a return type allow more types than void, though. If void is the only allowed type, and doesn't change behavior, it's just extra verbosity and fuel for code style arguments.

@moliata
Copy link
Contributor Author

moliata commented Jun 15, 2020

I'm closing this PR since people dislike it so much, that someone can go as far as to email me a request to commit suicide.

@moliata moliata closed this Jun 15, 2020
@drealecs
Copy link

That's totally inappropriate.

I respect you trying to help the community with what you think it's good.
You should keep doing that further, even if sometimes you might be wrong. That is part of life.

Thank you!

@KalleZ
Copy link
Member

KalleZ commented Jun 15, 2020

@moliata

First of all, I am terribly sorry you have had this experience. I highly advice you to send an email to systems@php.net with the offenders email and request for a block at the PHP infrastructure (unless the person obfuscated his or her details to prevent being identified). This kind of behavior is totally unacceptable for any open source project. It most definitely does not belong in any relation to the PHP project. If you feel uncomfortable with writing to systems, you are also welcome to reach out to me in private (my email is <myfirstname>@php.net) and I will do it for you.

While you may feel discouraged to continue on the RFC, and respectfully so. I would encourage you to try look past this behavior as a bump in the road and continue with the RFC.

On a personal note, I think with @carusogabriel's current RFC, something like this is definitely an avenue to explore and something I could see myself write as a style, given it is a finger habbit to type a return type statement after each method you write.

Thank you,
-K

@moliata
Copy link
Contributor Author

moliata commented Jun 15, 2020

Thank you @KalleZ and @drealecs!

No offense was taken regarding the suicide email, had a good laugh instead tbh:). I closed the RFC since it seemed that just about everyone didn't really want to allow void return type on constructors and didn't want to start code style wars.

I do have a vision of having perfect consistency but some might not, so it seemed it wasn't worth the time and effort to put this RFC into voting, given that there weren't a lot of people supporting the idea.

@KalleZ
Copy link
Member

KalleZ commented Jun 15, 2020

@moliata I am glad to hear that.

Regarding the RFC, I assume it would be an optional return type, therefore it is a more or less cosmetic addition. Those disagreeing it with simply do not have to use it, those who write cross version compatible code probably won't use it for a while anyway which will probably cover most of those nay-sayers. Only rarely you can ever come to a unanimous agreement (if ever) regarding style, but optional additions to for more verbose styles should always be welcomed (similar to the trailing commas RFCs).

If it is something you personally desire to have added to PHP, I still highly encourage you to pursue it.

@moliata
Copy link
Contributor Author

moliata commented Jun 15, 2020

So after some consideration, I am reopening the PR and will push the RFC under discussion in the upcoming days.

I'll try to emphasize a bit more that this change is optional.

Thanks everybody:)

@moliata moliata reopened this Jun 15, 2020
@mvorisek
Copy link
Contributor

mvorisek commented Jun 16, 2020

Hi @moliata , did you get inspired by my bug report or even my PR #5678 ?

Do you think you it is good idea to disallow explicit : void in the constructor method to make at least @nikic and me happy? :D

In other methods (destructor etc.) I think allowing : void is fine to be consistent with other magic methods. Wdyt?

@moliata
Copy link
Contributor Author

moliata commented Jun 16, 2020

Hi @moliata , did you get inspired by my bug report or even my PR #5678 ?

Do you think you it is good idea to disallow explicit : void in the constructor method to make at least @nikic and me happy? :D

In other methods (destructor etc.) I think allowing : void is fine to be consistent with other magic methods. Wdyt?

Hey @mvorisek,

Since both PHP core members and r/PHP redditors are mostly okay with this change, I'm not planning to change my RFC: allow void return type on both constructors and destructors.

By suggestion, what I'm proposing is to join forces :) and incorporate these optional, cosmetic-only additions as well as the validation into a single RFC. If that's okay with you, hit me up at my public email account.

Best regards,
Benas

@moliata
Copy link
Contributor Author

moliata commented Jun 16, 2020

Closing this PR and making a new one since the updated RFC will not only allow optional explicit void return type but also implicitly enforce void rules on constructors/destructors.

@moliata moliata closed this Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants