Add rule: `yoda_style` to Laravel

[lucasmichot]

PHP-CS-Fixer Rule: yoda_style

This PR enfoce non-yoda-style

[driesvints]

Can you give some examples?

[driesvints]

Or even better: add some tests :)

[lucasmichot]

-        if (null === $this->input->getOption('format')) {
+        if ($this->input->getOption('format') === null) {
             $this->progress->subscribe();
         }

[jasonmccreary]

I'm not sure if it's a "bug" as much as my lack of understanding. So I am asking it here...

After upgrading today, the following conditional was changed.

-if (count($failed_tasks) === (count($tasks) - intval(isset($tasks['composer/install'])))) {
+if ((count($tasks) - intval(isset($tasks['composer/install']))) === count($failed_tasks)) {

I didn't think the existing conditional was a "yoda style" conditional. At least not a traditional one. It seems the 'always_move_variable' => true is the culprit. When I set it to false, I don't have any "violations".

I'm sure this change works at inverting "yoda style" conditionals, but my concern would be it swaps all conditionals. Ideally, it would leave non-yoda conditionals alone.

[wouterrutgers]

Some more examples where I don't think it should be changed:

- if (! $referrer || $request->path() == $pattern) {
+ if (! $referrer || $pattern == $request->path()) {

- return $this->guard === $guard && (int) $this->user_id === $userId;
+ return $this->guard === $guard && $userId === (int) $this->user_id;

- Http::assertSent(fn (Request $request) => $request->url() === "https://testapi.multisafepay.com/v1/json/orders/{$payment->hash}/");
+ Http::assertSent(fn (Request $request) => "https://testapi.multisafepay.com/v1/json/orders/{$payment->hash}/" === $request->url());

- return array_values($array) !== $array;
+ return $array !== array_values($array);

[liamja]

I agree with @jasonmccreary - always_move_variable seems to actually introduce Yoda-style (for want of a better term) in places and should be set to false

always_move_variable¶ Weather variables should always be on non assignable side when applying Yoda style. (but we don't want to apply it!)

Allowed types: bool

Default value: false

Can we get this set to false?

[nunomaduro]

Fixed: https://github.com/laravel/pint/releases/tag/v1.13.1. Next time create an issue folks, because this type of discussions get lost with the other bazillion GitHub notifications we have.

[maxacarvalho]

I rarely add some criticism over open source projects since I believe in the hard work of the maintainers and respect their dedication.
But, I'm allowing myself to speak out my opinion about recent changes around the Laravel Pint package. I really hope that the maintainers don't get upset about it, this is supposed to be a friendly comment.

The above said. I recently had to do a lot of digging in order to understand why my style tests were failing or why my project changes were adding a lot of code style changes. It turns out that all come down to Laravel Pint recent additions.
Some of them were applying PHP CS Fixer default values but others, like this one, are not. The change in this PR overwrites the default PHP CS Rule value, always_move_variable, which is false.
I know that there are quite a lot of discussion around yoda conditions but, in my humble opinion, the advantages outweigh the disadvantages.

Yes, it is quite easy to overwrite the setting by adding an entry to my pint.json file but, it's not clear why those changes are being introduced. I can see from the discussion above that there wasn't any reasoning around it, did I miss something?