Php-cs-fixer: class_definition fixer erroneously re-formats valid line

[Bilge]

The PHP version you are using: 5.6.18
PHP CS Fixer version you are using: 2.1.1

The command you use to run the fixer (and the configuration file you are using if any).

php-cs-fixer "--allow-risky=yes" "--rules={"@Symfony":true,"array_syntax":{"syntax":"short"},"blank_line_after_opening_tag":false,"blank_line_before_return":false,"cast_spaces":false,"combine_consecutive_unsets":true,"concat_space":{"spacing":"one"},"dir_constant":true,"ereg_to_preg":true,"heredoc_to_nowdoc":true,"linebreak_after_opening_tag":true,"modernize_types_casting":true,"new_with_braces":false,"no_extra_consecutive_blank_lines":["continue","curly_brace_block","extra","parenthesis_brace_block","square_brace_block","throw","use","useTrait"],"no_php4_constructor":true,"no_unreachable_default_argument_value":true,"no_useless_else":true,"ordered_class_elements":["use_trait","constant","property","construct","destruct","magic","phpunit","method"],"ordered_imports":true,"protected_to_private":true,"psr0":true,"psr4":true,"single_blank_line_before_namespace":false,"strict_comparison":true,"strict_param":true,"ternary_to_null_coalescing":true,"php_unit_strict":true,"phpdoc_add_missing_param_annotation":{"only_untyped":false},"phpdoc_align":false,"phpdoc_annotation_without_dot":false,"phpdoc_no_empty_return":false,"phpdoc_separation":false,"phpdoc_summary":false}" "--using-cache=yes" "--verbose" --dry-run --diff fix Module.php

A minimal sample of valid PHP code that is not fixed correctly (if applicable).

final class Module implements
    ConfigProviderInterface,
    ConsoleUsageProviderInterface,
    ServiceProviderInterface,
    ControllerProviderInterface
{
}

The fixed version of that code after you run the fixer and the version you expected.

final class Module implements ConfigProviderInterface, ConsoleUsageProviderInterface, ServiceProviderInterface, ControllerProviderInterface
{
}

The original code does not violate PSR-2 but this PSR-2 fixer decides to place the entire class definition on one line, which exceeds the 120-character margin.

[keradus]

You asked about that change when you decided to run @Symfony ruleset. This change is made from @Symfony ruleset, not @PSR2 ruleset

[Bilge]

@keradus I do not understand what you are talking about. I know the class_definition fixer will run, and I want it to run; that is not the issue. The issue is that it modifies the file in a situation where it should not. This is not a question, it is a bug.

[SpacePossum]

class_definition is configured by the Symfony set to make the change it did.
If you want the class_definition fixer to not do this you must reconfigure it after you implicitly configured it by using the Symfony set.

[SpacePossum]

for ref.: https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/2.1/src/RuleSet.php#L66

[Bilge]

Oh, I see, the @Smyfony rule set modifies the fixer's defaults.

[keradus]

for ref: reasoning is available also without going into code:

λ php php-cs-fixer describe @Symfony | grep class_definition -A2
λ php php-cs-fixer describe class_definition

[Bilge]

@SpacePossum Thanks, I was able to disable the singleLine option and the behavior returns to the PSR-2 style that I wanted. :+1:

As an aside, it's strange that the class_definition options are named using camelCase when all other fixer options I've seen use underscores as a word separator.

[SpacePossum]

Good to hear your issue is resolved :+1:

On the configuration naming, yeah that's on me. When added we were still much trying to figure out how configuration could be best added to the system. Naming may not be done consistent, good thing to keep an eye on when new items are added.

[Bilge]

I don't think it's necessary to blame anyone, but would it be worth including underscore-separated names and deprecating the camelCase names for consistency?

By the way, I only understand the singleLine option. I could not figure out what the other two options are supposed to do, even after studying the example diffs. Are those documented somewhere in more detail?

[SpacePossum]

The options are not documented at another place, we try to provide all info through;

$ php-cs-fixer describe class_definition

I can give some more info on those;
singleItemSingleLine tells the fixer that if a class, interface or trait only extends or implements one (and only one) item, to make the definition one line;
for example;

<?php
class A extends
B
{
}

would be made one line, if it would extend another class it would not.

multiLineExtendsEachSingleLine acts the same as the default for multi line implements.

<?php
    interface Bar extends
    Bar, BarBaz, FooBarBaz

is changed to

    <?php
    interface Bar extends
       Bar,
       BarBaz,
       FooBarBaz

This case is not default behavior because it is not defined in PSR1/2 (prop. forgotten by the authors, but still)

[Bilge]

Thanks to your explanation, I now understand what these options do. I had read the output from the describe command, but it only shows a diff. Sometimes just a diff isn't enough to understand what a fixer option does. I think it would be better to decorate the diff with a bit of explanation, such as you just provided. Do you agree?

[julienfalque]

Same for me: I didn't understand the meaning of the options, so thanks for the explanation @SpacePossum!

👍 for adding more documentation about these options and using snake_case instead of camelCase.

Maybe we should rename them as well?

[SpacePossum]

Descriptions can be improved by adding more details to the FixerDefinition as returned by the fixer(s). Better docs are always welcome :)

On the rename of the configuration items, please check this is the only fixer that is off (I can do it later). If so it might be a nice improvement (for next minor, not next patch).

[Bilge]

I have created a new issue (#2579) to track this discussion.