Switch to PSR-2 as of Drupal 9

+1 to this.

I am in favor of this, and of adopting PSR-12 when it comes out. I've used PSR-2 for several generic PHP packages, and while it is different, the good tool support makes it as easy to work with as our current standards.

As PSR-12 proves, the PHP community at large actively tries to maintain its standards to keep up with recent PHP versions. As we have had difficulties keeping our own standards up to date, I feel switching to PSR-* will mean more valuable contributor time can be spent on ensuring our documentation code style meets the standards, rather than that our standards meet reality developing the standards themselves.

I find PSR-2/12's placing of braces rather ugly, but that of course isn't really an argument. Consistency on the other hand, is, and if most of the libraries we use follow PSR-2, then I guess in the long run it will be easier for everyone if we switch, even if the switch itself will be painful.
However, which of our libraries does actually follow PSR-2? I mostly look at PHPUnit of all our libraries, and that seems to use more of a custom "Let's see what mood I'm in today" code style.
If most of our libraries do follow it, though, this would be +1 from me as well. (And if we'll wait for Drupal 9 until switching, I guess libraries that already have fixed plans to switch in a future version would count the same.)

Also, do we really need this "[policy, no patch]" tag in every single issue here? As far as I see, there is never a patch in this issue queue – if anything, there would be one in the Drupal Core issue queue once a decision is reached here (if that ever happens).

Just added the link to make it clear for those of us who can't remember which PSR's are which.

I find PSR-2/12's placing of braces rather ugly, but that of course isn't really an argument. Consistency on the other hand, is, and if most of the libraries we use follow PSR-2, then I guess in the long run it will be easier for everyone if we switch, even if the switch itself will be painful.

For what it's worth, I agree. Maybe it's just habit, but I don't like all of PSR-2 either. It's mostly preference, and not based on anything factual. I feel the consistency with other projects and the decreased workload on our side are worth the switch.

Making the switch right before 9.0.x is opened, is definitely the least disruptive path, because it will result in the least amount of large, broken patches. What about making the switch right before we open up 8.2 or 8.3, though?

adding related #2571965: [meta] Fix PHP coding standards in core, stage 1 which is blocked by drupal CI (testbots) being able to do automatic coding standards checking.

oops. guess we dont want that tag on every issue in the coding standards project. :)

Oh dear...

The "be like everyone else" argument is compelling, especially for those of us that now have to switch between different modes in our IDEs. Honestly, I could probably get used to every part of PSR-2 except for the class/function braces, which are just plain dumb.

That said, I do think that, should PSR-12 be adopted in the near future, we should adopt the pieces of PSR-12 that we don't already have a standard for. That is mostly PHP 7-related stuff (and a little bit regarding a file header), so it should have no impact on our current code, but as people start using PHP 7 functionality in contrib at least those bits will be consistent with what will, presumably, get adopted elsewhere.

Our general policy has been to avoid adopting standards that large swaths of our Core/contrib code would immediately be in violation of. We generally instead adopt standards that put into writing our common practice, or that choose the most generally used variant in cases where not having a standard has led to an unstandardized mess. I think that is a good policy. Which would mean we should not adopt PSR-2.

However, one thing that we could do, potentially, is adopt *most* of it, with a statement like:

-----
The Drupal project coding standards comply with PSR-whatever, with the following exceptions:
- Indentation: We use 2 spaces of indentation, no tabs.
- Braces: All opening braces for functions, methods, and classes go on the function/method/class declaration line.
- ...

We also have the following additional coding standards:
....
---------

And by the way, someone above said something about documentation standards. PSR-2 does not talk about this at all -- it is only standards for the code itself. PSR-5 is the documentation header standard. It has not yet been adopted by PHP-FIG as a standard, as it is still under (rather contentious) development and discussion.

I fear the PSR-2/Drupal difference list would be larger than the sameness list. There's a lot of differences.

And by the way, someone above said something about documentation standards.

That someone occasionally talks nonsense when they're tired. Please excuse them.

"The Drupal project coding standards comply with PSR-whatever, with the following exceptions:"

Then they do not comply with any PSR and we should not give even the slightest indication that they might.

