phpBB Extension Coding Standard

[Senky]

Hi guys,
I always liked the Coding Guidelines phpBB provides - the code is nice and clean all the time. But unlike the core, Coding Guidelines aren't strictly forced for extensions. I, however, believe that maintaining the same formatting for extensions make it easier for others to read them, contribute to them and (from the real experience) validate them. This is the reason why I created phpBB Coding Standard.

phpBB Coding Standard is a standard for PHP_CodeSniffer, making sure all your code complies with Coding Guidelines. As for now, it is not yet complete. It only features a collection of already existent sniffs (plus snake_case validators), while JS and CSS is not yet covered at all. I will be creating more sniffs in the future, but the standard is already usable.

Usage is very simple:

• composer require --dev senky/phpbb-coding-standard "dev-master"
• Create a file phpcs.xml.dist in your ext root directory. Typical contents should be:

  Code: Select all

  <?xml version="1.0"?>
  <ruleset>
  	<file>.</file>
  	<exclude-pattern>./build</exclude-pattern>
  	<exclude-pattern>./tests</exclude-pattern>
  	<exclude-pattern>./vendor</exclude-pattern>
  
  	<rule ref="vendor/senky/phpbb-coding-standard/ruleset.xml"/>
  </ruleset>
  

• Then run ./vendor/bin/phpcs
• Optionally run the sniffer in your Travis builds:

  Code: Select all

    - sh -c "if [ '$SNIFF' != '0' ]; then phpBB/vendor/bin/phpcs phpBB/ext/$EXTNAME/; fi"

The standard is already in usage in my Akismet anti-spam extension.

I welcome any suggestions.

[david63]

Coding Guidelines aren't required for extensions

Since when?

As far as I am aware all extensions are required to conform to phpBB's coding guidelines and whilst minor deviations would not by themselves make an extension deny worthy a complete disregard for them would.

[Senky]

I had a discussion yesterday with Paul, concluded that only really badly readable code is denied. Anyway, you are right - it is required, but not forced. I updated the description above.

[david63]

I agree that it has never been "forced" on extension developers.

I am not sure how you can force some aspects anyway. How can you deny camel case when virtually all libraries use it? If an extension developer is using variables from a library then they have to be in the case that that library is written in.

I would further argue that the coding guidelines are somewhat dated and should be brought more in line with current working practices - but that is probably a discussion for another time and another place.

In my opinion any "checking" of code conformity should take place in the EPV - you have to remember that the majority of extension developers do so as a "hobby" and are not professional programmers. We need to be very careful as to what is imposed on extension developers for fear of loosing them as there are enough factors looming whereby they may cease developing.

[Senky]

In my opinion any "checking" of code conformity should take place in the EPV - you have to remember that the majority of extension developers do so as a "hobby" and are not professional programmers. We need to be very careful as to what is imposed on extension developers for fear of loosing them as there are enough factors looming whereby they may cease developing.

That is exactly why it is not part of EPV. Even seeing 100s of notices about coding guidelines would scare the users. That's why I released it independently so only experienced developers can apply it.

[MattF]

How is this any different than the code sniffing phpBB already has to make sure extensions adhere to all the same strict coding guidelines as any phpBB code submitted to github, on travis when - SNIFF="1" in .travis.yml?

[Senky]

There are additional checks such as snake_case names. But the biggest advantage is that it is easily usable on your local PC.

[david63]

That's why I released it independently so only experienced developers can apply it.

But surely that is not the problem. "Experienced" developers will, generally, fall into two categories:

1. Those experienced in phpBB development who know the rules and, in all probability, will not need their code checking.

2. Those who are experienced developers but not necessarily with phpBB who will code in their own preferred style and take no notice of the coding guidelines.

[Gwyneth Llewelyn]

Sorry for bumping a 5-year-old thread, but I just wanted to profusely thank @Senky for the Code Sniffer rules

In fact, I was a bit confused why these were not much more visible and available, but required several searches with different engines (!) and even some AI searching (!!!) until I finally stumbled upon this thread...

Anyway, here's some thoughts...

