Php-cs-fixer: Require empty line after class definition and before it end

[erickskrauch]

The PHP version you are using ($ php -v):

PHP 7.1.8

PHP CS Fixer version you are using ($ php-cs-fixer -V):

PHP CS Fixer 2.11.1

The command you use to run PHP CS Fixer:

php-cs-fixer --diff --dry-run --stop-on-violation --using-cache=no -v fix

The configuration file you are using, if any:

<?php
$finder = Symfony\Component\Finder\Finder::create()
    ->in(['api', 'common', 'console'])
    ->notPath('*/runtime')
    ->name('*.php')
    ->ignoreDotFiles(true)
    ->ignoreVCS(true);

return PhpCsFixer\Config::create()
    ->setRules([
        '@PSR2' => true,
        'braces' => [
            'allow_single_line_closure' => false,
            'position_after_functions_and_oop_constructs' => 'same',
        ],
        'blank_line_before_statement' => true,
        'cast_spaces' => [
            'space' => 'none',
        ],
        'linebreak_after_opening_tag' => true,
        'no_whitespace_in_blank_line' => true,
        'non_printable_character' => [
            'use_escape_sequences_in_strings' => true,
        ],
        'ordered_imports' => true,
        'single_quote' => true,
        'whitespace_after_comma_in_array' => true,
        'trailing_comma_in_multiline_array' => true,
    ])
    ->setRiskyAllowed(true)
    ->setFinder($finder);

If applicable, please provide minimum samples of PHP code (as plain text, not screenshots):

• before running PHP CS Fixer (no changes):

<?php
namespace common\rbac;

final class Roles {

    public const ACCOUNTS_WEB_USER = 'accounts_web_user';

}

• with unexpected changes applied when running PHP CS Fixer:

   1) project-name/common/rbac/Roles.php (class_definition, braces)
      ---------- begin diff ----------
--- Original
+++ New
@@ @@
 <?php
 namespace common\rbac;

 final class Roles {
-
     public const ACCOUNTS_WEB_USER = 'accounts_web_user';
-
 }


      ----------- end diff -----------

• with the changes you expected instead:

No changes

---

I want to have one blank line after defining the class and before closing it:

class Test {

    // Any contents here

}

But braces and class_definition fixers do not allow to configure it. How can I suppress or configure this param?

[keradus]

I don't recall that we have such an option.

hm... could you please provide some details how you end up with given coding standard? is it somehow established in some community ?

[erickskrauch]

is it somehow established in some community?

Unfortunately, I tend to think that no. This code style is used only in my own projects. Currently, I use a PHPStorm feature to fix code style:

I want to have some solution, that I may include into our CI. For now, I only see a possibility to hard fork braces and class_definition fixers and describe to them this strange code style rules.

[erickskrauch]

@keradus, @juliendufresne I want to try to implement this and send the pull request. Could you give me recommendations for implementation? I'm not quite sure how to split responsibilities now. Should I create a separate fixer for this rule or implement it as a configuration parameter of the existing one? In addition, as I understand it, I'll need to explain braces and class_definition fixers that they should ignore empty lines at the beginning and at the end of the class because it will become a responsibility of a separate fixer.

[keradus]

sorry for late feedback, I was having a little break in real life.

[keradus]

For us it is important that fixers/rules/configuration either follow a standard like PSR (as we like to promote the usage/following of standards) or are widely used (as we are more likely to get support on the fixer when maintaining it).
I must admit that I do not only not follow it, but I didn't saw it in other repos as well.
Is this a widely followed standard, or just your own personal preference? Could you share some references, who is using it ?

[erickskrauch]

As I said before, this is not widely followed standard and it's mostly personal (our team) preference. I saw such code style in some repositories, that follows braces['position_after_functions_and_oop_constructs'] = 'same' to keep same whitespace around properties/methods. But right now I can't provide you any actual reference, except our own code.

But I understand your feelings about maintenance. If you think it's not usable fixer for others than we can just keep it in our custom set of fixers.

[keradus]

adding it in generic way would be tricky and I will leave decision to you ;)

First of all, we don't like to have rules that can do the same, even partially.
2nd, we don't like to have rules that do the reverse, causing conflicts (first run of PHP CS Fixer does the change with fixer A, 2nd run reverts those changes with fixer B).

For that, here we would need to determine what would be those fixers.
I don't see conflicts with braces nor class_definition. If I'm wrong on that, please prove it ;)

What is see is at least conflict with no_blank_lines_after_class_opening.
Your proposal is good in such way that it allow to configure how many of line can be there.

What I mainly see that to be done is to confirm which existing rule is conflicting, and then decide one by one what to do with them.