Well if it's that much of a shift, my vote is that we continue to maintain standards that are "Drupal project status quo", as we always have, and not try to adopt "Someone else's idea of what the standards should be".

I understand, but I disagree.

I am trying to understand the arguments against switching from our current coding standards, but I'm not sure I do yet. It seems the two arguments are

1. It is disruptive
2. People do not like PSR-2(/PSR-12)

Is that the case, or is there more?

I think those are the two main arguments. The "disruptive" part is why we haven't adopted a LOT of coding standards that might seem to be a Good Idea. If we discontinue that policy I think you will see many more contentious standards discussions. Many of them get closed with "Yeah, sure, great idea but we in practice don't do that so ...".

Closing standards discussions is a Good Thing. We cannot and will not ever agree on aesthetics of code. It just can't happen, everyone has different taste...

I think it's not just disruptive, but immensely disruptive. Switching to PSR-[1]2 would touch pretty much every single line of code in the entire code base. It's on /core-mageddon levels of impact.

That's true regardless of how "good" or "bad" those standards in particular are.

That said, as acknowledged the task is large enough that it would only be scriptable. Basically someone would sit down, run phpcs on the entire code tree, and commit the result. The mechanics of doing so are actually pretty easy. It's the fallout afterward that is troublesome. :-)

That said, to #17 our code base size has resulted in us being more and more reluctant to adjust coding standards over time. I'm not suggesting we should change willy nilly, but with the level of automated tooling available these days we shouldn't assume that every change we make is going to require 500 people to file 5000 patches and have them manually reviewed. The transitions don't have to be quite that messy, even on the highly-active core.

Is there anything historical that we can look at to see why we landed at our current standards, and why they deviate in places from the PEAR coding standards?

And related, is there an accompanying rationale document for PSR-2/12 to explain the decisions?

IOW, I think the best place to start is to examine why we landed were we are today with the coding standards.

@crell, the PSR-2 brace placement for functions goes all the way back to K&R C, so there is 35+ years of precedent there. Coming from the C world and a K&R adherent (and having worked with non-ANSI compilers), the Drupal convention (and not using cuddles elses) took me a *very* long time to break.

Most of the PSR-2 decisions came from a survey of projects at the time, and a lowest-common-denominator approach. (That was one of the reasons that as the Drupal representative I voted against it.) I don't recall there being a great deal of logic put into it beyond that, although that could in large part be just my biased memory several years later.

PSR-12 is in discussion as we speak on the FIG list, with some arguing that we should do nothing for a year and see what people start doing, and others arguing we should nip any differences in the bud before practices diverge. Most of the additions to date are, I believe, based on consistency with PSR-2 and looking for natural, logical extensions to it. (The degree to which that is successfully applied is a subjective question, currently being debated. :-) )

I think it's not just disruptive, but immensely disruptive. Switching to PSR-[1]2 would touch pretty much every single line of code in the entire code base. It's on /core-mageddon levels of impact.

If we prepare for this and make sure we can do an automated conversion at the same time as we open the 9.x-branch, I don't think the fallout of it will be that big. I doubt there'll be 500 patches open that should be included for 9.x that wouldn't need a reroll otherwise.

In that light, I think we should go for PSR-12, we have the time to wait for that discussion to end; even if the FIG decides to wait for another year.

I don't think this discussion has completely considered the disruption this would cause. It's not as simple as "run a script, commit, done", even for coding standards changes that are scriptable. I can think of three considerations here:

a) If we only convert 9.x, this will be very disruptive for backports. In practice, many issues/patches in a given Core branch are for bug fixes, and as such they are candidates for backporting. So, a lot of patches will need to get ported back to the previous version. This becomes very non-trivial if the code is significantly different, stylistically, between the two branches. Normally in the first months after a major branch starts, backporting is relatively easy (until the architecture really diverges); this would make it very difficult from the start.

b) If there are patches already made, attached to issues, they would immediately not apply. This is also very disruptive.

c) And then you also need to think about contrib. Would you run the scripts on contrib as well, and if so, which branches of which projects? The same concerns apply there.

So. When we evaluate whether a given patch should go into a project, we think about gains vs. disruption. The disruption factor here is large. What, really, are the gains again? Let's get both, in detail, into the issue summary.

