Extended PSR-2 Recommentation

[Christopher Pitt]

Hey errybody,

There's a PHP-FIG vote on at the moment. It's about whether PHP-FIG should work on a set of recommendations for new syntax introduced in PHP 7 and not already covered by PSR-2.

I'm writing here to tell you what my initial feelings about it are, and ask for some feedback before getting us involved. :)

I don't think the syntax has been in the wild for long enough for best (or even common) practises to become established yet. PHP 7 is only in RC mode, and there have been a rapid number of language syntax additions happening in the last 6 months. No popular frameworks are built only to work on PHP 7, which means they're all doing a mix of PHP 5/7 compatible code (and none of the new syntax). I think that it might be a good thing to do after some time has passed and the community has settled on how they think the new syntax should be formatted.

What do you think?

Kind regards

Chris

[Patrick Nelson]

Maybe I'm just being a bit thick, but PSR-2 (http://www.php-fig.org/psr/psr-2/) apparently defines coding style guidelines. Could you provide a link to (and/or explanation of) the syntax you're referring to for PHP 7 and how that potentially will overlap with PSR-2?

--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to silverstripe-d...@googlegroups.com.
To post to this group, send email to silverst...@googlegroups.com.
Visit this group at http://groups.google.com/group/silverstripe-dev.
For more options, visit https://groups.google.com/d/optout.

[Loz Calver]

Patrick, my understand is that PHP-FIG is voting on whether or not to introduce new recommendations for PHP 7 (and apparently 5.5 & 5.6, though I’m not sure which syntaxes they’re referring to there), not voting on what those specific recommendations would be. See https://groups.google.com/forum/#!topic/php-fig/P9atZLOcUBM.

I’d actually give this a +1. Deferring this until libraries have adopted PHP 7 could harm adoption of it: people don’t want to pollute their git history/blame with stuff like this, it’s often an argument against adopting PSR-1 or PSR-2.

Loz

[Stevie Mayhew]

I'm against it - PSR-1 and PSR-2 came from votes on rules which had been established across multiple vendors and best practices which came from use of the language. I feel that defining language rules without the language being used and those rules evolving is a bad idea.

We could end up in a situation where we adopt something that seems sensible but further down the line isn't quite right - PSR-1 and PSR-2 didn't have this problem as they had known usage.

[Sam Minnée]

I think that the general principle of PSRs following existing conventions rather than making up their own ones is good. I'd be inclined to give it 6-12 months to let some early-adopter projects do this.

So I'd suggest voting -1 for now, and making a statement that you'd be likely to vote +1 once PHP7 language features have seen a bit more uptake.

On Thursday, 27 August 2015 04:02:03 UTC+12, Loz Calver wrote:

[Daniel Hensby]

Whilst I think that Chris' (and other people that agree with his point of view) position is pragmatic and sensible, I feel that coding conventions based on what habbits people found themselves in after a bit of use, isn't necessarily the best approach.

I also don't see coding standards really being something that PHP-FIG should be concerned about - there are enough coding standards already and it doesn't directly affect interoperability.

Seeing as consensus is somewhat split, I'd recommend we abstain. It seems like the vote is going to pass either way.

Dan

[Cam Spiers]

The FIG proposal makes sense to me. Code formatting in the general case is a trivial and unimportant detail which as little time as possible should be spent on while still deriving the benefits of consistency (this is why I believe FIG should be involved in setting code style, it eases contribution across frameworks and will make shared framework dependencies not break the frameworks code style). As someone who adopted PSR-2 early on, the needless cognitive burden of varying codestyles it lifted was great, and any project I worked on (that includes you SilverStripe) that didn't use PSR-2 was an annoyance. People may be overblowing the syntax introductions between 5.3 and 7. Most of the RFCs between 5.3 and 7 don't impact syntax or have clear implications that I don't think there will be much argument over. I think it would be better if we resolve these harder cases through FIG before the community needlessly diverges, as it has before.

- Short arrays - Easy, needs little thought

- Generators - Medium, requires a bit of thought but will be easy to resolve

- Anonymous classes - Medium, but I can't see much issue cause PSR-2 already defines most of what we need here