• All open-source projects have their coding standards and practices, and this is especially true of long-running projects such as phpBB. Providing the guidelines is a must, and they have been made public long ago.
• Volunteers may not have time to make sure they know all the rules by heart. In that aspect, I slightly disagree with @david63 — even experienced phpBB developers might forget the odd rule or two, possibly because the 'old' rule used in 2009 has been replaced by a 'new' rule in 2015... and unless your day job is maintaining the phpBB code, you might simply not be able to catch up with the changes over the decades.
• Thus, automated tools. These benefit developers of all kinds — newbies, veterans, experts in phpBB, experts in anything else but phpBB, those who are stronger on JavaScript than PHP, those that are stronger in PHP than JavaScript, those (like myself) that have no 'strength' in any particular programming language, etc. and so forth. All benefit from automated tools at some level.
• While the core developers prefer to work with PHP Storm, that's just an option for a few. If, like myself, you're not stuck to a single programming language, the last thing you want is to learn Yet Another IDE. There are far too many already. Some are free, some are cheap, some are neither. Whatever the reason, forcing volunteer developers to stick to a single IDE just for contributing code for one project is not a good solution. Rather, what works best are so-called 'universal' rules — which work with utilities that are not tied to specific IDEs or code editors. This is the province of things such as .editorconfig, PHP Code Sniffer, PHP Mess Detector, and their ilk. Then it's up to each developer to integrate such tools in their own favourite code editor or IDE, but, these days, that's the easy bit!
• As such, while I appreciate the work that was done with rule configuration files for PHP Storm, these are useless on any other platform/IDE/code editor. They're not even useful if you use GitHub's built-in CI tools — and while I agree that some things should be done locally and not remotely ("clean up your mess before you submit it!"), that's not an excuse. AFAIK, for instance, there are no free solutions out there which integrate GitHub Actions with rules from PHP Storm — while you have a gazillion (well, I use three different sets) that support PHP Code Sniffer directly. Which means that if you use PHP Code Sniffer locally, and you have it correctly configured, you won't need to worry with what happens remotely on GitHub.

While I can agree with @david63 that "experienced developers [...] will take no notice of the coding guidelines" — something I have repeatedly seen over and over again, in many different open-source projects, so there is certainly a measure of truth in that! — I think that the answer is not ignoring what "experienced developers" think about the rules (they all might have excellent reasons for doing X instead of Y). Rather, it's much better to give them properly formatted/configured tools for checking their code. If those developers are not only experienced but also professional, they will stick to rules, because that's what they're used to do in their day job.

Just beware of the well-intended, very experienced but amateur developer!

Anyway... That said, I'd love to see a page on the official documentation for extension developers with a list of official (in the sense of: rule-complying) configurations for at least the following tools:

.editorconfig — because even PHP Storm uses it!
.gitignore — mentioned here and there in the docs but rarely with a full recommended configuration that includes PHP, JS, HTML, CSS, SVG...
eslint.config.js — just because it's the most common JavaScript linter out there
phpcs.xml — for properly configuring PHP Code Sniffer (i.e., @Senky's package!)
GitHub workflows (which are already included, I'm just listing them here for completion's sake)
Optionally, PHP Mess Detector configuration (although the defaults are good enough, IMHO)

Sadly, there is a huge legacy of code in the core phpBB which is two decades old, and coding conventions have subtly changed over the years. Interestingly, the guidelines for JavaScript are pretty much contemporary (see, for instance, the usage of brackets and camelCase for most identifiers), while those for PHP are reminiscent of the olden days when WordPress was a blogging platform (WordPress being a good example of a vastly popular and well-known mainstream application which uses a hopelessly outdated and hard-to-read coding style — worse even than phpBB, IMHO! — but also strictly sticks to it, for exactly the same reason as phpBB: legacy code...)

Then again, I'm pretty sure that 'code styling' is the kind of thing that cannot be put to the vote; it has to be strictly enforced from above, because, otherwise, you'd spend years to endlessly discuss if there should be trailing commas or not, or if Yoda comparisons are better or worse, and so forth, and so on, and no actual code will be written So... I tend to agree with the overall concept: "these are the rules if you want to contribute code, stick to them; if you disagree, you're welcome to fork the code and rewrite it as you wish, but don't expect us to use any of it". That is, IMHO, the only way to guarantee code uniformity among a vast variety of developers with different backgrounds and personal styling tastes.

[As a side-note: some years ago, I was curious enough to learn Go — I love strictly-typed compiled languages — and, not surprisingly, the Google devs in charge of that project — who happened to be inventors of C and Perl, decades ago — also had to deal with the differing styles of writing code. So, they came up with one specific style, and force the compiler to apply it. There are a few ways to avoid the "default style" (by mistake or ignorance), which are separately caught by a built-in code-formatting tool. The result? There are no "coding standards" to discuss. All code is written using the same style. Most repos don't even accept code deliberately written in a non-standard way, or simply run a reformatting tool to make the code 100% compatible with the one and only single standard. End of discussion. If you don't like the strictness of those rules, you're welcome to go back to C or try Rust instead...]