This is "leave core and go back to contrib forever" levels of terrible.
There's a reason we didn't adopt PSR-2.

Then again, we could come up with a list of overrides/exceptions to PSR-[1]2 and say "follow that except where it's stupid about these things"...

Given the number of differences, it would be more practical to say "if our coding standards don't specify, defer to PSR-2/12". In practice that wouldn't apply to much at all except for the new PHP 7 bits of PSR-12.

A few points on my own, won't repeat what's already said:

1. What's PSR-12? https://github.com/php-fig/fig-standards/tree/master/accepted it is not here. Where should I look? Edit: ah, on second reading of the issue, I read "PSR-12 is in discussion". OK, then there's nothing to adopt yet.
2. The PHP constants true, false, and null MUST be in lower case. this is downright confusing. Constants should be upper case. This goes back decades before PHP existed. K&R (!) already has uppercase constants (and lowercase otherwise). Also, see http://stackoverflow.com/a/838963/308851 this answer about why C++/Java constants are uppercase.
3. The brace placement is not consistent: while, do while, for, foreach loops, if conditions, catch blocks have their braces on the same line, but functions and classes have it on the next line. Irregardless which convention you like, choosing one convention would've better.

All in all, PSR-2 is a confusing and inconsistent standard I'd rather not adopt.

As someone writing and integrating libraries, I just want us to move to camelCase: #2411911: [policy, no patch] snake_case in coding standards makes consistency interoperability hard.
Our current mix of snake_case and camelCase is an inconsistent mess.
Similar issue is #2474561: Coding standard: inconsistent requirement that arguments to setters methods should be $lower_case when private variable and setter methods are camelCase().

If programming history has taught us anything, it's that arguing over brace style is unproductive.
If Drupal adopts PSR-2, it won't be because of what's inside, but because of what it represents (a way to be a good citizen and do what everyone else is doing instead of maintaining our own).

> a way to be a good citizen and do what everyone else is doing instead of maintaining our own

