Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release WPCS 2.0.0-RC1 #1608

Merged
merged 116 commits into from
Dec 30, 2018
Merged

Release WPCS 2.0.0-RC1 #1608

merged 116 commits into from
Dec 30, 2018

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 26, 2018

⚠️ DO NOT MERGE (YET) ⚠️

PR for tracking changes for the 2.0.0-RC1 release. Suggested target release date: Friday December 28 Monday December 31.

(shortened version of the release checklist as it's an RC)

jrfnl and others added 30 commits December 18, 2018 11:50
The VIP ruleset and sniffs were deprecated in WPCS 1.0.0 and will now be removed for WPCS 2.0.0.
…ve-deprecated-VIP-ruleset-sniffs

WPCS 2.0.0: Remove the deprecated VIP ruleset and sniffs
* Removes sniffs which were deprecated because they were renamed in WPCS 1.0.0 or before.
     Includes removing the related error code severity changes from the `WordPress` ruleset.
* Removes the old "abstract" classes from before they were turned into proper abstract classes in WPCS 0.10.0.
     Some of these were still used to unit test the abstract classes, but as all of the remaining abstract classes are in used, those are in effect tested via the sniffs using them.
* Removes the `AbstractVariablesRestrictionsSniff` as it is no longer used by active sniffs.
While cleaning up a plugin, I noticed that the issue count for the `WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound` error code was different if I ran phpcs with the `-n` flag (no warnings).

Error codes should be unique. Having the same error code for something which is mandatory (`error`) and recommended (`warning`) is bad practice and does not properly allow for modular disabling of notices.

This PR fixes this.

This is a breaking change as for the warnings, `<exclude>`s for the old errorcode currently in custom rulesets will be invalidated by it, so this PR should go into WPCS 2.0.0.
…ved-other-deprecated-sniffs

WPCS 2.0.0: Remove other deprecated sniffs
While cleaning up a plugin, I noticed that the issue count for the `WordPress.Security.NonceVerification.NoNonceVerification` error code was different if I ran phpcs with the `-n` flag (no warnings).

Error codes should be unique. Having the same error code for something which is mandatory (`error`) and recommended (`warning`) is bad practice and does not properly allow for modular disabling of notices.

This PR fixes this.

As the error code is changing anyhow, I figured it made sense to also remove the duplication of the sniff name from the code.

This is a breaking change as `<exclude>`s for the old errorcode currently in custom rulesets will be invalidated by it, so this PR should go into WPCS 2.0.0.

N.B.: The ruleset change is necessary until the deprecated sniffs have been removed.
…ve-deprecated-method

WPCS 2.0.0: Remove deprecated Sniff::has_html_open_tag() method
... and code related to them.
To prevent conflicts with WordPress itself - especially now that WordPress will let go of the PHP 5.2 minimum supported version -, WPCS should not use the `WordPress` namespace.

While PHPCS 2.x still needed to be supported, this could not be helped as it was a PHPCS requirement. Now that PHPCS 2.x support is being dropped, the namespace can be adjusted.

Note:
It is still a requirement that the namespace contains `StandardName\Sniffs\Category`, so we cannot replace `WordPress` with just `WordPressCS`. It has to be `WordPressCS\WordPress`.

Note:
This is a BC-break for any external standard which extends sniffs from WPCS. They will need to update their `use` statements to use the new WPCS namespace.

Refs:
* squizlabs/PHP_CodeSniffer 1469
* squizlabs/PHP_CodeSniffer 1569
* https://github.com/squizlabs/PHP_CodeSniffer/wiki/Version-3.0-Upgrade-Guide
…e-verification-errorcodes

NonceVerification: use separate errorcodes for warning vs error
…ve-deprecated-properties

WPCS 2.0.0: Remove deprecated properties
…ix-all-globals-errorcodes

PrefixAllGlobals: use separate errorcodes for warning vs error
…-phpcs-2

WPCS 2.0.0: Change minimum PHPCS requirement to PHPCS 3.3.1
The duplicates of the PHPCS 2.x test controller files can now be removed.

We also no longer need to load our `PHPCSAliases` file, not even for the autoloading of the abstract classes as PHPCS 3.1.0+handles this without problem.

For PHPUnit 7.x support, PHPCS does need a bootstrap file, but that is included with PHPCS, so all we need to do is reference that file in the command send to PHPUnit.
While strictly speaking the `PHPCSHelper` class is no longer needed at this moment, PHPCS 4.x may again contain breaking changes, so leaving it in place as a cross-version compatibility layer makes sense.

All the same, the PHPCS 2.x specific code can now be removed.
…-phpcs-2-unit-test-work-arounds

WPCS 2.0.0: Remove unit test work-arounds for PHPCS 2.x
…ve-phpcs-2-code-work-arounds

WPCS 2.0.0: Remove work-arounds for PHP < 5.4 and PHPCS < 3.3.1
…ve-phpcs-2-phpcshelper-code

WPCS 2.0.0: Remove PHPCS 2.x code from PHPCSHelper class
…multiple interfaces

Interfaces can extend multiple other interfaces. Looking at the sniff code, no changes are needed in the sniff to account for this, however, adding an additional unit test to warrant against sniff changes in the future which may not take this into account.

N.B.: I've verified all other usages of the `T_INTERFACE` token in the whole of WPCS and this was the only one which looked at `extends/implements`, so the only one for which interfaces extending multiple other interfaces could be problematic.

See:
* http://php.net/manual/en/language.oop5.interfaces.php#example-208

Loosely related to:
* squizlabs/PHP_CodeSniffer#2128
As of PHPCS 3.0.0, you can include individual error codes, not just complete sniffs.

Where WPCS rulesets included sniffs, but excluded more than 50% of the error codes from that sniff, it makes sense to switch them over to include individual error codes instead.

This also means that where previously, if the `type` and/or `message` of an individual error code was adjusted via the ruleset, the sniff needed to be included first and only after that, the details could be adjusted.
Now, the inclusion can just be done based on error code, providing all error codes in the sniff are adjusted.

To explain it better:
* If a sniff contained five error codes and we adjusted the message for one, there is no real advantage to changing over.
* If a sniff contained three error codes and we excluded one, and adjusted another, there is no real advantage to changing over.
* However, if a sniff contained only one error code and we'd adjust that one anyway, the new way of including just the error code gets rid of some superfluous configuration.
* Similarly, if a sniff contained five error codes, we'd exclude two and adjusted the details for the other three, changing over, again, is to our benefit.
A new format for passing array elements to PHPCS properties was added in PHPCS 3.3.0.
This new array format is far more readable and less error prone than the old format.

This commit implements using this new array format throughout WPCS, wherever relevant, including in property setting examples in the documentation.

Ref:
* squizlabs/PHP_CodeSniffer 1665
…HPCS 3.2+/3.3.2+)

