[META] Finalizing PHPCS 2.x support

[mbabker]

Extracted from https://github.com/joomla/coding-standards/pull/109#issuecomment-268876448

• [x] In the ruleset.xml there are some pear rules with adjusted messages that we either need to accept messages reporting spaces or will need to create custom copies to properly report the number of tabs.

• [x] Fix ControlStructuresBrackets sniff issues with the unitTests.

• [x] Testing against the CMS needs a custom ruleset as a custom standard to mute some errors. The readme has some explanations for adjustments. An example xml file should be included. I also can provide an xml example (code in previous comment way back) and another xml example for projects that have php5.4+ requirements like the Joomla-stats repo (xml code in previous comment just above)

• [x] Review what our custom sniffs are based on for changes in PHPCS 2.7.x that need to be adopted into our custom sniffs.

  • [ ] review for consistency with the old ruleset warnings/errors from phpcs 1.5.x (after the automatic fixers and manual fixes the 1.5.x ruleset should not throw any warnings/errors due to the code style phpcs 2.7.x version being applied. (I've been occasionally running against the CMS admin components for this check, I've only completed PR's for 3 or 4 components)

• [x] Merge coding standards manual to phpcs-2 branch

[photodude]

• [ ] consider/implement [Proposal] Code standards change to implement psr-2 multiline class params for more readability https://github.com/joomla/joomla-cms/issues/13438

[photodude]

@mbabker where do you think the example xml files for selectively applying rules should live in the repo?

Added in ExampleRulesets via #152

[photodude]

• [x] Add composer instructions
  • see #154

[photodude]

Just notice this new issue with InstantiateNewClasses fixer as shown in the InstantiateNewClassUnitTest If we instantiate a New Class as $k = new self(); we end up with $k = new ; See https://travis-ci.org/photodude/coding-standards/jobs/200657756

InstantiateNewClass issue has been fixed, Thanks @wilsonge

we also have a bunch of Missing @package tag in file comment (Joomla.Commenting.FileComment.MissingPackageTag) issues now being reported as of phpcs 2.7.1+ fixed in #150

[photodude]

As best I can tell the ControlStructuresBrackets sniff issues are related to the section that deals with bracket indenting. https://github.com/joomla/coding-standards/blob/phpcs-2/Joomla/Sniffs/ControlStructures/ControlStructuresBracketsSniff.php#L148-L192

Likely in the str_repeat() with tabs~

ControlStructuresBrackets sniff issues have been fixed, Thanks @wilsonge

[photodude]

@wilsonge Are you still up for helping to work on these issues so we can move forward with the Alpha release?

~~This is the Branch I'm working with to try and create a PR to solve these issues. https://github.com/photodude/coding-standards/tree/patch-2~~

[photodude]

@rdeutz If your interested, this is the current list of Items to address with the PHPCS2 branch for a public alpha release of the updated standard with autofixers.

[photodude]

@nibra, @810, @vess if you are interested in continuing to follow the path to a public alpha release for the PHPCS2 branch, or contributing to fixing any of the issues in the PHPCS2 version of the standards prior to the alpha release this is issue to follow.

[wilsonge]

I very much am!

[wilsonge]

I've done fixes for the InstantiateNewClasses and the ControlStructuresBrackets for the patch-5 (i made them against patch-2 like you asked - but locally been testing against patch-5 given that was where your unit test was running against that you linked to) branch and then the package tag warning looks correct to me

[photodude]

• [x] fix cs errors in code style custom sniffs fixed in #150

[photodude]

added a number of fixes see

• 152 for Testing against the CMS needs a custom ruleset

• 151 fix for a newly found bug with how docblock tags are reported

• 150 fix for code style issues

[photodude]

Still need to

• address the messages for PEAR.Functions.FunctionCallSignature.Indent and PEAR.Functions.FunctionDeclaration.Indent we need to decide on one of the following

  • Accept messages reporting spaces
  • Accept messages reporting spaces and Add a note about autofixer will convert spaces to tabs
  • see #153
  • Create custom copies of these sniffs to properly adjust and report the number of tabs found/expected. (this has a high maintenance cost in the long run)

• Add composer instructions

  • see #154

• consider/implement [Proposal] Code standards change to implement psr-2 multiline class params for more readability

after that, I think the other review items can happen with the public alpha

[photodude]

@mbabker @wilsonge Once we merge PRs 150-153 we just need to add composer instructions and we are set for the Alpha release. All other items are changes to the existing ruleset or are review items that can be handled with the alpha release.

[wilsonge]

I've merged #150, #151 and #152. I want to have a quick look over #153 later today to convince myself that it is the best solution :)

[photodude]

Thanks @wilsonge.

[photodude]

I've added some composer install information (based on the framework composer instructions)

• see #154

[photodude]

With the merge of #153 we can check off the first item about dealing with the adjusted messages. That leaves the review items and the proposed change to adopt the PSR-2 multiline class params.

How do we want to proceed with those items and making the official Alpha release?

[mbabker]

Added a task on merging the manual to the right branch too since that's no longer in a self-contained branch (more of a reminder than anything since the dev site will read the manual from the master branch).

[photodude]

I think there are also some changes to the IDE section in the Master branch that will need to be merged into the into phpcs-2 as well.

[wilsonge]

So with merging #153 where does that leave us here?

[photodude]

The blog post needs to be finalized, @wilsonge have you reviewed the draft of that?

Dev Blog post

• [x] Finalize the Dev blog post about the public alpha release
• [x] Publish the Dev blog post about the public alpha release
  • Scheduled for publication on 2017/03/01

    Create the release file(s)

• [x] Create release file(s) and packagist item(s)

  Travis

• [x] add .travis.yml stub to master/1.x
  • PR #167
• [x] add repo to travis ci for testing

  As for other tasks, all of the following I feel can be addressed during the public alpha

  Review

• [x] Review what our custom sniffs are based on for changes in PHPCS 2.7.x that need to be adopted into our custom sniffs.
• [ ] Review for consistency with the old ruleset warnings/errors from phpcs 1.5.x after the automatic fixers and manual fixes have been applied

  Updates

• [x] Merge coding standards manual to phpcs-2 branch
  • PR #168
• [x] Merge changed IDE items to phpcs-2 branch (probably needs another review and update too)
  • PR #168

    Changes

• [ ] Consider/implement [Proposal] Code standards change to implement psr-2 multiline class params for more readability https://github.com/joomla/joomla-cms/issues/13438
• [ ] Contribute updates to Core PHPCS sniffs to eliminate many, or all, of our custom sniffs
  • priority on the sniffs reporting spaces as they lack a tabIndent = true param
  • https://github.com/squizlabs/PHP_CodeSniffer/pull/1362
  • https://github.com/squizlabs/PHP_CodeSniffer/pull/1355

[wilsonge]

Blog post looked good to me :)

[photodude]

@mbabker Are we good now to release the public alpha?

[photodude]

Custom Sniffs Review

• [x] Classes/InstantiateNewClassesSniff.php
  • Based on (this seems to be a fully custom sniff)
  • Reason for Custom sniff
  • Instanciating new class without parameters does not require brackets.
  • reviewed on 2017/02/19
• [x] Commenting/ClassCommentSniff.php
  • Based on PEAR.Commenting.ClassComment Sniff
  • Reason for Custom sniff
  • Tags are in a different order and have different requierments,
  • reviewed on 2017/02/19
• [x] Commenting/FileCommentSniff.php
  • Based on PEAR.Commenting.FileComment Sniff
  • Reason for Custom sniff
  • Tags are in a different order, don't ignore some things, difference in some tag processing
  • reviewed on 2017/02/19
  • see update PR #160
• [x] Commenting/FunctionCommentSniff.php
  • Based on (extends) PEAR.Commenting.FunctionComment Sniff
  • also based on Squiz.Commenting.FunctionComment Sniff
  • Reason for Custom sniff
  • Extended ruleset for parsing and verifying the doc comments for functions
  • we have a difference in our spaces to force alignment with @return
  • reviewed on 2017/02/19
  • see PR #159
• [x] Commenting/SingleCommentSniff.php
  • Based on PEAR.Commenting.InlineComment Sniff
  • Reason for Custom sniff
  • differences in SingleComment coding standards
  • reviewed on 2017/02/19
• [x] ControlStructures/ControlSignatureSniff.php
  • Based on Squiz.ControlStructures.ControlSignature sniff
  • Reason for Custom sniff
  • differences in control structure coding standards
  • reviewed on 2017/02/19
• [x] ControlStructures/ControlStructuresBracketsSniff.php
  • Based on PEAR.Classes.ClassDeclaration Sniff
  • Reason for Custom sniff
  • Checks if the declaration of control structures is correct Curly brackets must be on a line by their own
  • reviewed on 2017/02/14
• [x] ControlStructures/WhiteSpaceBeforeSniff.php
  • Based on (this seems to be a fully custom sniff)
  • Reason for Custom sniff
  • Checks that there is an empty line before a control structure or return statement
  • reviewed on 2017/02/19
• [x] Functions/StatementNotFunctionSniff.php
  • Based on PEAR.Files.IncludingFile Sniff
  • Reason for Custom sniff
  • add tokens so sniff covers all php statements
  • reviewed on 2017/02/19
• [x] NamingConventions/ValidFunctionNameSniff.php
  • Based on PEAR.NamingConventions.ValidFunctionName
  • Reason for Custom sniff
  • Extends PEAR.NamingConventions.ValidFunctionName.processTokenWithinScope to remove the requirement for leading underscores on private method names.
  • reviewed on 2017/02/19
  • PR for update #158
• [x] NamingConventions/ValidVariableNameSniff.php
  • Based on (extends) Squiz.NamingConventions.ValidVariableName Sniff
  • Reason for Custom sniff
  • changes to processMemberVar function in the sniff to ignore the Private member variable "%s" must contain a leading underscore requierment
  • reviewed on 2017/02/19
• [x] Operators/ValidLogicalOperatorsSniff.php
  • Based on Squiz.ValidLogicalOperators Sniff
  • Reason for Custom sniff
  • Exceptions for things like or jexit(), or JSession, or define, or sendResponse, or sendJsonResponse
  • reviewed on 2017/02/19
• [x] WhiteSpace/MemberVarSpacingSniff.php
  • Based on (extends) Squiz.WhiteSpace.MemberVarSpacing Sniff
  • Reason for Custom sniff
  • There needs to be 1 blank line before the var, not counting comments.
  • reviewed on 2017/02/19
  • PR for update #157

[photodude]

I have completed a review of our custom stiffs against the PHPCS core sniffs they are based on, (as best I could tell) Details of the review are posted above, and PR's have been opened for the related changes.

[photodude]

@mbabker what is the plan on releasing the public alpha?

[mbabker]

I'm working with my marketing liaison on getting the blog post scheduled for publishing (hopefully this coming week) then we'll sort out a timeline.

[mbabker]

Blog is scheduled for tomorrow.

[photodude]

Any thoughts on merging PR https://github.com/joomla/coding-standards/pull/159

• merged

[photodude]

@mbabker with the blog post scheduled for tomorrow what is the plan on tagging the alpha release?

[mbabker]

I'd say the next week or two.

[photodude]

I attempted to sync the master changes over to the phpcs-2 branch, but I definitely did something wrong in git and broke the branch I was making the attempt on. I'm starting to think the two branches have diverged too much and we'll just have to cherry pick the commits we want to bring over. Then again, maybe someone who is better skilled at git could make the rebase/merge sync from master to phpcs-2 without breaking everything like I did.

[wilsonge]

I think I can help with this :) You can't rebase. But I think merging gives not-so-bad conflicts at a very quick glance? Is there a time we can skype and I can help you with this?