This argument really should be put to rest already. We have seen at least with a) the Doctrine annotation readers dance (where they have two, the one we use will be removed and we need a mix of the two) b) the Symfony Event Dispatcher (of which by now an interface remains in use by D8 as we rewrote it to scale ) c) the Symfony MIME guesser (which D8 simply can't use) that just because someone else wrote it and maintains it doesn't mean at all it's a good fit for Drupal. The fact we do not need to maintain a certain piece of code is certainly an advantage but nothing more than that. Overlooking the (severe) shortcomings of code written by others and using them just because it is written by others caused a lot of problems for Drupal 8 already, let's not make the same mistake with coding standards.

@chx
You're free to contest it and I won't fight it. I just want my camelCase. That's the real interoperability issue.

That needs to be a different issue IMO: switch to using camelcase for local variables not just properties. I see the motivation but "our general policy has been to avoid adopting standards that large swaths of our Core/contrib code would immediately be in violation of." of course policies can change but this is definitely a different and even more difficult issue.

Also, PSR-12 should be split as it doesn't exist yet. And this issue is not difficult: I believe @tim.plunkett and @jhodgdon already raised enough concerns (not to mention mine) to won't fix this.

Our general policy has been to avoid adopting standards that large swaths of our Core/contrib code would immediately be in violation of. We generally instead adopt standards that put into writing our common practice, or that choose the most generally used variant in cases where not having a standard has led to an unstandardized mess. I think that is a good policy. Which would mean we should not adopt PSR-2.

jhodgdon: Honest question: My read of that means that once a convention is in place, since our codebase is large, we can never change ANYTHING EVER because it would require changing things. What constitutes a "large swath"?

I fear that approach, taken as is, would lead to "we can never change anything, even if there is overwhelming evidence that it would be a good idea." That's never been our position for code (as witnessed by Drupal 8), so I don't see why it should be for coding standards. Changes should be based on the balance of evidence pro/con, not on "no BC breaks ever".

I'm not saying we cannot ever change our coding standards, but in practice, that is what I have observed: we have almost never been able to reach agreement on adopting coding standards that completely change the details of how we write code (brackets, spaces, and stuff like that), or that most of our current code would be in violation of. And we generally don't adopt new coding standards that aren't widely agreed upon by the community, so I do not think this is likely to change as long as we still have a community-agreement procedure for adopting standards.

And on this particular change... I am not really seeing the merits of the "Pro" arguments. What is so compelling about PSR-2 (which seems to many of us to be internally inconsistent, with respect to bracket placement for instance)? What would really make us want to incur all the pain it would cause? Are other projects that we might want to collaborate with really following PSR-2, so that the "we want to be able to share code easier" argument even carries any weight?

And in any case... we put our 3rd-party code under "vendor" and don't apply our coding standards to it... I really just don't see why having the same coding standards as other projects matters at all. Don't you think other projects do the same? We don't have the same standards as Symfony or Composer now... if we adopted PSR-2 would we then and would it even matter? We don't even edit their code... I just don't get it.

OK this discussion reached its conclusion I believe. Let's accept #2648050: [policy, no patch] Stop disallowing camelCase for local variables / parameters and move on.

And on this particular change... I am not really seeing the merits of the "Pro" arguments. What is so compelling about PSR-2 (which seems to many of us to be internally inconsistent, with respect to bracket placement for instance)? What would really make us want to incur all the pain it would cause?

I personally don't like PSR-2 either and I don't think there are any compelling technical reasons to switch.

Are other projects that we might want to collaborate with really following PSR-2, so that the "we want to be able to share code easier" argument even carries any weight?

Well, all the symfony, zend and doctrine packages, guzzle, composer, twig we use in core follow PSR-2. Those are only the ones I know about, I'm not sure about the other core dependencies - but those are the big bulk of our dependencies. And we have submitted patches upstream for those before, and I assume we will continue to do so.

And in any case... we put our 3rd-party code under "vendor" and don't apply our coding standards to it... I really just don't see why having the same coding standards as other projects matters at all. Don't you think other projects do the same? We don't have the same standards as Symfony or Composer now... if we adopted PSR-2 would we then and would it even matter?

As mentioned above, if we adopted PSR-2, the coding standards would be the same.

We don't even edit their code... I just don't get it.

We have submitted patches upstream for a ton of our dependencies and that behavior should be encouraged. So we do edit the code.
Another point when editing or looking trough external code is when stepping trough with a debugger. It happens that you go into external code and back into drupal code as well, which is when it hurts the most, it's hard enough to follow complex code, let alone having to deal with code being formatted differently.

One thing that isn't mentioned yet in this issue is the example of developers at an agency that does more than just drupal.
This is higly anecdotal but I think it has merit anyway: At my previous employer I spent a period of ~7 months working on a symfony project for 4/5 days, the other day I was doing bugfixes on a Drupal 7 project. The first 2+ hours of each context-switch was spent heavily relying on PhpStorms autoformatting function and seeing code jump all over the place because I was still typing the wrong standards.

As @bojanz mentioned in #29, if we switch it won't be because of what's inside. I don't really like PSR-2 either but I like having consistent code.

I think if we only go by pro/con and the impact this has on the code, I have to agree with #39 and we should just move on. I don't think it that easy and we should reconsider.

> We have submitted patches upstream for a ton of our dependencies

of which almost all of were rejected.

> and that behavior should be encouraged.

I certainly won't waste my time going back begging to accept my code. Encourage upstream first.

−1 because PSR-2 is inconsistent and opinion based without any logic behind it.

Ran into this issue while working on some of the standards related issues. How is this conflict going to be resolved? It would be great to build some consensus on this and take this to a formal conclusion.

PSR are only recommendations, you can follow them or not.. but I think is easier follow their standards.

When we get libraries through composer they usually follow PSR2, if we need for some reason do a patch for them they probably are gonna require follow PSR2, why should we work with two different standards? and what benefits provides Drupal Standard to stick at it? the fact of maintain the module drupal/coder make even worse follow this way...

The style of your code has not impact on interoperability. Switching a coding style should also be an easy task for any and everyone. Or do you use PSR-2 in other languages you program with too? Java? JavaScript? Rust?

Fleshgrinder yes, is easy but can you argue why is better keep both? I guess because is so easy you can say a lot of good and brilliant arguments to convince me.

Drupal is not keeping both, Drupal is keeping one, their own. The cost of the change is simply not worth it. Also note that there is already a new PSR coming up for this topic, and there are probably many others to come. It is a serious investment for an entity (Drupal in this case) to invest into something external, since that external thing might implement changes that are disruptive and require a potentially huge investment. For Symfony it was easy, since PSR-2 was 100% compatible to what they already did. Let's see how this will be with future incarnations of the PSR CSs, because Symfony's own CS grew out of hands a long time ago.

On a side note, the biggest problem with all major PHP CSs is currently that they are inconsistent, this includes the Drupal CS. Writing a pretty printer that requires to keep state and context around is a huge pain in the a**. A CS that is based purely on consistency would make it easier to provide tools, and for human beings.

I just want to agree that Drupal should keep it's own standard and never change. The code looks so good and is so much more readable than PSR-2. PHP Fig are the ones who should at least consider changing their brace placements and control structures. Thanks @Fleshgrinder for confirming that.

If the decision to use Symfony was made, perhaps Drupal should try to honour the that decision in completion. Different standards are just inconvenient, and to be honest, serve no bigger purpose, aside from the personal agenda of the Drupal team or someone in that team.

It is a shame that such issues even deserve commenting and take up so much of everyone's time - perhaps the folks who make these decisions and work on justifying them can spend their time deciding how help those who can't get 3 meals a day!

#49 so according to your message the whole PHP community should change their code style in favour of one small part of the PHP community?

Change the code style is not as hard as everyone here seems to fear... for instance if you run phpcbf will fix a lot of things automatically.

But if people wants to keep this barrier without any other argument than is great, maybe is because there is no reason to keep it.

I don't want to cause any arguments. Personally, as a front end and back end developer I like Drupal standards because they work for HTLM, CSS, JS and PHP. Plus YAML is 2 spaces. So it works for YAML as well. It's not just about the PHP standards. Drupal sets a good high standard for all languages. Drupal is leading the way in setting a high standard for all languages. We can't be locked in to different standards for every language.

Hi Rick,

The statement that 'Drupal is leading the way...' is purely subjective, as is much of the discussion. All I can say is that almost half of the US didn't vote for Trump, but all of the US is subject to his picking a fight with, say, North Korea. Almost half of the UK did not vote for Brexit, but all of the UK will have to honour it.

The same can (and probably should) apply to PSRs.

But as Saphyel said, this is not necessarily hard. We don't even have to honour 'Drupal' standards. No one is stopping us from running phpcs and phpcbf against the Symfony standard in a Drupal project.

On another note, this just goes to show why the democratic process only leads to chaos or, at best, a scattered approach.

Rick: YAML is "consistent spaces". It works with 4 spaces just fine. It even runs with different code blocks indented different amounts, although that's generally a bad idea. Most of our docs and examples at Platform.sh use 4-space YAML, as does all our Python code.

Fair enough. I have learned something new now! Thanks for clarifying.

Big +1 on moving to PSR. The current code style is not friendly to the biggest part of the PHP community which keeps Drupal isolated on their own island. Major versions are exactly meant for things like these.

The only thing i have against the PSR is the use of spaces instead of tabs but the benefits of using this standard as opposed to others is far greater than my personal opinion. With that said, for all the reasons already cited and because i said "please": please reconsider this issue as the different coding convention is one of the greatest time sinks i've stumbled in years.

At the very least, provide comparisons to the PSR and PEAR Coding Standards in a very prominent spot at the documentation so we can at least reuse some of our previous knowledge. (#2991140)

Thanks! :)

About:

Lowercasing the true, false and null, constants flies against more than 40 years of practice of practically every C descendant / inspired language.

I agreed at the beginning but I'm now dubious:

https://stackoverflow.com/a/3807178/1941316

and:

JavaScript booleans are lowercase and case-sensitive.

[...] it's VERY common in web programming to write both PHP and JS code, well developed web development finger muscles continue to routinely apply the same letter case used in the last language context. At least I often found myself writing TRUE or FALSE in Javascript after switching from PHP. Using lower case in PHP fixed this one for good.