As of PHPCS 3.2.0, PHPCS includes an XSD schema for rulesets which defines what can be used in the XML ruleset file.

In PHPCS 3.3.0 a new array format for properties was introduced and as of PHPCS 3.3.2, the new array format is now also covered by this schema.

This commit:
* Adds the schema tags to the `ruleset.xml` files, the WPCS native `.phpcs.xml.dist` file, as well as to the example ruleset.
* Adds validation against the schema for the `ruleset.xml` files and the example ruleset to the Travis build.

Note:
The schema used in the tags points to the online schema in PHPCS `master` for two reasons:
1. While WPCS now has a minimum requirement of PHPCS 3.3.1, the schema in PHPCS `master` contains the changes which were made in PHPCS 3.3.2, so we can safely validate the rulesets against the schema.
2. While installation via Composer is supported for WPCS, it is **not** the _only_ supported installation method, so pointing to the version of the XSD schema in the `vendor` directory would be presumptuous.

At the same time, using the URL in the Travis script appears to be problematic and as we're doing a composer install there anyway, we may as well use the version in the `vendor` directory for the actual validation.
As the validation is done on a build with PHPCS `master`, there will be no difference anyway.

Fixes 1477
…d-functionname-extra-unittest

ValidFunctionName: Add additional unit test for interfaces extending multiple interfaces
…set-simplifications-1

Rulesets: minor simplifications (PHPCS 3.0+)
…set-adjust-array-format

Rulesets: Start using the new XML array format (PHPCS 3.3+)
…set-validate-against-xsd

