Php-cs-fixer: Ensure Blank Line After Class Opening Fixer

[wucdbm]

Hello Everyone,

I needed a custom Fixer that is specific to my own, opinionated code format configuration that would ensure a blank line after class opening braces, since I like to keep the opening brace at the same line.

https://github.com/wucdbm/php-cs-fixers

Would you guys consider it a good idea to include that fixer in the core library and possibly extracting the private function detectIndent(Tokens $tokens, $index) method from BracesFixer into any of the helpers / utility classes, since I also needed it and it did a perfect job streamlining and minimizing the required code?

PS Oh, and somebody would have to help with the tests as I still feel I have no clue what I am doing :)

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

=> 7.1.11

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

=> PHP CS Fixer 2.8.3 Round Numbers by Fabien Potencier and Dariusz Ruminski

The command you use to run PHP CS Fixer:

=>./bin/php-cs-fixer

The configuration file you are using, if any:

<?php

$finder = PhpCsFixer\Finder::create()
    ->in(__DIR__ . '/src');

return PhpCsFixer\Config::create()
    ->registerCustomFixers([
        new \Wucdbm\PhpCsFixer\Fixer\EnsureBlankLineAfterClassOpeningFixer()
    ])
    ->setRules([
        '@Symfony'                                     => true,
        'no_blank_lines_after_class_opening'           => false,
        'Wucdbm/ensure_blank_line_after_class_opening' => true,
        'array_syntax'                                 => [
            'syntax' => 'short'
        ],
        'braces'                                       => [
            'position_after_functions_and_oop_constructs' => 'same'
        ]
    ])
    ->setUsingCache(false)
    ->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 Logger\Logger;

if (!class_exists('Logger\Logger\Test')) {
    class Test {/** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {     /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {



        /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {



          /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {



      /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {



        /** @var EntityReferenceHelper */
        private $referenceHelper;



    }
}

if (!class_exists('Logger\Logger\Test')) {
    if (!class_exists('Logger\Logger\Test')) {
        class Test {


            /** @var EntityReferenceHelper */
            private $referenceHelper;
        }
    }
}

if (!class_exists('Logger\Logger\Test')) {
    if (!class_exists('Logger\Logger\Test')) {
        class Test {/** @var EntityReferenceHelper */
            private $referenceHelper;
        }
    }
}

• with unexpected changes applied when running PHP CS Fixer:

// Everything works great

• with the changes you expected instead:

<?php

namespace Logger\Logger;

if (!class_exists('Logger\Logger\Test')) {
    class Test {

        /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {

        /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {

        /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {

        /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {

        /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    class Test {

        /** @var EntityReferenceHelper */
        private $referenceHelper;
    }
}

if (!class_exists('Logger\Logger\Test')) {
    if (!class_exists('Logger\Logger\Test')) {
        class Test {

            /** @var EntityReferenceHelper */
            private $referenceHelper;
        }
    }
}

if (!class_exists('Logger\Logger\Test')) {
    if (!class_exists('Logger\Logger\Test')) {
        class Test {

            /** @var EntityReferenceHelper */
            private $referenceHelper;
        }
    }
}

[SpacePossum]

Hi @wucdbm and thanks for your questions,

IIUC you've two questions for us, I'll try to answer both from my POV.

The first one is to add an configuration option to the "class opening fixer" so it will assure a single blank line after the opening brace. As you stated it is your preference (it was mine as well for a long time) but not necessary one that is popular in an (F)OSS community.
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). Therefore I would not recommend adding it to the project.
However, you've a great custom fixer that you made available to the public so that is awesome :D

Than the second question is about changing the private method detectIndent from the BracesFixer to public. Public functions means we would have to keep a BC promise on those and therefore we would loose some flexibility. If this logic is to become a public method I would prefer it to be on a class dedicated to providing such logic and not the BracesFixer. I would like to get more input from other members on that however.

[keradus]

If this logic is to become a public method I would prefer it to be on a class dedicated to providing such logic and not the BracesFixer.

I would say even more, try to add any public method outside of the interface, and our meta-tests will fail :heart:

[keradus]

Fully :+1: about class opening fixer.

About detectIndent, it would have to be a dedicated class indeed, yet it would be an internal class. What does it mean for you? You would be able to use that class inside your project, just we won't give a BC promise for it (we need to cleanup Tokenizer subnamespace first).
That would be a great change indeed, could you please propose a PR @wucdbm ?

[wucdbm]

@keradus no, not really, I definitely won't have any time outside of my daily schedule and I barely find any to support several other libraries I've released

Lately I've been thinking it's a much better idea to just provide a class that extends PhpCsFixer\Config and offers a pre-defined configuration, which you could also create yourself and collect any other custom fixers you may need as dependencies and you have a 2-3 liner config for your .php_cs

By the way, the fixer now has tests, after I noticed several edge cases, so I had to make sure it does alright

Hope it helps you :)

@SpacePossum thumbs up for a separate class

[keradus]

OK, so...
This feature request didn't got community support. Combining that with your declaration that you cannot support it ATM, I don't think it shall land in official repo, at least for now.

about config - nothing new, ppl are writing custom config classes since years ;)

[wucdbm]

Alright, sounds great.

In that case, to whom it may concern, my opinionated stuff can be found at https://github.com/wucdbm/php-cs-fixers/blob/master/src/Config/ConfigFactory.php

If I get some slack time I'll get back here and separate detectIndent, then update my own library as well once it gets merged

[keradus]

Closing due to discussion