[11.x] Add Laravel Pint

[browner12]

A lot of my recent PRs have been focused on bringing some styling consistency to this repo. PRs are fine, but automation is better, and that's what this PR adds.

In #53748 I made changes so chained calls on newlines are indented exactly once, and laid out why I think that is the best option. This PR will automate that change. The only non-whitespace changes are the composer.json and the pint.json files.

We are currently using StyleCI in our pipeline, and my first attempt was to try and use StyleCI to enforce this standard. Unfortunately, as far as I can tell, it's not possible with StyleCI.

Based on a comment from @crynobone in #53807, it seems like there are currently some logistical annoyances with using a Pint preset, so my proposal is that to start we use both StyleCI and Pint in tandem, and for Pint we do NOT use a preset, but rather explicitly add rules 1 by 1 that StyleCI is currently unable to handle. The end goal would be to get everything moved over to explicit rules via Pint and drop StyleCI. An additional benefit of this would be contributors could run Pint locally prior to submitting PRs as well.

I excluded the /tests directory to start just to keep it simple and easier to review, but would also like to add that back in eventually.

It looks like you're deferring to some CI stuff maybe stored at the organization level that I don't have access to, so I thought it might be best to leave that part to you guys.

[GrahamCampbell]

We can for sure make this possible inside of StyleCI. The idea is to have the laravel preset in StyleCI be the Laravel Framework CS standard.

[GrahamCampbell]

I think this is a much better approach than running both at once.,

[browner12]

• does StyleCI use "PHP-CS-Fixer" under the hood? so we would have access to all its rules?
• what's the benefit of sticking with StyleCI when we have a 1st party(ish) package that should be able to handle all our needs? and also let's people run it locally?

my only reason for running both at once, for a short time, would be to sync them up without causing too much turmoil. as stated, my end goal would be to get it fully moved over to Pint rather quickly.

[KennedyTedesco]

This is really nice for contributors. I often see myself getting style ci errors when creating new PRs. With this, contributors can run it before submitting their PRs.

[GrahamCampbell]

The idea is that it doesn't matter that StyleCI failed. It's fixes just get merged in with the PR, as it's merged.

[browner12]

The idea is that it doesn't matter that StyleCI failed. It's fixes just get merged in with the PR, as it's merged.

Which is also now possible with Github CI. So Pint and StyleCI are on level footing here. While not needed, it is nice to be able to run it locally before submitting a PR, and making sure everything passes.

https://laravel.com/docs/11.x/pint#running-tests-on-github-actions

[KennedyTedesco]

I get that, but I still think this is a good idea to have locally. Personally, I prefer committing with CS fixes applied beforehand.

[tontonsb]

I kinda like the hand-crafted indents to align stuff...

[crynobone]

I still don't think we should move to pint (just for framework repository) and there are still issues with running pint locally using none preset.

1. Running vendor/bin/pint affects all files, not just files you want to commit.
2. Running vendor/bin/pint --dirty can be used to prevent (1) but can be done only before git commit.
3. WIthout (2), after a PR has been merged, StyleCI would reformat the code and push to 11.x.
4. Next PR running (1), will revert changes made by StyleCI, send it PR that would include code format for files unrelated to the PR.
5. If PR from (4) get merged, StyleCI will restructure the code format again and the cycle continues.

Even with none preset style changes should only applied for "dirty" files, which would work fine if you using pre-commit https://git-scm.com/book/ms/v2/Customizing-Git-Git-Hooks and run vendor/bin/pint --dirty

[browner12]

thanks for the insight @crynobone.

couple of clarifying questions:

• why would Pint revert changes made by StyleCI if we only start with explicit rules that SCI doesn't currently cover?
• would switching fully over to Pint immediately avoid these issues?
• could we switch the onus of having properly formatted files to the contributor? I already fix all my style errors on PRs anyway 🤣 . with a tool like Pint it'd be very easy to say all PRs must pass styling requirements. then the CI would run pint --test and either fail or succeed. this is actually what we do on ALL of our projects. (probably more of a question for Taylor)

[crynobone]

why would Pint revert changes made by StyleCI if we only start with explicit rules that SCI doesn't currently cover?

StyleCI doesn't cover it because it was made to follow Taylor preferred style from the start.

I'm not talking about now, but mainly in the near future when pint is updated with new php-cs-fixer with new rules and defaults (wouldn't this be applied too even on empty preset)?

would switching fully over to Pint immediately avoid these issues?

It's still a maintainer nightmare, again I don't see this an issue for 1st party packages as we rarely get PRs but the framework (currently at 43 pending PRs) it a big task.

---

Again, StyleCI handles styling errors for us without thinking about formatting. We never reject PRs if CI only failed due to StyleCI pipeline.

[browner12]

the problem is not that I disagree with the styling choices made previously. the issue is that there are new scenarios that have arisen from either new PHP language changes or newly used patterns that StyleCI seems to be (currently) unable to address.

my understanding is the "empty" preset has 0 rules. therefore any updates to the Pint or PHP-CS-FIXER package would have no effect as we are defining our desired rules explicitly.

[crynobone]

therefore any updates to the Pint or PHP-CS-FIXER package would have no effect as we are defining our desired rules explicitly.

What happens when someone PR adds new rules to pint.json that conflict with StyleCI? Before we would just ignore failure for StyleCI but after this:

1. It returns failure on StyleCI
2. Maintainer needs to change how we handle StyleCI failure, or
3. StyleCI will reformat the code again after PR has been merged.
4. Another PR using the updated pint.json will reformat the code again.

All these will adds additional work to maintainers when it was never been an issue at the moment.

[crynobone]

the problem is not that I disagree with the styling choices made previously. the issue is that there are new scenarios that have arisen from either new PHP language changes or newly used patterns that StyleCI seems to be (currently) unable to address.

The only failure recently was the new nullable_type_declarations rules that need to be enabled manually on .styleci.yml. We had an internal discussion and I was under the assumption that StyleCI would add it as default but it did not happen (@GrahamCampbell argument on why it didn't become the default made sense).

Even with pint with empty preset, as you explained it wouldn't be apply without manually adding the rules to pint.json. For above issue can be solved by adding the rule to StyleCI configuration:

php:
  preset: laravel
  version: 8.2
+ enabled:
+   - nullable_type_declarations

[browner12]

I think it's incredibly important in any discussion to have a clear understanding of the problem so everyone is on the same page, so let me lay some things out that I believe to be true:

• consistent code styling is important for readability and maintainability, which can also lead to a lower barrier for contributors.
• as the PHP language changes, we need to consistently review and add or edit rules that we enforce for code styling.
• having 1 tool to enforce our rules is ideal. having 2 tools would only be used as an interim solution to facilitate the transition.
• StyleCI is (currently) unable to handle rules that we desire to add (or did they just add it)
• Pint/PHP-CS-Fix are open source tools, and Laravel repos could benefit from the transparency this provides.
• If Pint becomes the sole tool for styling we can configure it in a way that still requires zero manual effort from maintainers.