[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