laravel / pint

Laravel Pint is an opinionated PHP code style fixer for minimalists.
https://laravel.com/docs/pint
MIT License
2.75k stars 138 forks source link

[1.x] Enable single_line_empty_body fixer #277

Closed Jubeki closed 3 months ago

Jubeki commented 3 months ago

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
     }
 };
lupinitylabs commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

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

driesvints commented 2 months ago

@ju5t no worries 👍