Update for PHPCS 2.x series

[photodude]

Reference issue #53
Reference issue #104

Reorder ruleset.xml to better match the Squiz/ruleset.xml
This improves readability when migrating versions
Also increased the max line the comment to 150 chars long since it was that way in the code

The T_DOC_COMMENT token has also been removed, replaced instead by T_DOC_COMMENT_OPEN_TAG

rebuild from source with modifications

• add T_FUNCTION, and T_CLASS, to array of tokens
• remove parenthesis_opener checks (as was done previously)
• add T_COMMENT and T_WHILE to the $tokens[$trailingContent] checks (as was done previously)
• This is now a Joomla class sniff
• add comment about the removed section "parenthesis_opener"
  -- by adding a comment about what was intentionally removed we will help the coding standard be able to make changes quicker when the source standard changes or has bug fixes applied.

Update rules to use source standards to eliminate unnecessary duplication
add Squiz.WhiteSpace.CastSpacingSniff rule so duplication can be eliminated.

• Since we are not making any specific changes here, this file should likely be eliminated and just include the Squiz standard
  add Generic.WhiteSpace.DisallowSpaceIndentSniff rule
  nothing special here, replace with generic rule
• add Generic.PHP.LowerCaseConstantSniff rule
  nothing special here, replace with Squiz rule
  nothing special here, replace with Squiz rule
  nothing special here, replace with PEAR rule
  Add PSR2.ControlStructures.ElseIfDeclaration rule
  note: Sniff is not part of the rule names just the file names

copy PR #106 Better documentation of differences from core standards

Note: this will be composer compatible

For consuming packages there are some items that will typically result in creating their own project ruleset.xml, rather than just directly using the Joomla ruleset.xml.

1. Excluding the rule Generic.Arrays.DisallowShortArraySyntax because the consuming package does not support PHP5.3
2. Excluding the rule Joomla.NamingConventions.ValidVariableName.NotCamelCaps because the consuming package is supporting snake_case variable names for backwards compatibility reasons or because they are supporting legacy database schemas that used sname_case naming.
3. Excluding files or folders from having code standards enforced. This is common for excluding vendor libraries and 3rd party code. Although this can be done on the command line, a project ruleset.xml is easier to maintain and is easy to implement with other modifications.
   Details on using a project ruleset.xml can be found in the PHP Code Sniffer Documentation

[photodude]

New todo item

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

[mbabker]

What's the changelist looking like for 3.x? Depending on scope both could
conceivably be supported.

On Thursday, October 13, 2016, Walt Sorensen notifications@github.com
wrote:

At what point to we stop work on this phpcs2.x branch in favor of a
ruleset for the soon to be released phpcs3.x ?

I'm thinking we should complete this soon then put effort into converting
our custom sniffs to phpcs3.x

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#109 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoTHOkZVQNMSNm2V1_5U887gvLwBfks5qzqUigaJpZM4EIodL
.

[photodude]

@mbabker for PHPCS3.x It again breaks backwards compatibility for all custom sniffs and custom reports. An upgrade guide for sniff and report developers is available here: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Version-3.0-Upgrade-Guide

it also has a php 5.4 minimum requirement

[photodude]

on the upside all of the built in sniffs in the ruleset.xml will continue to work.

[mbabker]

Oy vey

On Thursday, October 13, 2016, Walt Sorensen notifications@github.com
wrote:

@mbabker https://github.com/mbabker for PHPCS3.x It again breaks
backwards compatibility for all custom sniffs and custom reports. An
upgrade guide for sniff and report developers is available here:
https://github.com/squizlabs/PHP_CodeSniffer/wiki/Version-
3.0-Upgrade-Guide

it also has a php 5.4 minimum requirement

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#109 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoSQLcTkGD7PfMcBWEar8qP7A79qHks5qzqb1gaJpZM4EIodL
.

[810]

could you add support for:

<?php if (!empty($actions)) : ?>

now its saved to:

[mbabker]

Ran a test with this branch on the stats server repo.

Build result against PHPCS 2.6.2: https://travis-ci.org/joomla-extensions/jstats-server/jobs/168115206
Result of phpcbf: joomla/statistics-server@2fce9a1
Build result after phpcbf: https://travis-ci.org/joomla-extensions/jstats-server/jobs/168115586

No other custom changes applied.

[photodude]

@mbabker Some of those items indicate that the stats server repo needs to run a modified version of the coding standards. It doesn't need to reject short array syntax since it's not supporting php 5.3.
I make some of these modifications when testing the CMS

The readme has some explanations for adjustments.

Here is a modified ruleset.xml similar to the one I've been using to test the cms with since the cms has a number of legacy code style issues that have BC implications if changed.

<?xml version="1.0"?>
<ruleset name="Joomla-stats">

    <arg name="report" value="full"/>
    <arg name="tab-width" value="4"/>
    <arg name="encoding" value="utf-8"/>
    <arg value="sp"/>
    <arg name="colors" />

    <!-- Exclude folders not containing production code -->
    <exclude-pattern>*/build/*</exclude-pattern>
    <exclude-pattern>*/tests/*</exclude-pattern>
    <exclude-pattern>*/lib/*</exclude-pattern>
    <exclude-pattern>*/tmpl/*</exclude-pattern>
    <exclude-pattern>*/layouts/*</exclude-pattern>

    <!-- Exclude 3rd party libraries. -->
    <exclude-pattern>*/libraries/*</exclude-pattern>
    <exclude-pattern>*/vendor/*</exclude-pattern>
    <exclude-pattern>*/editors/*</exclude-pattern>

    <rule ref="Joomla">
        <exclude name="Joomla.NamingConventions.ValidVariableName.MemberNotCamelCaps"/>
        <exclude name="Joomla.NamingConventions.ValidVariableName.NotCamelCaps"/>
        <exclude name="Joomla.NamingConventions.ValidFunctionName.ScopeNotCamelCaps"/>
        <exclude name="Generic.Arrays.DisallowShortArraySyntax"/>
    </rule>

    <rule ref="Joomla.Classes.InstantiateNewClasses">
        <properties>
            <property name="shortArraySyntax" value="true"/>
        </properties>
    </rule>
</ruleset>

[photodude]

@810 I'm not able to replicate the issue you describe with the current version of the standard and phpcs 2.7.0.

That also looks like mixed php/html code for a layout/view file. At the moment I'm not sure if we are going to test against those mixed code layout files.

In my testing, I intentionally exclude files with mixed php/html code

[810]

Can we merge this pr to new branch, then do a release

[photodude]

@mbabker what are your thoughts on merging this and preparing for an alpha release?

These are the items I still had listed to be addressed

• 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.

• Fix ControlStructuresBrackets sniff issues with the unitTests.

• 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)

• 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)

[mbabker]

We need a plan to communicate this because we WILL break people's stuff going through with this.

The current master branch should become a 1.x branch.

The phpcs-2 branch should become the master branch.

Once that's done, we add the package to Packagist.

[810]

We can't use phpcs-2 now: because jorobo still needs edgedesign/phpqa and that package is only for 1.x

[mbabker]

That package declares "squizlabs/php_codesniffer": "*" so it says it will work with all PHPCS versions. If that's a lie it's a bug in that package and has nothing to do with the efforts in this repo.

Nothing says that every consumer of this repo has to update to PHPCS 2.x as soon as it's merged into this repo. The 1.x compatible sniffer will still be available, you would just need to update the git submodule references to use the appropriate branch since the current definition isn't on Packagist (though we may want to look at if we can make 1.x Composer installable since merging the efforts in this PR will put the whole repo into Packagist).

[photodude]

According to the travis testing of EdgedesignCZ/phpqa they are testing with phpcs 2.7.1 so there shouldn't be an issue... of course that package has nothing to do with the efforts in this repo.

Even jorobo is using a PHPCS 2.x coding standard that was based on this PR... of course that package also has nothing to do with the efforts in this repo.

[mbabker]

I'll throw a post together to run on the dev site announcing the need for the branch change and the new Composer integration so we can get the branches sorted out right. That'll give a couple weeks for anyone who might be pulling the repo to update resources (we can do a 1.x branch and keep it in sync with current master for a short transition period before merging phpcs-2 to master).

If I remember, I'll do a once over here and merge over the weekend.

[photodude]

Sounds good. Let me know what to do about the 5 items listed above. Maybe we open new PR's and/or issues related to those items after this PR is merged into the phpcs-2 branch...