Then, for conflict with no_blank_lines_after_class_opening, IMHO we shall deprecate it in favor of new rule.
To to this, it shall become a proxy fixer, that would use new fixer with some concrete configuration, with which the functionality would remain the same.

[erickskrauch]

I don't see conflicts with braces nor class_definition. If I'm wrong on that, please prove it ;)

This issue began just with these fixers. I will check it.

// 10 mins later

I performed some tests and found, that ClassDefinitionFixer is removes my dearest empty lines. But for some reasons my fixer implementation won't work if have a priority higher then -25, which equals to BracesFixer.

Then, for conflict with no_blank_lines_after_class_opening, IMHO we shall deprecate it in favor of new rule.

Sure. I can deprecate it and rewrite as proxy to my fixer in PR.

[erickskrauch]

I think it's okay to have such behavior. ClassDefinitionFixer normalizes class definition according to widely used style and then my fixer (if enabled) will adjust it for some preferred amount of empty lines.

[keradus]

I performed some tests and found, that ClassDefinitionFixer is removes my dearest empty lines.

show concrete examples

[erickskrauch]

Another test. This time I've setted up a clean environment. Input file:

<?php
class Test {

    public function method() {
        // body
    }

}

Initial config:

<?php
$finder = PhpCsFixer\Finder::create()
    ->in(__DIR__);

return \PhpCsFixer\Config::create()
    ->setRules([
        'braces' => [
            'position_after_functions_and_oop_constructs' => 'same',
        ],
        'class_definition' => true,
        'class_attributes_separation' => true,
    ])
    ->setFinder($finder);

Result:

 <?php
 class Test {
-
     public function method() {
         // body
     }
-
 }

Then I got the same result for:

• ONLY braces.
• ONLY class_attributes_separation.

ONLY class_definition produced such output:

 <?php
-class Test {
+class Test
+{

     public function method() {
         // body
     }

 }

Looks like it's partially duplicating braces behavior about position_after_functions_and_oop_constructs.

---

At least, now I have an answer why my fixer must have less priority than BracesFixer.

[keradus]

that's the thing.
if "by design" new fixer gonna do the same as any other fixer, it won't be accepted, as they can be configured to follow different styleguide, thus those rules will be in conflict.
for that, if rule will do sth that other rule already does, but in more generic way, allows for config and so, it shall deprecate that part of already existing rule

[erickskrauch]

So, this is my suggestion:

• Add to the fixer next config params:

  • count_lines_before_class_body = 1;
  • count_lines_after_class_body = 1;
  • count_lines_before_anonymous_class_body = 1;
  • count_lines_after_anonymous_class_body = 1;

• Replace no_blank_lines_after_class_opening with a proxy to this fixer with all params set to zeroes.

If you agreed, then I'll rework my PR to complete this features.

P.S. maybe I should change some names?

[erickskrauch]

@keradus

[InvisibleSmiley]

I was just about to create a bug report for the behavior of the braces and class_attributes_separation rules as well as class_definition (like erickskrauch commented on 17 Apr). Those are not meant to enforce the removal of blank lines but currently they do.

The above mentioned rules must not dictate the amount of blank lines. They should be indifferent to whether there is one or not, be it at the beginning or end of a block. I can file a bug report for that if need be.

Otherwise I'd love to see this particular issue to be fixed, i.e. have the ability to require exactly one blank line at the start and end of a class body.

Personally I think it'd be best to first fix the existing rules and only then create a new one, but YMMV. Please let me know if I you'd like me to separate the two issues.

[erickskrauch]

@InvisibleSmiley I'm agreed with you. But I still have no response from the core team to have any solution about final implementation, that will satisfy everyone.

[esemlabel]

Currently there is no ability to add blank lines before/after class body, if position_after_functions_and_oop_constructs = same. Need fix.

[erickskrauch]

@esemlabel, you may try to use our custom implementation: https://github.com/elyby/php-code-style#elyblank_line_around_class_body

[esemlabel]

@erickskrauch, not my case, I'm using vscode extension.

[erickskrauch]

@esemlabel, but this extension can use the configuration from .php_cs file, so there is no difference between usage of PHP-CS-Fixer directly or via VSCode extension.

[esemlabel]

@erickskrauch, I'd like to have all clear and in one place, instead of installing additional fixes and remember to uninstall after php cs fixer updates.
Will wait..

[erickskrauch]

I'm waiting for any response for more than a year. Good luck :)

[smuuf]

This would be great.

[SpacePossum]

As this is not part of a standard or widely used in (F)OSS I prefer not that have this in the project to avoid the maintenance cost of the more complex code. IMHO this would be better fitted in a 3rd party custom fixer.

[erickskrauch]

Okay 😢