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

Laravel preset class parentheses #285

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

brdv
Copy link
Contributor

@brdv brdv commented Jul 22, 2024

This PR adds the rule new_with_parentheses to the Laravel preset with the configuration. It's a followup for #284. The PR also runs Pint with the new rule over it's own codebase.

In after this change, running Pint with the Laravel Preset will make sure empty parentheses () will be removed from class Instantiations.

// the following line ...
$item = new Item();

// will become
$item = new Item;

@driesvints
Copy link
Member

@brdv can you fix the tests?

@driesvints driesvints marked this pull request as draft July 22, 2024 08:07
@driesvints driesvints marked this pull request as ready for review July 22, 2024 08:19
@taylorotwell taylorotwell merged commit 72f3117 into laravel:main Jul 22, 2024
6 checks passed
@rojtjo
Copy link

rojtjo commented Jul 25, 2024

For anyone looking to revert to the previous behavior. You can disable the rule by changing the value to false:

{
    "preset": "laravel",
    "rules": {
        "new_with_parentheses": false
    }
}

@dm-pf
Copy link

dm-pf commented Jul 25, 2024

Today I got a +5000/-5000 changelog after running pint. That's how I found this PR. I like the laravel preset because until now it has closely matched the PSR-12 one (which I like quite a lot).

On that note, how are these decisions made to change the 'laravel' preset? A discussion somewhere? On a whim?

@driesvints
Copy link
Member

@dm-pf we make these decisions internal at Laravel. This preset is how we want to format code at Laravel for our products/projects. Using it as a third party it's intended that this changes code in your project to match our preset. It doesn't matters how much lines of codes are changed as long as your project is up to date with the preset. If you don't want that then this preset isn't for you and it's better to roll one with your own rules.

@dm-pf
Copy link

dm-pf commented Jul 26, 2024

@driesvints thanks for the info. I'm fine with the changes, I just wanted some info on how decisions are made and you already clarified that, which I appreciate.

@kylemilloy
Copy link

Quite the spicy PR... 😏

I seem to remember at one point that this was the default behaviour and then got removed but maybe I'm remembering pre-Pint era when there was a Laravel preset for CS-Fixer that did this...personally I like the lack of control syntax when not required so thank you for bringing it back @brdv

@rrmesquita
Copy link

If both ways were fine, and you wanted Pint to opinionate on this rule, why not stick with PSR-12?

When instantiating a new class, parentheses MUST always be present even when there are no arguments passed to the constructor.

@kylemilloy
Copy link

@rrmesquita - see above.

This preset is how we want to format code at Laravel for our products/projects

tabuna added a commit to orchidsoftware/platform that referenced this pull request Aug 11, 2024
@ralphjsmit
Copy link

ralphjsmit commented Aug 24, 2024

The code by @rojtjo is correct, but it will not force the parentheses if they are not there yet in new code. If you want to enforce them always (which I prefer and find logical), you can use this:

"new_with_parentheses": {
    "anonymous_class": false,
    "named_class": true
}

Note: set the anonymous_class based on your own preferences. If enabled, it will change your migrations from return new class extends Migration to return new class() extends Migration.

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.

8 participants