Php-cs-fixer: Multiline signatures fixer

[greg0ire]

Hey! This weekend, I wrote a blog post about multiline signatures, and I think it would be great to have a check for this, based on the line length, or @Soullivaneuh proposed based on the number of arguments (or both).

I can work on it unless someone else is very enthusastic about doing it themselves. What do you think?

[ostrolucky]

Whoever is going to code this might want to get inspiration from https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Standards/Generic/Sniffs/Files/LineLengthSniff.php

[keradus]

Sure, go for it !
yet, it will have to allow both approaches, enfore multiline and enforce singleline

ref https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/1980 , especially last comment

[julienfalque]

This would be an awesome feature, but I think it should be a fixer dedicated to reduce any line length, not only method signatures. Something like LineMaxLengthFixer that is able to chop down long lines when possible (arrays, method signatures, method calls, ...). Maybe the first version would support method signatures only, and other types of lines support would be added later. WDYT?

[greg0ire]

@julienfalque great idea, but I think this is contradictory with

yet, it will have to allow both approaches, enfore multiline and enforce singleline

I'm all for it though, because I like how this would be a way to enforce the soft limit described in PSR2 and make many warnings disappear. WDYT @keradus ?

It's also incompatible with @Soullivaneuh 's feature request (number of args vs number of chars)

[soullivaneuh]

IMHO, the multi-line signatures or arrays must have an argument option as I said in https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/1217#issuecomment-338436961

Please see also the eslint example.

BTW, we may add the max_length option but the max-length condition does not apply only for multi-lines signatures or arrays.

[julienfalque]

To be honest I don't like the idea of couting the number of arguments because you may have a signature with a few long parameters:

class Foo
{
    public function someVeryLongMethodName(SomeLongClassName $someLongParameterName, AnotherLongClassName $anotherLongParameterName)
    {
    }
}

or with a lot of short parameters:

class Foo
{
    public function getFoo(Bar $bar1, Bar $bar2, Bar $bar3, Bar $bar4, Bar $bar5, Bar $bar6)
    {
    }
}

In both examples, a fixer that simply counts the number of arguments to make a decision would probably be wrong (ignoring the first one while it should fix it, fixing the second while it should ignore it).

[greg0ire]

Here is a draft of how it might look if we accept @julienfalque 's proposal: https://github.com/greg0ire/PHP-CS-Fixer/commit/da28d7a32d918ed732eae03ca0590bddd796bf73

[soullivaneuh]

@julienfalque You took very extreme sample here.

Both options should be proposed in this case:

• Elements count for consistency (not having lines-broke method on some place, and not elsewhere).
• Max length for such extreme cases.

In this case, it will fill all needs.

@greg0ire BTW, we need the first option for sonata.

Also, I'm not sure "line length fixer" is the best name to have, because a line length is a global constraint, not a way to fix.

How about, based on eslint rule, ElementNewLineFixer with such options?

• element choices between (array_element, signature_argument, call_argument, anything else?)
• min_items: The minimum items needed to break the line. false or null if you don't want this option.
• max_length: The maximum length needed to break the line. false or null if you don't want this option.
• Also maybe: inclusive: If true, both min_items and max_length must be respected to break the line. But I'm not sure about this one.

[greg0ire]

LengthyLineFixer could be a better name than LineLengthFixer. I don't like ElementNewLineFixer, it's meaningless to me.

Also, I fail to see how the number of items should play a role in deciding whether to linebreak or not. Not sure why eslint decided to implement that.

[soullivaneuh]

I don't like ElementNewLineFixer, it's meaningless to me.

Length based fixer name is meaningless if we propose both options.

Also, I fail to see how the number of items should play a role in deciding whether to linebreak or not.

As some don't see why method should be line broken at all. AFAIK, php-cs-fixer is here to provide fixer that you can use (or not) and configure as you wish and do/should not decide how the code should be presented.

Not sure why eslint decided to implement that.

Certainly by feature request, has it's done here. Feel free to make a PR to eslint. :trollface:

[ostrolucky]

Hey everybody, this fixer is available at https://github.com/Symplify/CodingStandard#arguments-should-be-on-the-samestandalone-line-to-fit-line-length (Symplify\CodingStandard\Fixer\LineLength\BreakMethodArgumentsFixer)

cc @TomasVotruba

[nicolas-grekas]

On Symfony, we only want single line signatures, whatever the line length.
It'd save us some annoying review comments if php-cs-fixer could enforce it for us.
:+1: for what it's worth :)

[keradus]

Can you raise a PR, @nicolas-grekas ?

[nicolas-grekas]

Nope sorry I need help also.

[julienfalque]

@nicolas-grekas Since version 2.12, the method_argument_space rule has a on_multiline option that can be set to ensure_single_line.

[keradus]

@julienfalque , that means this issue is outdated and we can close it already ?

[julienfalque]

No, this issue is about implementing a rule that chooses between single line or multi line function signature (and maybe other things) depending on the line lenght.

[nicolas-grekas]

@julienfalque thanks for the hint. I tried it on the symfony code base, and it also changes function calls, no only signatures. Only signatures should be forced on a single line. Is there any way to achieve this?

[julienfalque]

@nicolas-grekas I don't think this is possible at the moment. Please submit a new feature request.

[k00ni]

Any update on this?