Skip to content
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

Allow abbreviated empty bodies. #44

Merged
merged 3 commits into from
Nov 10, 2022
Merged

Conversation

Crell
Copy link
Collaborator

@Crell Crell commented Sep 12, 2022

Having worked with CPP a ton in the last year and a half, I feel very strongly that we need this. The rules otherwise require wasting 2 extra lines of pointlessness that does nothing to aid readability.

I'm making this a SHOULD rather than MUST for BC reasons, but I would be OK with making it a MUST.

@KorvinSzanto
Copy link
Contributor

This makes sense to me, the one question I'd pose is would it be better to require the method take up at least two lines? Ex:

class Point
{
    public function __construct(private int $x, private int $y)
    {}
    
    // ...
}
class Point
{
    public function __construct(
      public readonly int $x,
      public readonly int $y,
    ) {}
}

This feels like it'd help distinguish abbreviated methods from class properties.

@Crell
Copy link
Collaborator Author

Crell commented Sep 12, 2022

Honestly, I don't think so. It doesn't offer any visual benefit for me; it just leaves a {} handing there like a forgotten appendix.

We should probably do the same for empty classes, like exceptions. I'll add that.

@samdark
Copy link
Member

samdark commented Sep 13, 2022

While it looks a bit better, it is inconsistent i.e. adds more rules to follow making everything more complicated.

@Crell
Copy link
Collaborator Author

Crell commented Sep 13, 2022

Hence why a MAY.

Having worked with a lot of CPP-only constructors now, I firmly believe we do need to make this adjustment. What the current conventions (and thus any automation tools based on them) force you into is just ugly, wasteful noise. I know I'm not the only one, either.

@samdark
Copy link
Member

samdark commented Sep 14, 2022

Alright. With MAY it's OK.

@Crell
Copy link
Collaborator Author

Crell commented Sep 16, 2022

I'd also be OK with a SHOULD here... 😄

@KorvinSzanto
Copy link
Contributor

KorvinSzanto commented Nov 10, 2022

I'm still not a fan of method declarations taking a single line, I'd be more happy requiring the {} be on the next line to ensure methods don't look too much like class properties.

That said in the interest of progress let's merge this as is and discuss in a separate issue if any one else would like to.

@KorvinSzanto KorvinSzanto merged commit f89d1c9 into php-fig:master Nov 10, 2022
@Crell Crell deleted the no-body branch November 11, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants