October: Make the OctoberCMS codebase PSR-2 compatible

[florianstancioiu]

OctoberCMS seems to mostly use the PSR-2 coding standard, but the namespace keyword is positioned incorrectly in every single class file. Please make the code base PSR-2 compatible, this will enforce a sensible standard on everyone coding on OctoberCMS. PSR-2 is the defacto coding style standard for PHP and it makes a lot of sense to use it.

The enforcement of PSR-2 can be done really fast, there's a couple of scripts available for automatic conversion of code (without bugs). The most noteworthy of them is PHP-CS-Fixer.

[bennothommo]

@florianstancioiu Thank you for reporting this, however, October CMS is compliant with the PSR-2 guidelines. The PSR-2 guidelines make no mention of any requirement of placement of a namespace declaration, only that an empty line is placed after it. See the following section of the PSR-2 guidelines:

3. Namespace and Use Declarations

When present, there MUST be one blank line after the namespace declaration.

When present, all use declarations MUST go after the namespace declaration.

There MUST be one use keyword per declaration.

There MUST be one blank line after the use block.

You'll also note that their PSR-12 guidelines, which are supposed to supersede the PSR-2 guidelines, place the namespace line the same way we do.

We also automatically detect violations of PSR-2 with PHPCS here: https://github.com/octobercms/october/blob/develop/phpcs.xml.

Thanks for your interest in October!

[florianstancioiu]

The examples of neither PSR-2 nor PSR-12 shows the code being exactly like you guys write it.

The PSR-2 documentation doesn't mention the starting tag: <?php should be alone on its line, but the example shows just that.

In PSR-12, they mention that explicitly:

When the opening

Laravel started to do the same since version 5.1 which came out five years ago.

[LukeTowers]

@florianstancioiu then what you really are asking for is strict adherence to PSR-12.

We're not going to change this. We are currently matching the PSR-2 standard which does not define where the namespace statement is put, and I personally think having it on the same line is the cleanest look and can't stand the separate line.

Unless you can demonstrate a tangible benefit from changing it then we're not going to consider it. Feel free to do whatever you want in your own plugins.

[bennothommo]

@florianstancioiu please consider the following pseudo-code before making a big deal about this:

if (php parses regardless of empty spacing around namespace) then
  allGood()
else
  submitPRtoResolveIssue()
end if

[florianstancioiu]

@LukeTowers The reason why I'm asking for this is because I'm used to the way Laravel classes are written, that's why I'm using this CMS in the first place. Whenever I create a new file using the built-in artisan tools, my OCD kicks in and I add a new line to each class and this ain't the kind of thing I should waste my time on. Updating the code to Laravel's form of PSR-2 will make for a better experience for people coming from Laravel, which I think is the majority of developers using this software. Plus it will make the code base future proof.

So, there are 2 benefits of doing this: a better experience for Laravel devs and future proof compatibility.

[LukeTowers]

@florianstancioiu changing this does not make the code future proof, all it does is make devs coming straight from Laravel very slightly more at home with the coding style. We optimize for the October developer experience, not Laravel's.

We're always willing to do things that would make it a nicer experience for Laravel devs but not at the expense of October devs. Should you become very involved with the October community and code base then I'd be more inclined to weigh your preferences when making this decision, but as of right now we're not going to change it.