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

Update to WPCS 1.0.0 #165

Merged
merged 259 commits into from
Jul 28, 2018
Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 25, 2018

  • Merge in WPCS 1.0.0
  • Update the WordPress-Theme ruleset to account for the changes in WPCS 1.0.0

The only really relevant commit is: ed820f0 (the ruleset changes).

Once this PR has been merged, we have a good base to de-fork from WPCS.


Update: I've added an additional commit - 762902b - to get the build to pass. This is a ruleset check which would need to be adjusted or removed when de-forking anyway, so this should be fine for now.

jrfnl and others added 30 commits November 26, 2017 04:14
The original sniff class still exists, but extends the renamed sniff now.
In addition, a deprecation warning will be thrown if this sniff is included directly by a custom ruleset.
A deprecation warning will also be thrown if the sniff is included directly just to set custom properties.

Note: if the sniff was explicitly **excluded** via a custom ruleset, no deprecation warning will be thrown and the exclusion will have no effect. The custom ruleset will need to be updated to use the new sniff name for the exclusion to be reinstated.
The original sniff class still exists, but extends the renamed sniff now.
In addition, a deprecation warning will be thrown if this sniff is included directly by a custom ruleset.
A deprecation warning will also be thrown if the sniff is included directly just to set custom properties.

Note: if the sniff was explicitly **excluded** via a custom ruleset, no deprecation warning will be thrown and the exclusion will have no effect. The custom ruleset will need to be updated to use the new sniff name for the exclusion to be reinstated.
The original sniff class still exists, but extends the renamed sniff now.
In addition, a deprecation warning will be thrown if this sniff is included directly by a custom ruleset.
A deprecation warning will also be thrown if the sniff is included directly just to set custom properties.

Note: if the sniff was explicitly **excluded** via a custom ruleset, no deprecation warning will be thrown and the exclusion will have no effect. The custom ruleset will need to be updated to use the new sniff name for the exclusion to be reinstated.
The original sniff class still exists, but extends the renamed sniff now.

Note: if the sniff was explicitly **excluded** via a custom ruleset, no deprecation warning will be thrown and the exclusion will have no effect. The custom ruleset will need to be updated to use the new sniff name for the exclusion to be reinstated.
The message thrown by the sniff has been downgraded to a warning and the message text adjusted to make the sniff more generically usable.

For the VIP ruleset, this change is undone via a custom ruleset property.
…egory

The original sniff class still exists, but extends the renamed sniff now.
In addition, a deprecation warning will be thrown if this sniff is included directly by a custom ruleset.
A deprecation warning will also be thrown if the sniff is included directly just to set custom properties.

Note: if the sniff was explicitly **excluded** via a custom ruleset, no deprecation warning will be thrown and the exclusion will have no effect. The custom ruleset will need to be updated to use the new sniff name for the exclusion to be reinstated.
The original sniff class still exists, but extends the renamed sniff now.
In addition, a deprecation warning will be thrown if this sniff is included directly by a custom ruleset.
A deprecation warning will also be thrown if the sniff is included directly just to set custom properties.

Note: if the sniff was explicitly **excluded** via a custom ruleset, no deprecation warning will be thrown and the exclusion will have no effect. The custom ruleset will need to be updated to use the new sniff name for the exclusion to be reinstated.
The message thrown by the sniff has been downgraded to a warning and the message text adjusted to make the sniff more generically usable.

For the VIP ruleset, this change is undone via a custom ruleset property.
The original sniff class still exists, but extends the renamed sniff now.
In addition, a deprecation warning will be thrown if this sniff is included directly by a custom ruleset.
A deprecation warning will also be thrown if the sniff is included directly just to set custom properties.

Note: if the sniff was explicitly **excluded** via a custom ruleset, no deprecation warning will be thrown and the exclusion will have no effect. The custom ruleset will need to be updated to use the new sniff name for the exclusion to be reinstated.
…xclude-assignment-in-condition-while

CS: exclude while assignment in condition from own CS rules
Also:
* Add alternatives for a couple of functions.
* Annotated differences where the sniff is actually correct and WP Core is lagging behind.
…te-minimum-wp-version

Update default minimum WP version
This reverts commit c27064a.
…d rename

The sniff has been renamed from `GlobalVariables` to `GlobalVariablesOverride` to make it easier to understand at one glance what the sniff is about.

The original sniff class still exists, but extends the renamed sniff now.
In addition, a deprecation warning will be thrown if this sniff is included directly by a custom ruleset.
A deprecation warning will also be thrown if the sniff is included directly just to set custom properties.

Note: if the sniff was explicitly **excluded** via a custom ruleset, no deprecation warning will be thrown and the exclusion will have no effect. The custom ruleset will need to be updated to use the new sniff name for the exclusion to be reinstated.
…rt-build-failure-fix

Revert "Fix build failure"
…-part-2-rename-sniffs

Issue 1157/Proposal 2: rename sniffs
Split this sniff into two: one in VIP checking for disabling of pagination, one in WP checking for high pagination limit.

The WP version now throws a `warning`. This is reset to an error for use by VIP via the VIP ruleset.
…-part-2-split-postsperpage

PostsPerPage: split sniff
A couple of sniffs which were previously in VIP are also useful for `Extra`. The messages in these sniffs have - where appropriate - been downgraded to warnings.

This commit adds these sniffs to the `Extra` ruleset.

Note: There are three more candidates for this:
* `DB.DirectDatabaseQuery`
* `DB.SlowDBQuery`
* `Security.ValidatedSanitizedInput`