- Spaceship operator - Easy, it's just an infix operator so it is already handled

- Return type declarations - Easy, needs little thought

- Null Coalesce - Easy

- Unicode Codepoint Escape Syntax - Easy

- Uniform Variable Syntax - Medium, I am unsure of all the codestyle implications of this change

- Abstract syntax tree - Easy - I don't think this impacts codestyle

- Remove alternative PHP tags - Easy - obvious effect

- Switch default multiple - Easy - obvious effect

- Group Use Declarations - Medium - can see some argument over this one

- Scalar type hints - Medium - can see argument over this one

- Exponential operator - Easy - infix

- Debug info magic method - Easy - covered by existing

- Importing namespaced function - Easy

- Constant scalar expressions - Medium

- Variadics and argument unpacking - Easy

- Finally keyword for exceptions - Easy

- ::class resolution - Easy - obvious

- Foreach supporting list() - Medium

- Const array/string dereference - Easy - Covered by uniform variable syntax I believe

- Binary int syntax - Easy

Cam

[Stephen McMahon]

I'm with Cam on this. The syntax introductions are easily solvable and it's considerably better to have a standard from the start even if it's the wrong one. Having everyone using the same rules from the get go makes it considerably easier to change them later on than having everyone doing it differently and then trying to merge them together later. Given that PSR's can't be amended I would agree that rushing to finalise one has risks, but getting in front of the ball would set a good example in the community and for PHP-FIG. 

[Conrad Dobbs]

+1, would be better to have it clear from the start. 

[Damian Mooyman]

Hey guys,

I think that given our supported module definition already includes psr-2 as a recommendation, and we are looking at a new major stable version in the next few months (4.x) now is the time to commit to either adopting or not adopting psr-2 standards.

Regardless of how this is implemented, we should look at confirming our decision on this issue soon, as it will affect any work we do on the framework over the next few months.

I personally would be keen on agreeing to adopt this standard, but not actually implementing this closer to the final release of 4.0.x.

As a part of this change we would need to change our silverstripe coding conventions to follow the psr-2 rules as outlined at http://www.php-fig.org/psr/psr-2/. This of course would cause havok with merging up from any 3.x branches into master, so the later this is left the better. :)

I'd like to put my support behind it, in order to bring our coding conventions closer in line with the wider PHP community.

I don't think that our support of PHP 7 should greatly affect the outcome of this, as we are unlikely to adopt any php-7 only syntax in core for quite some time.

Kind regards,

Damian Mooyman

[Damian Mooyman]

Something else I forgot to mention is that there are already a lot of great tools out there for automatic conversion. I'm using https://github.com/FriendsOfPHP/PHP-CS-Fixer in my development at the moment (and there are several plugins for various IDEs that integrate with this).

I also thought that we could use this as a tool for merging up, so that we can run this on (for instance) the 3 branch before merging it into master for a cleaner merge (although we don't actually commit those changes back to 3).

So no problem that is terribly difficult to solve. :)

On Wednesday, 26 August 2015 17:11:19 UTC+12, Christopher Pitt wrote:

[Sam Minnée]

To comply there are some things we're already doing, such as putting classes in namespaces. Aside from that, the two notable changes I can see are:

- Use spaces instead of tabs. This is annoying because it affects every line of of every file, but it at least it can be done automatically.

- Use lowerCamelCase() for static methods. Given that we had a lot of static lowerCamelCase methods changed to snake_case in 3.x to suit the SS standards, this would be a bit of an embarrassing flip-flop, but embarrassment aside. Note that we would probably preserve the snake_case methods as deprecated methods in 4.x, and remove them in 5.x, to ease the upgrade pain. So 4.x wouldn't be strictly PSR-2 complaint but would be on the way.

Thanks,

Sam

--

[Loz Calver]

Definite +1 from me.

I’ve used PHP-CS-Fixer before, and can also recommend PHP_CodeSniffer. 

Switching from tabs to spaces looks like it might pollute the blame view. Annoyingly Github doesn’t seem to support the ?w=0 flag on its blame pages (though CLI git does support it). As far as I can see, that and the merge pains are the only drawbacks to this.

I think it’s perfectly acceptable to have deprecated methods that don’t follow the conventions - we shouldn’t let coding style dictate upgrade pain after all.

Loz

[Nicolaas Thiemen Francken - Sunny Side Up]

are we talking about this:

https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md

then I'd vote: -1 

What exactly are we trying to fix?  Most of the time, most SS code and its modules, written by most developers is easy to read.  Having a bit of flexibility is better than one standard for all.  For example, I love the way we do 

private static $my_static_var = "";

Some of the other recommendations I don't like: 

• Code MUST use 4 spaces for indenting, not tabs.
  
• There MUST NOT be a hard limit on line length; the soft limit MUST be 120 characters; lines SHOULD be 80 characters or less.
  
• Opening braces for classes MUST go on the next line, and closing braces MUST go on the next line after the body.
  
• Opening braces for methods MUST go on the next line, and closing braces MUST go on the next line after the body.
  
• format recommendation for elseif, etc...
  

It seems that most of the other recommendations most of us have already adopted, most of the time. 

Again, I really wonder if there is any major advantage in adopting this.  Would this not, in a small way, limit innovation, personal expression, and "smart" formatting (using a formatting that  makes sense in a particular situation - i.e. the rules are too crude). 

Nicolaas

​

[Christopher Pitt]

Nicolaas,

What are the benefits to any code style? Consistent code styles reduce the cognitive overhead in code reviews, so errors can be easily spotted. We already have a code style, so I guess you could make your argument against having a code style at all, and not specifically against adopting/changing to a new code style. However, adopting PSR-2 as a code style means developers already familiar with this widely accepted style will not have any new code style to learn to be able to contribute to SilverStripe. 

Kind regards

Chris

[James Pluck]

On 4 November 2015 at 22:34, Loz Calver <kingl...@gmail.com> wrote:

Definite +1 from me.

Ditto +1 from me.  We've adopted PSR-2 as far as possible at work and it seems to be working well.  For existing codebases we are rolling it out on an as-touched basis.  All new work should comply and old work updated as it gets modified in the course of maintenance/enhancement.

 

Switching from tabs to spaces looks like it might pollute the blame view.

We had this issue as well.  It can be an annoyance but only once it's done.  After that it's back to normal.

 

I think it’s perfectly acceptable to have deprecated methods that don’t follow the conventions - we shouldn’t let coding style dictate upgrade pain after all.

Agreed. 

 

- Use lowerCamelCase() for static methods. Given that we had a lot of static lowerCamelCase methods changed to snake_case in 3.x to suit the SS standards, this would be a bit of an embarrassing flip-flop, but embarrassment aside. Note that we would probably preserve the snake_case methods as deprecated methods in 4.x, and remove them in 5.x, to ease the upgrade pain. So 4.x wouldn't be strictly PSR-2 complaint but would be on the way.

I like this staged approach.

 

Kind regards

James Pluck 

[Sam Minnée]

We already have a code style, so I guess you could make your argument against having a code style at all, and not specifically against adopting/changing to a new code style. However, adopting PSR-2 as a code style means developers already familiar with this widely accepted style will not have any new code style to learn to be able to contribute to SilverStripe. 

A big focus of our work with 4.0 is making SilverStripe a better member of the overall PHP ecosystem; adopting PSR-1 and PSR-2 supports that.

[Nicolaas Thiemen Francken - Sunny Side Up]

On 5 November 2015 at 09:19, Christopher Pitt <cgp...@gmail.com> wrote:

Nicolaas,

What are the benefits to any code style? Consistent code styles reduce the cognitive overhead in code reviews, so errors can be easily spotted.

​I agree - and it would be hard not to agree.  But what we have at the moment works fine for me. For example, changing tabs to spaces may conform with some norm, but I don't see the point as to me tabs make more sense than spaces in that they are much easier managed, much less likely to be screwed up, and more flexible 

(

  AFAIK, you use spaces for outlining stuff in code: 

  e.g.

  $myVar              = 10 

  $myLongVar      = 20

) 

