minor: [BinaryOperatorFixer] Deprecate single_space option in favor of single_space_minimal

[VincentLanglet]

Related to https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/4396

single_space currently means "Exactly one space" align_single_space means "At least one space" aligne_single_space_minimal means "Exactly one space"

I think we should rename single_space to single_space_minimal.

In 4.0, single_space can be newly implemented to at least one space.

[coveralls]

Coverage increased (+0.0004%) to 92.837% when pulling b2208b2b3ba8c186b0236166eef711417df9c0f6 on VincentLanglet:singleSpaceMiniaml into 17e55913427ee15e73f5f7001c025ecc83f09ff7 on FriendsOfPHP:master.

[SpacePossum]

I think we should rename single_space to single_space_minimal.

I don't think so, I think single_space_minimal would mean: in:

$a =$b&&$c; 
$d =$a   &&    $e;

out:

$a =$b && $c;
$d =$a   &&    $e; 

and not out:

$a =$b && $c;
$d =$a && $e; 

this is also discussed here: https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/4497

besides, if we are going to rename option I think should be properly deprecated so it will be the docs and triggers warning if still using the old configuration value

[VincentLanglet]

I think we should rename single_space to single_space_minimal.

I don't think so, I think single_space_minimal would mean: in:

$a =$b&&$c; 
$d =$a   &&    $e;

out:

$a =$b && $c;
$d =$a   &&    $e; 

If we do this, this will "conflict" with the definition for align_single_space and align_single_space_minimal. You can't say that single_space_minimal means at least 1 space when align_single_space_minimal means align with exactly 1 space.

	Exactly one space	At least one space
Aligned	align_single_space_minimal	align_single_space
Not aligned	single_space	single_space_minimal

would be confusing

We could say that align_single_space should have been exactly one space but changing this does not allow to provide a BC upgrade path... A solution could be to rename the align_ options to align_with_

Something like

	Exactly one space	At least one space
Aligned	align_with_single_space	align_with_single_space_or_more
Not aligned	single_space	single_space_or_more

The upgrade path for user (helped by deprecation message) would be

• Changing align_single_space_minimal to align_with_single_space
• Changing align_single_space to align_with_single_space_or_more And we could introduce single_space_or_more in this version without waiting for 4.x.

WDYT ?

besides, if we are going to rename option I think should be properly deprecated so it will be the docs and triggers warning if still using the old configuration value

Sure I can, I'll wait for the right name to be decided

[SpacePossum]

On 4.x I would go with just two options: align and one_space, where align is always minimal spacing and of at least one space on the left of the operator and always one space on the right of it. All other options are just there for BC reasons AFAIK.

Not sure about the upgrade path here, like to hear some thoughts on it.

[VincentLanglet]

On 4.x I would go with just two options: align and one_space, where align is always minimal spacing and of at least one space on the left of the operator and always one space on the right of it. All other options are just there for BC reasons AFAIK.

So basically you want to only propose align which is the current align_single_space_minimal one_space which it the current single_space option.

For the align, there is the following tricky subject, how do you align

$a = [
    1 => [
    ],
    1111111111111 => 2
];

Because currently, it's not aligned but, people which aligns => are generally expecting

$a = [
    1             => [
    ],
    1111111111111 => 2
];

This is the current reason I have for using the align_single_space and not align_single_space_minimal option.

For the one_space, looking at comment like https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/4396#issuecomment-519911680, it seems like people doesn't want every time to restrict to strictly one space.

So, so far, I would say there are still reason for 4 options.

[SpacePossum]

I would expect

$a = [
    1             => [
    ],
    1111111111111 => 2
];

as it is still:

• minimal one space, like 2
• the => [ is aligned with the one below
• there is one space after the =>'s

would there be use case for another option than? I never align stuff, so I might be overseeing this here, but I think it would be better to reduce the amount of options as four is a bit much to understand IMHO

[VincentLanglet]

I would expect

$a = [
    1             => [
    ],
    1111111111111 => 2
];

as it is still:

• minimal one space, like 2
• the => [ is aligned with the one below
• there is one space after the =>'s

I expect the same :) Currently it's not fixed because the => are not on consecutive lines, and the BinaryOperatorFixer only align groups by groups. Example, the following code with align_single_space is untouched:

$a = 1;
$b = 2;

$aaa = 1;
$bbb = 1;
```

I created https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/6590 to discuss about this special case.

> would there be use case for another option than? I never align stuff, so I might be overseeing this here, but I think it would be better to reduce the amount of options as four is a bit much to understand IMHO

I agree that less option are better.

Also, maybe the right solution would be to split the rule.
- BinaryOperatorFixer would just check that there is a space before and after every operator. (The execution might be faster than the current rule)
- AlignmentBinaryOperatorFixer would be executed after BinaryOperatorFixer and would align the binary operators.

[VincentLanglet]

@julienfalque @keradus WDYT ?