Conversation
The `basepath` and `parallel` options were introduced in PHPCS 3.0 and can now be enabled for the WPCS native PHPCS checking. And as WPCS 2.0.0 drops the `VIP` ruleset, we can now include the `WordPress` ruleset instead of including the `WordPress-Extra` + `WordPress-Docs` rulesets separately.
Bumped suggested installer version
Merge back Master into Develop
… global functions The `PrefixAllGlobals` sniff verifies that functions declared in the global namespace are prefixed with one of the whitelisted prefixes as passed to the sniff in the `prefixes` property in a custom ruleset. The sniff prevents false positives for polyfills for PHP native functions - which should be named exactly as named in PHP without prefix for them to be usable as a polyfill - by checking that the function didn't exist. Generally speaking this works fine in 95% of all cases as PHPCS is normally run stand-alone and doesn't contain any functions defined in the global namespace. Similarly, when PHPCS is installed via Composer, this would normally work fine as the commonly used `autoload` options are `PSR-0`, `PSR-4` and `classmap` which are all based on code being in classes. However, if the Composer `autoload` `files` option is used to load, for instance, a functions file declaring functions in the global namespace, this "polyfill false positive prevention" would incorrectly cause errors _not_ to be thrown for functions declared in the global namespace which were now autoloaded via Composer. I have now fixed this by, instead of using `function_exists()`, using a check against a functions list retrieved via `get_defined_functions()`. This change does not have unit tests as: * Preventing false positives for polyfills is already unit tested. * Checking that autoloaded user defined functions are not recognized as PHP native functions is not something which can be unit tested as such. This would need an integration test including a composer setup to be tested. Based on the test case provided in the issue which originally reported this, I have confirmed that this PR fixes the issue though. Fixes 1632
…e correct variable is examined The `Sniff::is_validated()` method did not check whether the variable being validated was in actual fact the same variable as the one for which validation was needed. This could cause false negatives for the `InputNotValidated` error if an array key in, for instance, `$_GET` was validated, but that same array key was later requested - without validation - from `$_POST`.
…-prefixallglobals-fix-false-negative-composer-file-autoload PrefixAllGlobals: prevent false negatives for autoloaded user-defined global functions
…f-is-validated-check-variable-name ValidatedSanitizedInput: only recognize a variable as validated if the correct variable is examined
Just like, `isset()` and `empty()`, `array_key_exists()` is a way to validate a variable exists and should therefore be recognized by this function.
Just like, `isset()` and `empty()`, `array_key_exists()` is a way to validate a variable exists and should therefore be recognized by this function.
…f-isset-empty-add-array-key-exists ValidateSanitizedInput: allow for validation using array_key_exists()
…ve-phpcs-ruleset-tweaks WPCS native PHPCS ruleset: minor tweaks
…wp-testcase-class Sniff::$test_class_whitelist: add newly added base test class to the list
Nightly has become PHP 8.0 since PHP 7.4 has been branched, so to continue to also test against PHP 7.4, it needs to be added separately. Refs: * https://twitter.com/nikita_ppv/status/1089839541828112384 * https://twitter.com/nikita_ppv/status/1094897743594770433
As of recently, the Travis images for PHP 7.2+ ship with PHPUnit 8.x. The PHPCS native test framework is not compatible with PHPUnit 8.x and won't be able to be made compatible with it until the minimum PHP version would be raised to PHP 7.1. So for the unit tests to be able to run on PHP 7.2+, we need to explicitly require PHPUnit 7.x for those builds. This has been implemented in the same way as a similar requirement was previously implemented fo HHVM. As for nightly: there is no PHPUnit version which is currently compatible with PHP 8. As that either mean that the builds for `nightly` would always fail or - if the unit tests would be skipped -, the only check executed on `nightly` would be linting the files, I've elected to remove build testing against `nightly` for the time being.
…is-test-against-php-7.4 Travis: test builds against PHP 7.4 & work around for PHPUnit 8
Identified by joyously in the ThemeReviewCS repo: WPTRT/WPThemeReview 202 When inline JQuery is used to reference a stylesheet link tag, the sniff misidentifies this as a stylesheet which needs to be enqueued. This very simple fix should prevent that, at least for the reported cases. Includes unit test.
…euedresources-bug-fix WP.EnqueuedResources: bug fix
…e-sudo Remove `sudo: false`
When people run the sniffs without the `-s` option, the messages for some of the errors in this sniff were not specific enough as the "Prefix**AllGlobals**" part would not be seen. I considered adjusting the error message template `ERROR_MSG`, however, this would cause more confusion as, for instance, namespace declarations will **always** be in the global namespace. So, I've opted to adjust select error messages instead.
Reported by remcotolsma in 49e7700#commitcomment-32391794 While a little esoteric in a WP context, using the WP File Wrapper functions is not a suitable alternative when `php://input` is being read. Includes unit test.
…rnative-functions-allow-file-get-contents-input-stream AlternativeFunctions: allow for php://input used with file_get_contents()
Co-Authored-By: NielsdeBlaauw <niels.de.blaauw@gmail.com>
Co-Authored-By: NielsdeBlaauw <niels.de.blaauw@gmail.com>
…peoutput-recognize-more-array-walking-functions EscapeOutput: allow for map_deep() to output escape arrays
The null coalesce operator `??` is a special comparison operator, in the sense that it doesn't compare a variable to whatever is on the other side of the comparison operator. For this reason, it should be possible to disregard it.
…way to validate a variable This adds recognition of the coalesce operator `??` (PHP 7.0) and the coalesce equals operator `??=`, as will be added in PHP 7.4, to the `Sniff::is_validated()` method. This prevents false positives where variables would be seen as "not validated", when the variable has in fact been validated via a coalesce equals assignment in a previous statement. Related to 764, 840
…oalesced variable
…l coalesce equals PHP 7.0 introduced the null coalesce operator, while PHP 7.4 will introduce the null coalesce equal operator. These operators should be accounted for in the `ValidatedSanitizedInput` sniff as valid ways to validate a variable, but should still allow for the sniff to *also* check for sanitization. Refs: * https://php.net/manual/en/language.operators.comparison.php#language.operators.comparison.coalesce * https://wiki.php.net/rfc/isset_ternary * https://wiki.php.net/rfc/null_coalesce_equal_operator Related to 764 Fixes 837 Closes 840 which is superseded by this PR
… check This builds onto the similar changes made for the `ValidatedSanitizedInput` sniff in ... This fixes false positives as reported in 1114 and 1506. Note: it is not currently checked that the nonce check is done within the same conditional scope as the comparison. Just that it is done within the same _function_ scope. Includes unit tests. Fixes 1114 Fixes 1506
…-1506-nonceverification-allow-comparisons-before Sniff::has_nonce_check(): allow for comparing a variable before nonce check
…840-validatedsanitizedinput-allow-for-null-coalesce ValidatedSanitizedInput: handle null coalesce (equal) correctly
As correctly pointed out in https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/ 869#issuecomment-478329921 and confirmed via a trace-back of the functions used by `wp_unslash()`, WP contains two additional unslashing functions which are perfectly valid to use for unslashing received data. Refs: * https://developer.wordpress.org/reference/functions/wp_unslash/ * https://developer.wordpress.org/reference/functions/stripslashes_deep/ * https://developer.wordpress.org/reference/functions/stripslashes_from_strings_only/ This commit adds allowance for using these. It also updates the error message to reflect this. Includes unit tests.
…datedsanitizedinput-allow-for-other-unslashing-functions ValidatedSanitizedInput: allow for more unslashing functions
…e check This also gets rid of a bug were sanitization was not recognized as such if the call to `wp_unslash()` was nested inside it. Includes unit tests. Fixes 572
…nonceverification-allow-for-wp_unslash Sniff::has_nonce_check(): allow for unslashing a variable before nonce check
… check Any usage of superglobals in calls to sanitization functions were up to now ignored. As the output of sanitization is normally assigned to a names variable, this meant that it was possible to bypass the nonce verification sniff that way. I don't believe that was the intended behaviour. Instead IMO, a sanitization call should be allowed prior to the nonce verification, but should not negate the nonce verification check. This fixes that. Includes unit tests.
As per the proposal in https://make.wordpress.org/core/2019/03/26/coding-standards-updates-for-php-5-6/ Open actions: * [x] Adjust Core PHP handbook to mention this rule in the `Clever code` section
…everification-sanitization-alone-is-not-enough NonceVerification: bug fix - sanitization is no alternative for nonce check
…-no-assignments-in-condition Core: forbid assignments in conditions
As per the proposal in 1624, this moves the sniffs which `warn` against the use of loose comparisons to the Core ruleset. Open actions: * [x] Adjust Core PHP handbook to mention this rule in the `Clever code` section (or wherever else it should go).
* Release date set at this Monday April 8th. * Includes all currently merged changes. * Includes a minor update to the Readme file for the DealerDirect Composer plugin version number. Related to 1628 (which is included in this release)
…-use-strict-comparisons Core: warn against the use of loose comparisons
…gelog-wpcs-2.1.0 Changelog for WPCS 2.1.0
Member
Author
|
Eh... @WordPress-Coding-Standards/wpcs-collaborators can I please have at least one approval so I can do the release ? |
GaryJones
approved these changes
Apr 8, 2019
GaryJones
left a comment
GaryJones
left a comment
Member
There was a problem hiding this comment.
Approved, with one observation.
| <arg value="sp"/> | ||
| <arg name="extensions" value="php"/> | ||
| <arg name="basepath" value="."/> | ||
| <arg name="parallel" value="8"/> |
Member
There was a problem hiding this comment.
Noting that PHPCS appears to have an issue with this arg being used on some installs of PHP 7.3.
It might be worth leaving this out for now, though it would only potentially affect contributors to WPCS.
Member
Author
There was a problem hiding this comment.
As this is the ruleset for WPCS itself, I'm not too concerned. If this (upstream) issue would affect a contributor, they can easily get round it by passing --parallel=1 on the command line.
Correct me if I'm wrong, but I don't think changing this is a reason to delay the release.
Member
There was a problem hiding this comment.
I don't think changing this is a reason to delay the release.
Completely agree, which is why I approved it - just mentioning it for future reference for future readers.
Member
Author
|
Release tweet: https://twitter.com/jrf_nl/status/1115207606308614145 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR for tracking changes for the 2.1.0 release. Target release date: Monday April 8.
Open PR to mergemasterintodevelopto get rid of the This branch is out-of-date with the base branch messages on each release. (only after major releases)