To me adjusting to this norm is basically lowering our standards to a less desirable common denominator. 

 

We already have a code style, so I guess you could make your argument against having a code style at all, and not specifically against adopting/changing to a new code style. However, adopting PSR-2 as a code style means developers already familiar with this widely accepted style will not have any new code style to learn to be able to contribute to SilverStripe. 

​What we have at the moment is pretty good.  We have a recommendation that makes sense to me and that seems to be OK with most of us. ​

 

​Has anyone ever complained about them? 

​Thus, it is not so much that I disagree with the idea than that there are far bigger issues that need attention.  In future upgrades to 4.0 and 5.0 I dont want to focus on rewriting $my_var to $myVar but rather I'd love to focus on rewriting, for example, MyAnnoyingTinyMCEField to M​yAmazingHTMLEditorField.  It is not so say that we can not do both, but I am worried about too much new form and not enough new function.

​Nicolaas

[off...@netwerkstatt.at]

The biggest benefit from using standard PSR-2:

 

There are tons of tools to do the formatting work for you. Every good IDE has it built in.

 

So you can automate converting your stuff to PRS-2 on save, on commit, whatever.

 

But as it’s a matter of taste this discussion is pretty useless. Either you like it or not. But you cannot discuss about taste!

 

So I’m happy with PSR-2, as it would make it easier for me to format my code (automatically) to fit the standard.

 

I’m also happy to leave it as is, but I cannot guarantee that my code fits 100% the current standards, but I try my very best.

 

And with every refactoring you might find better names for your variables ;)

 

Cheers,

Werner

 

--

[Nicolaas Thiemen Francken - Sunny Side Up]

On 5 November 2015 at 20:57, <off...@netwerkstatt.at> wrote:

The biggest benefit from using standard PSR-2:

 

There are tons of tools to do the formatting work for you. Every good IDE has it built in.

​Do you need a tool to format your code? Does anyone have trouble reading your code if you do not use that tool? 

[Loz Calver]

I don’t think anyone needs tools to fix standards for them, but it’s helpful to have them. Many of the remaining code style issues in our current codebase would’ve been avoided had we already been using the tools that are available. You might argue that it’s down to the developer to ensure that code is consistent and readable, but automating minor things like this reduces the amount of work both contributors and core committers have to do to keep things that way.

The conventions we have at the moment don’t have any issues per se, but we haven’t been great at following those conventions - the tools available would fixing this much easier and help prevent issues from creeping back in.

The main benefits are the ones that Sam & Chris highlighted. We want SilverStripe to become a bigger & better part of the PHP community as a whole, part of that is making it as easy as possible for people to pick up our codebase and work with it: our current conventions are effectively another barrier (albeit minor) for new contributors. To quote the PSR-2 meta document: “the benefit of this guide is not in the rules themselves, but in the sharing of those rules”.

Loz

[Mark Guinn]

I’m for PSR-2 in general. I don’t love it as much visually and I really don’t look forward to dealing with the change, but all in all I’m a +1 on this.

I worked on a project a year or so ago where we had to modify the Omipay PayPal driver pretty heavily and submit those changes back as PR’s. Omnipay is PSR-2 and won’t accept and commits that violate it so I was constantly dealing with issues where PHP Storm would rewrite my omnipay files with tabs etc or rewrite my Silverstripe files with spaces. It was a constant annoyance. For those of us looking to actively contribute back to existing OSS projects in the course of Silverstripe projects I only see that scenario increasing if we don’t adopt the standard as well.

Mark

[Conrad Dobbs]

I got caught out submitting PSR-2 formatted code to a SilverStripe module, forgot to check my format settings.

It's like the NZ give way rule change, why be different than everyone else if there is no benefit? Especially where there are benefits to using the same standards.

[Shoaib Ali]

+1 vote from me.

I also have reservations and personal preferences and opinions about following PSR2 vs where SilverStripe deviates. However I do not wish express them, as that achieves nothing.

As I am all for standardisation and consistency. 

My 2 cents worth.

On Wednesday, 26 August 2015 17:11:19 UTC+12, Christopher Pitt wrote: