[RFC] Dropping our own code styles in favour of PSR-12

[rdeutz]

It still takes but it already took years to update our code styles to use PHPCS2.0 (we are still on 1.5.whatever). With PHP7 and other new PHP versions new concepts are added to PHP and we need to find out how this can be integrated in our code styles. This will also take time to discuss and implement.

The PHP - Framework Interop Group. (FIG) is working on a PHP Standards Recommendation for code styles (PSR-12) https://github.com/php-fig/fig-standards/blob/master/proposed/extended-coding-style-guide.md as replacement for the old PSR-2.

So there are people already discussing these topics and how to integrate new PHP language features.

I think we should not reinvent the wheel here and just drop our own code style and use PSR-12.

Pros:

• We could concentrate on developing a CMS rather then discuss code style issues
• We could use a set for PHPCS3.0 and use automatic fixing for code styles
• Developers joining us can use the well know code style and don't have to learn a new one.

Cons:

• We drop a lot of work in the trash can
• It is not OUR code style

[810]

PSR-12 will not be on codesniffer 2.0 but on 3.0, So we should skip the 2.0 version

see: squizlabs/PHP_CodeSniffer#1309

[rdeutz]

@810 ah ok, updated RFC

[zero-24]

Do we really want If structures like this:

        if ($a === $b) {
            bar();
        } elseif ($a > $b) {
            $foo->bar($arg1);
        } else {
            BazClass::bar($arg2, $arg3);
        }

This looks unreadable for me. I need to look twice or more times to understand the code as everything is in one line. (maybe i'm the only one)

IIRC there is no reason in case of speed or whatever to do this, maybe someone can correct me.

And please remember that we do not allways have php devs working on our code. Please also think about the designers who using our code in ther templates etc.

Blank lines MAY be added to improve readability and to indicate related blocks of code except where explictly forbidden.

Sounds not that good for me :( So we dont be alloed add clean lines in our code. Like we currently do a clean line befor the if etc.

[mbabker]

PSR-2/12 does not explicitly forbid a blank line before control structures as we currently enforce in our code, they do allow you to add that requirement if you want.

[yvesh]

There are a lot of advantages in using a common code style.. And we are in need of new contributors, so this is definitely something to consider.

Personally i have no issue with reading structures like Tobias posted (Except that i really don't like any Else), but it's still possible to have blank lines before control structures.

[dgrammatiko]

This looks unreadable for me. I need to look twice or more times to understand the code as everything is in one line. (maybe i'm the only one)

Actually this is more like javascript and IMHO that's a good thing because at some point all the web will be js... 😃

[Bakual]

Imho, if the codestyle can be applied automatically, then go for it. We will get used to the new style and the automatic correction is the bigger advantage here.
If we have to change the code manually to apply to the new rules, then you can trash the proposal. I don't want to go through another round of codestyle PRs. We don't even have yet the current rules enforced everywhere.

[mbabker]

@zero-24 FWIW this screenshot comes out of a Symfony build I'm working on right now. It is fully PSR-2 compliant and doesn't sacrifice readability.

It's also pretty consistent with the JavaScript code style we're using in this same project.

[rdeutz]

Readability has also to do with how you code, the style is one aspect but not the big one I think.

[dgrammatiko]

@mbabker i see vanilla becomes your favourite ice cream taste 😜

[mbabker]

Not really. There are a lot of functions in that file I changed from jQuery to Vanilla as an exercise to see how painful it was 😉

[frankmayer]

I too, am for this proposal. Automatic codestyle fixing should of course also be implemented.

[photodude]

We are nearing a public alpha release of our code style using PHPCS 2 that includes autofixers for our custom sniffs. We also eliminated many of the custom sniffs that did nothing but duplicated included sniffs in PHPCS (see the new ruleset https://github.com/joomla/coding-standards/blob/phpcs-2/Joomla/ruleset.xml ).

Check out the current branch for the PHPCS2 version of the standard https://github.com/joomla/coding-standards/tree/phpcs-2

and the current list of issues to address/review prior to the public release joomla/coding-standards#143

as for PSR-2 vs PSR-12 I think we should adopt what we can as PHPCS implements PSR-12 support

[rdeutz]

@photodude I know you have spent a lot of time on updating the code style to PHPCS 2, but even if we have them it will not solve the problem that this needs constant attention and working on it. Past experiences have shown that we don't have the resources for keeping it up to date.

Using something all are using will make our life easier

[photodude]

@rdeutz Our custom sniffs, tab indenting messages, and odd ordering/spacing in docblocks tags are the only reasons updating to PHPCS2 has taken so long. If all we had was a custom ruleset using the standards included in PHPCS we could have already been running on phpcs2.

The same goes for moving to the future PHPCS3 release. If we didn't have custom sniffs to update due to the BC breaks we could switch to phpcs 3 on the day of release. (note: we have completed the phpcs 3.x updates which just involved namespacing the code, phpcs3.x could be used for any project with a minimum php of 5.4)

If we get rid of all of our non-typical rules that resulted in custom sniffs we would make something far simpler and make all of our lives easier. If we find something that there is something we would really like to have as a code style rule, rather than making a custom sniff, just contribute that to PHPCS and hope for inclusion.

Additionally, what about code style rules that PSR-2 and/or PSR-12 do not cover, How are we going handle those? I would suggest that we would implement just a custom ruleset and we pick from the rules included in PHPCS to cover those edge cases and eliminate all of the custom sniffs.

In anycase, there really isn't much left to do to complete the phpcs 2 version of the code standards. I feel that project needs to be released while we work towards updating our code standards for transitioning to PSR-2/PSR-12 plus any additional rules that need to be included.

[brianteeman]

Does this need to remain open?

[rdeutz]

All arguments are shared. The fact that it is now 10 months later and we still running our code style checks on 1.5 can be some kind of sign that the basic idea was not so bad.

[photodude]

Just a note on 2.x ruleset progress.
The framework is getting close to having all repos running on the 2.x ruleset.
I have a custom CMS 2.x ruleset nearing completion.

As mentioned above I'm all for eliminating the custom sniffs; that is part of the heavy lifting is that has held back updating to PHPCS 2.x. The transition to PHPCS 3.x for J4 will be much faster as the namespacing has already been done.

[brianteeman]

so can i close this RFC

[rdeutz]

I will add it to the next production team meeting agenda, for now we can close it

[brianteeman]

thanks

[photodude]

I think it's important to mention that PHPCS does not currently support PSR-12.
They do support most of PSR-2 (if not the whole standard).