I've not included the first two (yet) as those may need some more discussion first.
The third one is not included as it throws a lot of noise messages and has quite some open issues surrounding it, which should be addressed first IMO.
…-add-some-previous-VIP-rules-to-extra

Add a couple of extra sniffs to `Extra`
Hard excluding check makes it impossible for a custom ruleset to re-enable a rule.
By changing the severity instead, a custom ruleset can up the severity of a check to re-enable it.

Note: whether this will work or not depends on the loading order of rules. No guarantees given.
The fact that it may not work in certain circumstances, is something which should be addressed upstream.
…incorrect-deprecation-message

Fix two "typo"'s in deprecation messages
Warns about the use of the `wp_redirect()` function, suggesting
`wp_safe_redirect()` instead, to avoid malicious redirects.

This is based on the warning currently in VIP.RestrictedFunctions, but
in a more generic form.

I've added the sniff to the `WordPress-Extra` ruleset.
…strictedFunctions

Also adds back in the reminder to call `exit()`.
…rity/wp-safe-redirect

New Security.SafeRedirect sniff to encourage safe redirects
The `WP.I18n` sniff did not properly verify whether the `T_STRING` found was actually a function call, in contrast to a method call or a namespaced function call.

I've fixed this now by extending the `AbstractFunctionRestriction` sniff and using the logic contained therein to make sure that only plain function calls are being addressed.

Includes unit tests.

A future further improvement could be extending the `AbstractFunctionParameter` sniff instead to benefit from using the logic from the `get_function_call_parameters()` utility function.

For now, I've just made the function parenthesis check slightly more stable, by making it code style independent and adding an extra safeguard for live coding situations.

Fixes #1266
GaryJones and others added 20 commits July 13, 2018 15:22
When the next token after a parenthesis is a colon, AND the parenthesis has a `parenthesis_owner` that means it relates to a function or closure, then we're dealing with a colon that is being used for a return type declaration, and therefore it should NOT be flagged.

Fixes #1420.
…-return-type-colon-spacing

Fix false positive violation for return type colon
Since this week, the PHPCompatibility has been moved to a dedicated GitHub organisation account.

A PHPCompatibilityWP ruleset is now also available in a separate repo which already excludes all back-fills/poly-fills which WP provides.

For more details, see the release notes of the 8.2.0 release: https://github.com/PHPCompatibility/PHPCompatibility/releases/tag/8.2.0
…oser-update-phpcompatibility

Composer & Readme: update organisation name PHPCompatibility
Remove several boolean functions from the list of auto escaped functions
... and fix namespace for the example sniff used as the sniff has been moved.
…ributing-update-whitelist-comment-info

Contributing: discourage adding new whitelist comments
This "hack" works and seems to be the simplest solution to the issue at hand.

**Beware**: just like in the original VIP deprecation, the VIP sniffs are being `exclude`d. This means that if people use the `WordPress` ruleset and in their custom ruleset reference any of the VIP sniffs, they will **not** see the deprecation warning.

Note: This uses `<exclude>` rather than `severity` as in PHPCS 2.9 severity could only be changed on error code level.
Custom rulesets can overrule this  by changing the `severity` of either the category if using PHPCS 3.x or of the error codes when using PHPCS 2.9.

Fixes 1425

Includes a few more spelling fixes in the same ruleset.
…VIP-deprecation-notices

WordPress ruleset: fix deprecation notices coming through
WPCS itself uses `WordPress-Extra` + `WordPress-Docs` for its CS, which includes `WordPress-Core`.

Those rulesets were therefore "tested" on each build, just by running them against the WPCS code.
However, the WPCS native CS ruleset also excludes some things and the CS check ignores warnings for the Travis run and it also left the `WordPress-VIP` and `WordPress` rulesets _untested_.

This PR intends to fix this by adding a minimal test file which should trigger most sniffs and testing each ruleset against it.

This should, from now on, prevent the ruleset issues we've seen related to the moving and deprecating of sniffs.
This will also serve as an early warning system in case any of the upstream sniffs which are included in the rulesets would be removed/renamed or otherwise have a change in behaviour which WPCS will need to take into account.

Notes:
* This is complimentary to the unit tests as those are run in a ruleset-agnostic manner, i.e. the ruleset isn't loaded and any settings in the ruleset are ignored.
* At this moment, the test file is incomplete, but complete enough for our purposes.
    Further improvement of the test file in the future to make sure that all sniffs are triggered is recommended.
* The check only needs to be run once against every PHPCS version supported/tested.
    With that in mind, "old-style" ignore annotations have been used.
    Once the minimum PHPCS version WPCS supports goes beyond PHPCS 3.2.0, those should be swopped out for selective ignores instead.
* The check should be run on a high PHP version to prevent triggering the `Generic.PHP.Syntax` sniff when modern PHP code is included in the test file to ensure that this triggers the appropriate sniffs.
    At this moment, the most modern syntax included in nullable types which was introduced in PHP 7.1 which is why that is the PHP version against which these tests are currently run.
…is-add-ruleset-check

Travis: add a new check for early detection of ruleset problems
@joyously
Copy link

You've been so busy!

Copy link
Member

@dingo-d dingo-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, the VIP rules will be rewritten so you can merge this :)

@jrfnl jrfnl merged commit 538a99f into feature/theme-review-sniffs Jul 28, 2018
@jrfnl jrfnl deleted the feature/update-to-wpcs-1.0.0 branch July 28, 2018 14:26
@jrfnl jrfnl added this to the 0.1.0 milestone Aug 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.