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

[1.x] Enable single_line_empty_body fixer #277

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

Jubeki
Copy link
Contributor

@Jubeki Jubeki commented Jun 5, 2024

In many of my applications I have an empty constructor body, with only private or public attributes directly defined in the constructor parameter list. This results in an ugly code formatting in many cases.

This PR proposes to enable the single_line_empty_body fixer in the laravel preset.

Given the following file:

<?php

new class
{
    public function __construct(
        private string $name = '',
    ) {
    }

    public function function2() {}

    public function function3()
    {
        //
    }

    public function function4(
        string $name
    )
    {
        //
    }

    public function function5(
        string $name
    ) {
        //
    }

    public function function6() {
        // should still be fixed by the braces fixer
    }
};

The following changes would occur:

@@ -4,8 +4,7 @@
 {
     public function __construct(
         private string $name = '',
-    ) {
-    }
+    ) {}

     public function function2() {}

@@ -16,8 +15,7 @@

     public function function4(
         string $name
-    )
-    {
+    ) {
         //
     }

@@ -27,7 +25,8 @@
         //
     }

-    public function function6() {
+    public function function6()
+    {
         // should still be fixed by the braces fixer
     }
 };

@taylorotwell taylorotwell merged commit 5a75db5 into laravel:main Jun 5, 2024
8 checks passed
@Jubeki Jubeki deleted the single_line_empty_body branch June 5, 2024 15:00
@lupinitylabs
Copy link

lupinitylabs commented Jun 20, 2024

Frankly, I would appreciate if changes like these would not simply become a default for all Pint configurations that are based on that preset. I can see how this could be a strong preference for some people, but it's just that - a preference, not a necessary fix.

One needs to keep in mind that this results in an awful lot of changes to code in reviews from one pint version to the next, and it's quite an annoyance, and that on a global scale, for a lot of repositories at once.

That being said, I suppose the best practice for someone like me would be to just not base my config on the laravel preset anymore, because on the other hand, the laravel preset is opinionated after all and as such should be able to adapt to trends and preferences that have been agreed on.

@driesvints
Copy link
Member

Hey @lupinitylabs, I think you summarised that quite well, thank you. The Laravel preset is indeed opinionated and if you don't wish to be on par with (future) Laravel coding standards then the Laravel preset isn't for you and you might want to roll your own.

@ju5t
Copy link

ju5t commented Jun 20, 2024

@driesvints I think changes to presets should not be in a patch version. It should be a minor version as it feels like a new feature. If preset changes get released in minor versions you can pin Pint to the minor version and not miss out on bug fixes.

@driesvints
Copy link
Member

@ju5t this has been answered a couple of times already on this repo. We don't necessarily consider this a "new feature" but as a fix for the preset instead.

@ju5t
Copy link

ju5t commented Jun 20, 2024

Ah sorry, it didn't pop up when I searched for it. Thanks for the explanation.

@driesvints
Copy link
Member

@ju5t no worries 👍

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.

5 participants