PHPCS fixes based on PHPCS-2

[mbabker]

PHPCS v2 is generally a bit more strict and a bit better at catching errors. While testing the Joomla Coding Standard with that version of PHPCS, the tracker throws a lot of errors on the v2 code base. All of these are things that aren't really covered in the code standard explicitly but at least make our code consistent with how we handle things. There's still a lot of stuff that we either need to address in the PHPCS sniffs, but this covers a good chunk of the logical stuff.

[b2z]

How do I test it ? :)

[photodude]

@b2z , you could use the current development version if you want to test the PHPCS-2 version of the Joomla coding standards (thanks to @mbabker for the work on fixing the custom sniffs that couldn't be directly replaced with an existing standards rule)
https://github.com/photodude/coding-standards/tree/phpcs-2

[b2z]

@photodude thanks! @mbabker does it mean that we are now sticking to PHPCS-2?

[mbabker]

Not yet. Eventually when we get the project's ruleset stabilized on
PHPCS-2 we'll migrate up to it (as should the rest of the project's code
repos).

On Sat, Jul 18, 2015 at 12:20 AM, Dmitry Rekun notifications@github.com
wrote:

@photodude https://github.com/photodude thanks! @mbabker
https://github.com/mbabker does it mean that we are now sticking to
PHPCS-2?

—
Reply to this email directly or view it on GitHub
#680 (comment).

[elkuku]

May I just bark in here to confess that I definitely LOVE multiline concats and terneries?

I mean we should adapt the standards to our needs not the other way around...

[mbabker]

Some of it is definitely needing to be adapted still.

On Saturday, July 18, 2015, Nikolai Plath notifications@github.com wrote:

May I just bark in here to confess that I definitely LOVE multiline
concats and terneries?

I mean we should adapt the standards to our needs not the other way
around...

—
Reply to this email directly or view it on GitHub
#680 (comment).

[photodude]

At this point most of the work has been in getting a functional version of the joomla coding standards and reducing the number of custom Sniffs and replacing them with existing standards included in PHPCS. Still lots to do to get the standards matching with all of the points in the written standards.

The PHPCS2 version of the joomla coding standards will definitely be a huge help in enforcing the written coding style since PHPCS 2 includes fixer methods to automatically correct coding style deviations.

[elkuku]

@photodude they say that german people tend to talk "rude". I don't know why... sorry if my words had a negative tone, that was not my intention.

Adapting our coding standards to V2 of phpcs is a long outstanding task (at least since it has become stable and being installed by default).
Great work here - BIG THANKS!
(btw I was heavily involved in the creation of our V1 standards and most of the custom sniffs were adapted or created by me).

On the other hand I don't think that we should change beautiful looking code because a standard says so.
No this is not a discussion about code style, but I think we should adapt the sniffs to permit currently used formattings. If you want me to get my hands dirty on the code I can definitely do so (I guess). 😉

[photodude]

@elkuku it's all good. Nothing seemed "rude" to me. I think there are nuances lost in writing that can make a simple statement seem to have a different tone than was actually intended.

It's all volunteer work, you are more than welcome to help out. As they say in the movie Robots, "See a need, fill a need".

[b2z]

Ok we are not sticking to v2 then I am continue to code with v1? I cannot switch to different sniffers in my IDE :( And what to do with this PR then? The transition path it not clear to me.

[mbabker]

Thinking about this on my drive home from the airport, specifically the
ternery operators being multiline, as that is essentially an if/else
statement, I would almost suggest our code standard disallows multiline
ternery operations. It's just my opinion, but when those start becoming
multiline statements, it seems to make sense to me to write it as a proper
if/else structure.

On Saturday, July 18, 2015, Nikolai Plath notifications@github.com wrote:

May I just bark in here to confess that I definitely LOVE multiline
concats and terneries?

I mean we should adapt the standards to our needs not the other way
around...

—
Reply to this email directly or view it on GitHub
#680 (comment).

[photodude]

I think multi-line ternary operators could have a benefit, if they improve the readability of the code. I would definitely suggest disallowing nested or stacked ternary operators as those situations just create a mess for code readability and maintenance.

With that said; based on some things I have recently read, I question even allowing ternary operators. Or at least strongly discouraging them.

In general I think if a code standard or code style guidelines achieve the following then they are probably something that should be considered for adoption.

• Does it improve code consistency?
• Does it improve code readability?
• Does it improve future maintainability?
• Does it improve code portability?
• Does it improve code performance?
• Does it improve code security?

So applying those questions to ternary operators:

• They are a deviation from standard if/else, thus creating a question of consistency.
• They generally reduce readability
• They generally reduce maintainability
• They can reduce performance (since ternary operators do a copy operation, larger data sets will cause more performance issues)
• As for portability, and security I don't know of any pros or cons.

I would also say that there are places where short hand code is discouraged or not allowed (PHP tags for instance). So this maybe another area where the short hand operators may not be the best choice.

[b2z]

Guys so what should I do? Continue to code JTracker with v1 or switch to v2?

[mbabker]

Keep on with v1 for now. The v2 standard isn't stabilized yet.

[b2z]

ok thanks :)

[mbabker]

Closing for now. Will keep testing the PHPCS changes and come back when the standard is more stable 😉

[elkuku]

Too bad. Actually I liked most of the changes proposed here... Hope you come back soon 😉

[mbabker]

Still have the branch locally and still using this as my test bed for the PHPCS 2 standard. Just depends how much time I can push into helping stabilize that.

[photodude]

@elkuku Work is still active on the PHPCS2-joomla code standard. This PR was just related to testing of the in progress work.

Follow this PR in the Code standard to keep tabs on the work towards a stable release.
joomla/coding-standards#109

[elkuku]

I was just reading about the possibility of chaining null coalesce operators in ternaries - I hope our coding standards will allow them multi lined 😉

[vess]

@elkuku I agree with you that multi lines should be supported here, however, Squiz.WhiteSpace.OperatorSpacing.NoSpaceBefore throws an error here (multi and single line), seems that the operator isn't supported yet, but I guess it will be in near future.

Right now there are still some issues left, so if you have some time to spend, get your hands dirty please.