Build/Rulesets: Add XSD schema tags and validate rulesets against schema (PHPCS 3.2+/3.3.2+)
jrfnl and others added 9 commits December 25, 2018 17:55
Follow-up on PR 1560. As the PHPCS 2.x code has been removed, we can now use a `use` statement for the PHPCS 3.x `PHP_CodeSniffer\Config` class.
Removing:
* Unused `use` statement (no longer used after code removal).
* Some unnecessary interim variables (no longer needed as code which added to the variables has been removed).
* Unnecessary `return` statement.
* Simplify an (re-)assignment to the same variable.
* Remove an unused function parameter (no longer used after previous refactor of the function code).
…me-tweak

README: improve instructions about using PHPCompatibility (PHPCS 3.0+)
…aredsql-errorcode-change

PreparedSQL: use unique errorcode for messages
…alvariablesoverride-errorcode-change

GlobalVariablesOverride: minor error code adjustment
This sniff is based on an array of constants the sniff looks for.
Previously, the sniff had two distinct error codes for the different use-cases in which the constant could be found (use of vs declaration).

This change makes the error codes unique to the constant found, making it easier to silence the error message(s) for just one particular constant.
…shelper-use-use-statement

PHPCSHelper: clean up
…ouragedconstants-modular-errorcodes

DiscouragedConstants: make the error codes more modular
jrfnl and others added 9 commits December 26, 2018 17:01
This adds similar fixes which were made to the upstream `PEAR` sniff to the WPCS native version of the sniff in as far applicable.

* The `WordPress.NamingConventions.ValidFunctionName` sniff would throw the same error twice for functions in nested classes.
    The reasoning and solution used is the same as applied upstream.
    > Turns out the `processTokensWithinScope()` method gets called for each whitelisted scope a `T_FUNCTION` token exists in.
    >
    > In effect, this means that the first time it gets called for the `nestedFunction()` method, it is because it is within the scope of the `Nested` class, the second time the function gets called, it is for the method being in the scope of the anonymous class.
    >
    > The fix I've now applied checks that the scope for which the method is called is actually the deepest scope first and bows out if it's not.
    This fixes the error messages being thrown twice.
* For methods in anonymous classes, the error message would read `Method name "nestedFunction" in class  is not ....` (take note of the missing class name) which could be considered confusing.
    I've changed this to now read: `read `Method name "nestedFunction" in class [Anonymous class] is not `
* Class and method names in PHP are case _in_sensitive. While the sniff took that into account for the magic methods, it did not when checking for PHP4 style constructors/destructors.
     This has now been fixed by lowercasing both the class name as well as the function name before comparing them.

Includes unit tests.
As the "method name" check exist out if it finds an extended class or implemented interface anyway, checking the name of underscore magic methods which are only used in extended classes is redundant.
…te-contributing

Contributing: update the documentation about installing and running the unit tests
…dfunctionname-sync-in-upstream-changes

ValidFunctionName: sync in fixes from upstream
... rename it to `get_snake_case_name_suggestion()` and make it a static method so it's also available to sniffs extending upstream sniffs.
* Adjusts the error messages to include an alternative name suggestion in snake_case.
* Adds back the previously stripped `$` to the variable name for the error message.
* Adjusts the error messages the sniff throws to be more consistent.

Fixes 1280
Minor changes missed in PR 1586.

* Update the class documentation.
* Remove a, now unused, property array key.
... to confirm that the sniff handles multi-variable/property declarations correctly.

(some of these would have failed prior to PHPCS 3.3.0)
@jrfnl
Copy link
Member Author

jrfnl commented Dec 28, 2018

FYI: I haven't done the RC release yet to allow #1612 to be reviewed and merged before the RC goes out as after that, only bug fixes against the release should be allowed to go in until 2.0.0 has been released.

Hoping for a response on #1612 before Monday to still be able to release the RC in 2018.

…-validvariablenames-errorcodes

ValidVariableName: adjust error messages and codes
@GaryJones
Copy link
Member

#1612 has now been merged.

@jrfnl
Copy link
Member Author

jrfnl commented Dec 29, 2018

@GaryJones Appreciated!

I'll update the changelog release date to Monday. Now we only still need some approvals for this PR 👋 @WordPress-Coding-Standards/wpcs-admins

Changelog for WPCS version 2.0.0-RC1

* Release date set at this Monday December 31..
* Includes all currently merged changes.
@jrfnl jrfnl merged commit c0877a9 into master Dec 30, 2018
@jrfnl
Copy link
Member Author

jrfnl commented Dec 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants