New option to align arrows (binary_operator_spaces)

[kenjis]

Feature request

I would like the option for the behavior in v3.13.0 binary_operator_spaces align_single_space_minimal for arrays.
Ref: #6590

config:

        'binary_operator_spaces' => [
            'default'   => 'single_space',
            'operators' => [
                '='  => 'align_single_space_minimal',
                '=>' => 'align_single_space_minimal',
                '||' => 'align_single_space_minimal',
                '.=' => 'align_single_space_minimal',
            ],
        ],

v3.13.0 result:

$a = [
    'a'  => 'a',
    'aa' => 'aa',
    // bra bra bra
    'aaa' => 'aaa',
];

v3.13.1 result:

$a = [
    'a'   => 'a',
    'aa'  => 'aa',
    // bra bra bra
    'aaa' => 'aaa',
];

[Gavrisimo]

I would really love to get that previous behaviour back... :(

[VincentLanglet]

This is a kinda tricky question.
In your example, the arrow are not fully aligned, and at least two issues were asking for the 3.13.1 behavior
#4395, #5546

Adding a new option was one of the three proposal but the issue is that it would make only sens for =>.
But maybe one solution would be to split the binary_operator_spaces rules.

The logic inside is already different:

• One method is computing everything related to the => operator
• One method is computing everything for all the other operator
  So the split could be done this way and the =>-related rule could have more option.

Also I don't find => in https://www.php.net/manual/en/language.operators.php, so I might be wrong but this is not really an "operator" and it would justify having this in another rule than a rule with the name "operator spaces".

Moreover a specific rule about => would also add the ability for having a different behavior for arrays, and for match.

[Gavrisimo]

I understand, but all of a sudden the rule that we use got changed and I've got hundreds of files that "should" be updated. We really liked how the rule worked prior to this change btw.

Is it possible to have multiple rules? One that supports the old behaviour and one that supports this new that you implemented?

[kenjis]

It is true that the previous behavior is not perfectly aligned, but that is preferable to us.
The behavior in 3.13.1 changes our code significantly.

We would appreciate having the option to provide 3.13.0 behavior.
Having another rule for => is no problem.

[Gavrisimo]

I wasn't able to reproduce the good behaviour by downgrading friendsofphp/php-cs-fixer but I was able to go back to desired functionality by downgrading laravel/pint so now I am a bit confused really.

More details here: laravel/pint#135

[FrittenKeeZ]

The 3.13.1 update also completely wrecked our Laravel config files that are well scoped having /* */ blocks between the array values. Now there's just unintentional indentation everywhere.

[llessa-godaddy]

+1 vote for giving us the option of the old behavior for => or reverting.

In addition to the OP report, I believe the new behavior is not working as intended for yield with arrays. It seems to align the internal fat arrows with the fat arrow on either the following or previous yield line, not deterministic.

Recent diff after running php-cs-fixer 3.13.1:

         yield 'null customer' => [
-            'expected'    => null,
-            'ourCustomer' => null,
+            'expected'             => null,
+            'ourCustomer'          => null,
         ];
         yield 'no underlying user' => [
-            'expected'    => null,
-            'ourCustomer' => Customer::seed(),
+            'expected'        => null,
+            'ourCustomer'     => Customer::seed(),
         ];

• array 1 is aligned with yield 2?
• array 2 is aligned with yield 1?

Configured array and binary_operator_spaces rules 👇🏽

        'array_indentation' => true,
        'array_syntax' => ['syntax' => 'short'],
        'binary_operator_spaces' => [
            'default' => 'single_space',
            'operators' => [
	            '=>' => 'align_single_space_minimal',
            ]
        ],

[kenjis]

I've tested, and it is not expected behavior.

v3.13.1 result:

function test()
{
    yield 'null customer' => [
        'expected'             => null,
        'ourCustomer'          => null,
    ];
    yield 'no underlying user' => [
        'expected'    => null,
        'ourCustomer' => Customer::seed(),
    ];
}

[VincentLanglet]

Please take a look at #6724 and give your opinion about it (especially the name of the options). It should satisfy everyone.

I've tested, and it is not expected behavior.

v3.13.1 result:

function test()
{
    yield 'null customer' => [
        'expected'             => null,
        'ourCustomer'          => null,
    ];
    yield 'no underlying user' => [
        'expected'    => null,
        'ourCustomer' => Customer::seed(),
    ];
}

Thanks for the report, it will also be fixed in the #6724 PR

[mc0de]

Functionality of v3.13.0 should be preserved, and new option added to correspond v3.13.1 style.

While this shouldn't be an issue for small snippets, but for config files where they are more likely to be spaced with newlines this change is crucial, and makes readability hell.

[VincentLanglet]

Please try the new 3.14 release and tell me if this is ok now.

[stayallive]

Thanks everyone, from my end this looks "fixed" (or back to how it was in 3.13.1) ❤️

[mc0de]

@VincentLanglet awesome, thanks