[photodude]

@wilsonge that would be great. We got about a 6 hour time difference to figure out for scheduling (and I've never skyped) I have some additional availability this week I'm sure we can work something out. Do you have my email (or twitter) for direct messaging?

[photodude]

• [x] Merge coding standards manual to phpcs-2 branch
  • PR #168

[photodude]

With PR #168 and PR #167 are we good to finalize the change over?

[photodude]

Are we good to go with a tagged alpha release now?

[rdeutz]

a while ago I created this https://github.com/joomla-projects/docker-joomla-cms-phpcs for the 1.x code styles, I think it should be easy to tweak it to use the 2.x code styles. So we could let them run side by side and compare

[wilsonge]

Go for it :)

[photodude]

@rdeutz I recommend using the example sub-ruleset for the CMS that is included with the 2.x version in the master branch. https://github.com/joomla/coding-standards/tree/master/Joomla/ExampleRulesets/Joomla-CMS/ruleset.xml

This sub-ruleset will exclude the portions of the CMS that are not tested. more details are in the readme https://github.com/joomla/coding-standards#selectively-applying-rules

[photodude]

Are we good to go with a tagged alpha release now? The review items can happen during the Alpha release.

[photodude]

@mbabker please check off "Merge coding standards manual", that has been completed.

The last item listed for review is the review for consistency with the old ruleset, this is something that I feel can, and should, be done during the alpha release phase and completed with the beta release phase.

Is there anything holding back tagging an alpha release?

[mbabker]

Off the top of my head, I don't think so. I keep meaning to get folks together to start up a subteam in automated testing to take full lead on this and let you all manage the repo and release schedule for the tooling, hopefully with a little of the slowdown between 3.7 and starting up on 3.8 I can find time for that.

[photodude]

Would it be possible to get the alpha release tagged so public testing can go forward and let the subteam fall into place as the team is put together?

[mbabker]

https://github.com/joomla/coding-standards/releases/tag/2.0.0-alpha

We can throw more resources into this over the next couple of weeks.

[mbabker]

Also, https://packagist.org/packages/joomla/coding-standards

[photodude]

Thank you

[elkuku]

Thank you ;)

Am 26.04.2017 8:46 nachm. schrieb "Walt Sorensen" <notifications@github.com

:

Thank you

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/joomla/coding-standards/issues/143#issuecomment-297588315, or mute the thread https://github.com/notifications/unsubscribe-auth/AACEuidjAPmE8ONp2DoWJTHDi2tI13klks5rz_NmgaJpZM4LdeyI .

[photodude]

I have done a baseline test of the alpha release with the CMS using the example ruleset for the CMS. As expected I got no new errors reported.

Using the full ruleset I got the expected errors for things like failure to use camelCased naming.

you can see the configuration I used for .travis.yml and composer.json in my cms fork patch 9 https://github.com/photodude/joomla-cms/tree/patch-9

note: the composer.json file for the standard fails to correctly install the standard. Some minor adjustment will be needed to fix the path.