From 60ba1c8632026610d31f43659b3e0ca3d3a15fac Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Sun, 18 Aug 2019 14:37:46 +0100 Subject: [PATCH 01/25] wp_remote_get: expand on message Encourage developers to switch to the VIP version of the function by telling them a benefit. Fixes #436. --- .../Sniffs/Functions/RestrictedFunctionsSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php index 658af707..dfd3ff62 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php @@ -295,7 +295,7 @@ public function getGroups() { // @link VIP Go: https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#remote-calls 'wp_remote_get' => [ 'type' => 'warning', - 'message' => '%s() is highly discouraged, please use vip_safe_wp_remote_get() instead.', + 'message' => '%s() is highly discouraged. Please use vip_safe_wp_remote_get() instead which is designed to more gracefully handle failure than wp_remote_get() does.', 'functions' => [ 'wp_remote_get', ], From 13bcfb4ccae59a99fb4a447d142a8c5095b924eb Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Sun, 18 Aug 2019 14:45:34 +0100 Subject: [PATCH 02/25] append: Remove error for use of append in VIP Go It's not clear why this violation is marked as an Error for VIP Go ruleset, when none of the other HTMLExecutingFunctions are left as their default of Warnings. Since the best the Sniff can do is provide warnings when a variable is used (though that variable may already be safe, and not contain user input), then it would seem odd to change just one function call to an Error for VIP Go. --- WordPress-VIP-Go/ruleset.xml | 3 --- 1 file changed, 3 deletions(-) diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 34cd751a..390ee73e 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -112,9 +112,6 @@ warning File system operations only work on the `/tmp/` and `wp-content/uploads/` directories. To avoid unexpected results, please use helper functions like `get_temp_dir()` or `wp_get_upload_dir()` to get the proper directory path when using functions such as %s(). For more details, please see: https://wpvip.com/documentation/vip-go/writing-files-on-vip-go/ - - error - warning %s() is uncached. If the function is being used to fetch a remote file (e.g. a URL starting with https://), please use wpcom_vip_file_get_contents() to ensure the results are cached. For more details, please see https://wpvip.com/documentation/vip-go/fetching-remote-data/ From fc0ae5f2f1d791b4dc6eb49601e8c5bd5dccb273 Mon Sep 17 00:00:00 2001 From: Rebecca Hum Date: Tue, 27 Aug 2019 12:17:17 -0600 Subject: [PATCH 03/25] WordPress-VIP-Go: Downgrade to "Warning" level for AdminBarRemovalSniff with message customized for Go --- WordPress-VIP-Go/ruleset-test.inc | 18 +++++++++--------- WordPress-VIP-Go/ruleset-test.php | 24 ++++++++++++------------ WordPress-VIP-Go/ruleset.xml | 10 ++++++++++ 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index 22a10859..ef4c8ff4 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -218,11 +218,11 @@ function foo_bar_foo() { } // WordPressVIPMinimum.UserExperience.AdminBarRemoval -add_filter( 'show_admin_bar', '__return_false' ); // Error. +add_filter( 'show_admin_bar', '__return_false' ); // Warning. add_filter( 'show_admin_bar', '__return_true' ); // Ok. -show_admin_bar( false ); // Error. +show_admin_bar( false ); // Warning. show_admin_bar( true ); // Ok. -add_filter( 'show_admin_bar', 'my_own_return_false' ); // Error. +add_filter( 'show_admin_bar', 'my_own_return_false' ); // Warning. echo ''; ?> 1, 187 => 1, 188 => 1, - 221 => 1, - 223 => 1, - 225 => 1, - 228 => 1, - 229 => 1, - 230 => 1, - 235 => 1, - 236 => 1, - 237 => 1, - 245 => 1, - 246 => 1, - 247 => 1, 252 => 1, 255 => 1, 256 => 1, @@ -197,6 +185,18 @@ 208 => 1, 212 => 1, 217 => 1, + 221 => 1, + 223 => 1, + 225 => 1, + 228 => 1, + 229 => 1, + 230 => 1, + 235 => 1, + 236 => 1, + 237 => 1, + 245 => 1, + 246 => 1, + 247 => 1, 265 => 1, 269 => 1, 273 => 1, diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 390ee73e..1e36f791 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -82,6 +82,16 @@ 6 File system operations only work on the `/tmp/` and `wp-content/uploads/` directories. To avoid unexpected results, please use helper functions like `get_temp_dir()` or `wp_get_upload_dir()` to get the proper directory path when using functions such as %s(). For more details, please see: https://wpvip.com/documentation/vip-go/writing-files-on-vip-go/ + + warning + 6 + Removal of admin bar is highly discouraged for user roles of "administrator" and "vip_support" -- if these roles are already excluded, this warning can be ignored. + + + warning + 6 + Hiding of admin bar is highly discouraged for user roles of "administrator" and "vip_support" -- if these roles are already excluded, this warning can be ignored. + error 6 From 5a88cb04524aee8238ac0e6575d0534b2a1a0bb5 Mon Sep 17 00:00:00 2001 From: Ian Jenkins Date: Fri, 13 Sep 2019 15:27:38 +0100 Subject: [PATCH 04/25] Add get_page_by_path() warning to VIP restricted function sniffs. On VIP there is an alternative which should be used for caching `wpcom_vip_get_page_by_path()` make developers aware of this by warning of usage of `get_page_by_path()`. --- .../Sniffs/Functions/RestrictedFunctionsSniff.php | 7 +++++++ .../Tests/Functions/RestrictedFunctionsUnitTest.inc | 3 +++ .../Tests/Functions/RestrictedFunctionsUnitTest.php | 1 + 3 files changed, 11 insertions(+) diff --git a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php index dfd3ff62..eb34c058 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php @@ -326,6 +326,13 @@ public function getGroups() { 'create_function', ], ], + 'get_page_by_path' => [ + 'type' => 'warning', + 'message' => '%s() is highly discouraged due to not being cached; please use wpcom_vip_get_page_by_path() instead.', + 'functions' => [ + 'get_page_by_path', + ], + ], ]; $deprecated_vip_helpers = [ diff --git a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc index b518275e..d26ac8b4 100644 --- a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc @@ -221,3 +221,6 @@ update_site_option( $bar, 'foo' ); // Warning. update_option( 'foo', $bar ); // Ok. delete_site_option( $foo ); // Warning. delete_option( $bar ); // Ok. + +wpcom_vip_get_page_by_path(); // Ok - VIP recommended version of get_page_by_path(). +get_page_by_path( $page_path ); // Warning. diff --git a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php index 5a1fdf44..598ff307 100644 --- a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php @@ -133,6 +133,7 @@ public function getWarningList() { 138 => 1, 139 => 1, 208 => 1, + 226 => 1, ]; } From d8fbf909bff6991db130add04649a80303c4b654 Mon Sep 17 00:00:00 2001 From: kevinfodness Date: Thu, 21 Nov 2019 12:46:03 -0500 Subject: [PATCH 05/25] Use new sniff WordPress.DateTime.RestrictedFunctions --- WordPressVIPMinimum/ruleset-test.inc | 2 +- WordPressVIPMinimum/ruleset.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc index d3a894cb..8d3641bc 100644 --- a/WordPressVIPMinimum/ruleset-test.inc +++ b/WordPressVIPMinimum/ruleset-test.inc @@ -41,7 +41,7 @@ $args = array( _query_posts( 'posts_per_page=999' ); // Warning. $query_args['posts_per_page'] = 999; // Warning. -// WordPress.WP.TimezoneChange +// WordPress.DateTime.RestrictedFunctions date_default_timezone_set( 'FooBar' ); // Error. // WordPress.DB.PreparedSQL diff --git a/WordPressVIPMinimum/ruleset.xml b/WordPressVIPMinimum/ruleset.xml index 68084c8e..1ff8b2a0 100644 --- a/WordPressVIPMinimum/ruleset.xml +++ b/WordPressVIPMinimum/ruleset.xml @@ -26,7 +26,7 @@ - + From 38ec33981a88f9f2a912b376d42a2bf06e4422cc Mon Sep 17 00:00:00 2001 From: Rebecca Hum <16962021+rebeccahum@users.noreply.github.com> Date: Mon, 2 Dec 2019 15:39:08 -0700 Subject: [PATCH 06/25] Allow short array syntax and fix tests failing for WordPress.Arrays.ArrayKeySpacingRestrictions.TooMuchSpaceAfterKey --- .phpcs.xml.dist | 5 +++++ .../Sniffs/Functions/DynamicCallsSniff.php | 20 +++++-------------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index 5096365e..db53aea3 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -34,6 +34,11 @@ + + + + + diff --git a/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php index 05318571..e1f6a22e 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php @@ -106,9 +106,7 @@ private function collect_variables() { return; } - $current_var_name = $this->tokens[ - $this->stackPtr - ]['content']; + $current_var_name = $this->tokens[ $this->stackPtr ]['content']; /* * Find assignments ( $foo = "bar"; ) @@ -202,9 +200,7 @@ private function find_dynamic_calls() { */ if ( ! isset( - $this->variables_arr[ - $this->tokens[ $this->stackPtr ]['content'] - ] + $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ] ) ) { return; } @@ -220,16 +216,12 @@ private function find_dynamic_calls() { $i++; } while ( 'T_WHITESPACE' === - $this->tokens[ - $this->stackPtr + $i - ]['type'] + $this->tokens[ $this->stackPtr + $i ]['type'] ); if ( 'T_OPEN_PARENTHESIS' !== - $this->tokens[ - $this->stackPtr + $i - ]['type'] + $this->tokens[ $this->stackPtr + $i ]['type'] ) { return; } @@ -242,9 +234,7 @@ private function find_dynamic_calls() { */ if ( ! in_array( - $this->variables_arr[ - $this->tokens[ $this->stackPtr ]['content'] - ], + $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ], $this->blacklisted_functions, true ) ) { From 136e7bcee88c2a82eaf4c32bd932f25cf93e4ba0 Mon Sep 17 00:00:00 2001 From: Rebecca Hum <16962021+rebeccahum@users.noreply.github.com> Date: Wed, 29 Jan 2020 11:10:41 -0700 Subject: [PATCH 07/25] Update Bug report issue template for develop check --- .github/ISSUE_TEMPLATE/bug_report.md | 4 ++++ .github/ISSUE_TEMPLATE/enhancement.md | 3 +++ 2 files changed, 7 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 8bdf38b2..f01a03d2 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,6 +1,9 @@ --- name: Bug report about: Create a report to help us improve +title: '' +labels: '' +assignees: '' --- @@ -50,3 +53,4 @@ Use `php -v` and `composer show` to get versions. ## Tested Against `master` branch? - [ ] I have verified the issue still exists in the `master` branch of VIPCS. +- [ ] I have verified the issue still exists in the `develop` branch of VIPCS. diff --git a/.github/ISSUE_TEMPLATE/enhancement.md b/.github/ISSUE_TEMPLATE/enhancement.md index 980c3c1f..8015ab5e 100644 --- a/.github/ISSUE_TEMPLATE/enhancement.md +++ b/.github/ISSUE_TEMPLATE/enhancement.md @@ -1,6 +1,9 @@ --- name: Enhancement about: Suggest an improvement for this project +title: '' +labels: '' +assignees: '' --- From dc8321467efd30ab86dcb1e06561b03d61bae45c Mon Sep 17 00:00:00 2001 From: Rebecca Hum <16962021+rebeccahum@users.noreply.github.com> Date: Thu, 30 Jan 2020 17:06:18 -0700 Subject: [PATCH 08/25] IncludingFileSniff: Add get_parent_theme_file_path to safelist of path functions --- WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php | 1 + 1 file changed, 1 insertion(+) diff --git a/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php b/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php index 2b6efd09..c99aa535 100644 --- a/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php +++ b/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php @@ -31,6 +31,7 @@ class IncludingFileSniff extends AbstractFunctionRestrictionsSniff { 'get_stylesheet_directory', 'get_template_directory', 'locate_template', + 'get_parent_theme_file_path' ]; /** From 15a1cd96a9a7ccb398c00df25f5c928e6dbce039 Mon Sep 17 00:00:00 2001 From: Rebecca Hum <16962021+rebeccahum@users.noreply.github.com> Date: Thu, 30 Jan 2020 17:17:54 -0700 Subject: [PATCH 09/25] RestrictedFunctions: Add stats_get_csv() along with unit tests --- WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php | 1 - .../Sniffs/Functions/RestrictedFunctionsSniff.php | 7 +++++++ .../Tests/Functions/RestrictedFunctionsUnitTest.inc | 3 +++ .../Tests/Functions/RestrictedFunctionsUnitTest.php | 1 + 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php b/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php index c99aa535..2b6efd09 100644 --- a/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php +++ b/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php @@ -31,7 +31,6 @@ class IncludingFileSniff extends AbstractFunctionRestrictionsSniff { 'get_stylesheet_directory', 'get_template_directory', 'locate_template', - 'get_parent_theme_file_path' ]; /** diff --git a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php index eb34c058..3edb5672 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php @@ -268,6 +268,13 @@ public function getGroups() { 'delete_site_option', ], ], + 'stats_get_csv' => [ + 'type' => 'error', + 'message' => 'Using `%s` outside of Jetpack context pollutes the stats_cache entry in the wp_options table. We recommend building a custom function instead.', + 'functions' => [ + 'stats_get_csv', + ] + ], 'wp_mail' => [ 'type' => 'warning', 'message' => '`%s` should be used sparingly. For any bulk emailing should be handled by a 3rd party service, in order to prevent domain or IP addresses being flagged as spam.', diff --git a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc index d26ac8b4..9b778b40 100644 --- a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc @@ -224,3 +224,6 @@ delete_option( $bar ); // Ok. wpcom_vip_get_page_by_path(); // Ok - VIP recommended version of get_page_by_path(). get_page_by_path( $page_path ); // Warning. + +$popular = stats_get_csv( 'postviews', [ 'days' => 2, 'limit' => 20 ] ); // Error. +$popular = custom_stats_get_csv( 'postviews', [ 'days' => 2, 'limit' => 20 ] ); // Ok. \ No newline at end of file diff --git a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php index 598ff307..147c7cfa 100644 --- a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php @@ -109,6 +109,7 @@ public function getErrorList() { 218 => 1, 220 => 1, 222 => 1, + 228 => 1, ]; } From 0faf12fad8736e80a2fa8d43934855741d0d83fb Mon Sep 17 00:00:00 2001 From: Rebecca Hum <16962021+rebeccahum@users.noreply.github.com> Date: Mon, 3 Feb 2020 12:45:34 -0700 Subject: [PATCH 10/25] IncludingFileSniff: Add get_parent_theme_file_path to safelist --- WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php | 2 +- WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc | 1 + WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php b/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php index c99aa535..0f383800 100644 --- a/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php +++ b/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php @@ -31,7 +31,7 @@ class IncludingFileSniff extends AbstractFunctionRestrictionsSniff { 'get_stylesheet_directory', 'get_template_directory', 'locate_template', - 'get_parent_theme_file_path' + 'get_parent_theme_file_path', ]; /** diff --git a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc index fa1d6b18..b225a4a7 100644 --- a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.inc @@ -12,6 +12,7 @@ require dirname( __FILE__ ) . '/my_file.php' ); require dirname( __FILE__, 2 ) . '/my_file.php' ); require dirname( dirname( __FILE__ ) ) . '/my_file.php' ); require dirname( dirname( __DIR__ ) ) . '/my_file.php' ); +require_once( get_parent_theme_file_path( '/my_file.php' ); ); // Warnings. include ( MY_CONSTANT . "my_file.php" ); // Custom constant. diff --git a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php index c6a748de..5957cc5d 100644 --- a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php +++ b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php @@ -22,11 +22,11 @@ class IncludingFileUnitTest extends AbstractSniffUnitTest { */ public function getErrorList() { return [ - 23 => 1, 24 => 1, 25 => 1, 26 => 1, 27 => 1, + 28 => 1, ]; } @@ -37,10 +37,10 @@ public function getErrorList() { */ public function getWarningList() { return [ - 17 => 1, 18 => 1, 19 => 1, 20 => 1, + 21 => 1, ]; } From 0fab6acc686fa11b367b83b0d4d7e3c211683ef5 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Fri, 17 Apr 2020 16:24:28 +0100 Subject: [PATCH 11/25] Travis: rearrange, and fix warnings and infos Tested at https://config.travis-ci.com/explore --- .travis.yml | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9707c948..b7b3d471 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,35 +1,17 @@ +language: php +os: linux dist: xenial -cache: - directories: - - $HOME/.cache/composer/files - -language: - - php - -before_install: - # Speed up build time by disabling Xdebug. - # https://johnblackbourn.com/reducing-travis-ci-build-times-for-wordpress-projects/ - # https://twitter.com/kelunik/status/954242454676475904 - - phpenv config-rm xdebug.ini || echo 'No xdebug config.' - -install: - - composer require squizlabs/php_codesniffer:"$PHPCS_BRANCH" --update-no-dev --no-suggest --no-scripts - - composer install --dev --no-suggest - -script: - # Run the unit tests. - - ./bin/unit-tests - - # Run ruleset tests. - - ./bin/ruleset-tests - env: # `master` is now 3.x. - PHPCS_BRANCH="dev-master" # Lowest supported release in the 3.x series with which VIPCS is compatible. - PHPCS_BRANCH="3.3.1" +cache: + directories: + - $HOME/.cache/composer/files + php: - 5.6 - 7.0 @@ -85,3 +67,19 @@ jobs: - ./bin/phpcs +before_install: + # Speed up build time by disabling Xdebug. + # https://johnblackbourn.com/reducing-travis-ci-build-times-for-wordpress-projects/ + # https://twitter.com/kelunik/status/954242454676475904 + - phpenv config-rm xdebug.ini || echo 'No xdebug config.' + +install: + - composer require squizlabs/php_codesniffer:"$PHPCS_BRANCH" --update-no-dev --no-suggest --no-scripts + - composer install --dev --no-suggest + +script: + # Run the unit tests. + - ./bin/unit-tests + + # Run ruleset tests. + - ./bin/ruleset-tests From 59383c9d9d769d0bead995db26b865688ba091bd Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Fri, 17 Apr 2020 16:51:51 +0100 Subject: [PATCH 12/25] Travis: Change install: false to install: skip Seems that this got changed at some point. --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index b7b3d471..39d775d0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,8 +38,8 @@ jobs: php: 7.3 env: PHPCS_BRANCH="dev-master" before_install: phpenv config-rm xdebug.ini || echo 'No xdebug config.' - install: false - cache: false + install: skip + cache: skip script: # Lint the PHP files against parse errors. - ./bin/php-lint From 65b937f79f68261e4319504d72c529195dbdec7d Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Fri, 17 Apr 2020 16:58:16 +0100 Subject: [PATCH 13/25] CS: Add missing trailing comma --- .../Sniffs/Functions/RestrictedFunctionsSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php index 3edb5672..164021cf 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php @@ -273,7 +273,7 @@ public function getGroups() { 'message' => 'Using `%s` outside of Jetpack context pollutes the stats_cache entry in the wp_options table. We recommend building a custom function instead.', 'functions' => [ 'stats_get_csv', - ] + ], ], 'wp_mail' => [ 'type' => 'warning', From 8cc2c8ea55344fea57fe1acea50656742096ca0f Mon Sep 17 00:00:00 2001 From: rebeccahum <16962021+rebeccahum@users.noreply.github.com> Date: Thu, 2 Jul 2020 12:49:47 -0600 Subject: [PATCH 14/25] RestrictedFunctions: Remove get_super_admins rule on Go ruleset --- WordPress-VIP-Go/ruleset-test.inc | 2 +- WordPress-VIP-Go/ruleset-test.php | 1 - WordPress-VIP-Go/ruleset.xml | 3 +++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index ef4c8ff4..d6992fe9 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -353,7 +353,7 @@ opcache_compile_file( $test_script ); // Error. opcache_​is_​script_​cached( 'test_script.php' ); // Error. opcache_​get_​status(); // Error. opcache_​get_​configuration(); // Error. -get_super_admins(); // Error. +get_super_admins(); // OK. wpcom_vip_irc(); // Error. flush_rewrite_rules(); // Error. $wp_rewrite->flush_rules(); // Error. diff --git a/WordPress-VIP-Go/ruleset-test.php b/WordPress-VIP-Go/ruleset-test.php index 1370e6ba..28493bca 100644 --- a/WordPress-VIP-Go/ruleset-test.php +++ b/WordPress-VIP-Go/ruleset-test.php @@ -44,7 +44,6 @@ 353 => 1, 354 => 1, 355 => 1, - 356 => 1, 357 => 1, 358 => 1, 359 => 1, diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 1e36f791..57c751a8 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -305,6 +305,9 @@ 0 + + 0 + 0 From f5d34378d07ac03f3fd170fe612473ea96f8a546 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Fri, 3 Jul 2020 14:26:13 +0100 Subject: [PATCH 15/25] Dependencies: Update versions --- .travis.yml | 2 +- composer.json | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 39d775d0..bb2bd8ef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ env: # `master` is now 3.x. - PHPCS_BRANCH="dev-master" # Lowest supported release in the 3.x series with which VIPCS is compatible. - - PHPCS_BRANCH="3.3.1" + - PHPCS_BRANCH="3.5.5" cache: directories: diff --git a/composer.json b/composer.json index c37ec9f9..d545d818 100644 --- a/composer.json +++ b/composer.json @@ -16,16 +16,16 @@ ], "require": { "php": ">=5.6", - "squizlabs/php_codesniffer": "^3.3.1", - "wp-coding-standards/wpcs": "^2.1" + "squizlabs/php_codesniffer": "^3.5.5", + "wp-coding-standards/wpcs": "^2.3" }, "require-dev": { - "dealerdirect/phpcodesniffer-composer-installer": "^0.5", + "dealerdirect/phpcodesniffer-composer-installer": "^0.7", "phpcompatibility/php-compatibility": "^9", "phpunit/phpunit": "^5 || ^6 || ^7" }, "suggest": { - "dealerdirect/phpcodesniffer-composer-installer": "^0.5 || This Composer plugin will sort out the PHPCS 'installed_paths' automatically." + "dealerdirect/phpcodesniffer-composer-installer": "^0.7 || This Composer plugin will manage the PHPCS 'installed_paths' automatically." }, "minimum-stability": "RC", "scripts": { From b5375222c647021ee60ffb3c3eab451a838d136a Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Fri, 3 Jul 2020 16:20:52 +0100 Subject: [PATCH 16/25] Switch to Generic.VersionControl.GitMergeConflict Removes WordPressVIPMinimum.VersionControl.MergeConflict sniff. The merge conflict boundary detection sniff in the built-in Generic standard is a more comprehensive sniff, and it means one less thing for VIP to maintain. Requires PHPCS 3.4+ (made a requirement in #484). Fixes #330. --- WordPress-VIP-Go/ruleset-test.inc | 14 +-- WordPress-VIP-Go/ruleset-test.php | 3 +- .../VersionControl/MergeConflictSniff.php | 98 ------------------- .../VersionControl/MergeConflictUnitTest.inc | 13 --- .../VersionControl/MergeConflictUnitTest.php | 41 -------- WordPressVIPMinimum/ruleset-test.inc | 15 ++- WordPressVIPMinimum/ruleset-test.php | 5 +- WordPressVIPMinimum/ruleset.xml | 1 + 8 files changed, 17 insertions(+), 173 deletions(-) delete mode 100644 WordPressVIPMinimum/Sniffs/VersionControl/MergeConflictSniff.php delete mode 100644 WordPressVIPMinimum/Tests/VersionControl/MergeConflictUnitTest.inc delete mode 100644 WordPressVIPMinimum/Tests/VersionControl/MergeConflictUnitTest.php diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index d6992fe9..11c2b764 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -567,12 +567,8 @@ $_SERVER['HTTP_X_FORWARDED_FOR']; // Error. $_SERVER["REMOTE_ADDR"]; // Error. // phpcs:enable WordPress.Security.ValidatedSanitizedInput.InputNotValidated,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -// WordPressVIPMinimum.VersionControl.MergeConflict -function is_prime( $n ) { - if ( 2 === $n ) { - } -//phpcs:ignore Generic.PHP.Syntax.PHPSyntax -======= // Error. - if ( $n % 2 === 0 ) { - } -} +// Generic.VersionControl.GitMergeConflict +?> +<<<<<<< HEAD // Error. + +>>>>>>> // Error. diff --git a/WordPress-VIP-Go/ruleset-test.php b/WordPress-VIP-Go/ruleset-test.php index 28493bca..26be9a5a 100644 --- a/WordPress-VIP-Go/ruleset-test.php +++ b/WordPress-VIP-Go/ruleset-test.php @@ -115,7 +115,8 @@ 565 => 1, 566 => 1, 567 => 1, - 575 => 1, + 572 => 1, + 574 => 1, ], 'warnings' => [ 4 => 1, diff --git a/WordPressVIPMinimum/Sniffs/VersionControl/MergeConflictSniff.php b/WordPressVIPMinimum/Sniffs/VersionControl/MergeConflictSniff.php deleted file mode 100644 index a2922dac..00000000 --- a/WordPressVIPMinimum/Sniffs/VersionControl/MergeConflictSniff.php +++ /dev/null @@ -1,98 +0,0 @@ -tokens[ $stackPtr ]['code'] ) { - $nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true ); - if ( T_SL !== $this->tokens[ $nextToken ]['code'] ) { - return; - } - $nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true ); - if ( T_STRING !== $this->tokens[ $nextToken ]['code'] || 0 !== strpos( $this->tokens[ $nextToken ]['content'], '<<< HEAD' ) ) { - return; - } - - $message = 'Merge conflict detected. Found "<<<<<<< HEAD" string.'; - $this->phpcsFile->addError( $message, $stackPtr, 'Start' ); - - return; - } - - if ( T_ENCAPSED_AND_WHITESPACE === $this->tokens[ $stackPtr ]['code'] ) { - if ( 0 === strpos( $this->tokens[ $stackPtr ]['content'], '=======' ) ) { - $this->addSeparatorError( $stackPtr ); - - return; - } - - if ( 0 === strpos( $this->tokens[ $stackPtr ]['content'], '>>>>>>>' ) ) { - $message = 'Merge conflict detected. Found "%s" string.'; - $data = [ trim( $this->tokens[ $stackPtr ]['content'] ) ]; - $this->phpcsFile->addError( $message, $stackPtr, 'End', $data ); - - return; - } - } elseif ( T_IS_IDENTICAL === $this->tokens[ $stackPtr ]['code'] && T_IS_IDENTICAL === $this->tokens[ $stackPtr + 1 ]['code'] ) { - $this->addSeparatorError( $stackPtr ); - - return; - } - } - - /** - * Consolidated violation. - * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. - */ - private function addSeparatorError( $stackPtr ) { - $message = 'Merge conflict detected. Found "=======" string.'; - $this->phpcsFile->addError( $message, $stackPtr, 'Separator' ); - } - -} diff --git a/WordPressVIPMinimum/Tests/VersionControl/MergeConflictUnitTest.inc b/WordPressVIPMinimum/Tests/VersionControl/MergeConflictUnitTest.inc deleted file mode 100644 index 31965cc3..00000000 --- a/WordPressVIPMinimum/Tests/VersionControl/MergeConflictUnitTest.inc +++ /dev/null @@ -1,13 +0,0 @@ ->>>>>> branch-a -} diff --git a/WordPressVIPMinimum/Tests/VersionControl/MergeConflictUnitTest.php b/WordPressVIPMinimum/Tests/VersionControl/MergeConflictUnitTest.php deleted file mode 100644 index f0e40e67..00000000 --- a/WordPressVIPMinimum/Tests/VersionControl/MergeConflictUnitTest.php +++ /dev/null @@ -1,41 +0,0 @@ - => - */ - public function getErrorList() { - return [ - 4 => 1, - 8 => 1, - 12 => 1, - ]; - } - - /** - * Returns the lines where warnings should occur. - * - * @return array => - */ - public function getWarningList() { - return []; - } - -} diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc index 8d3641bc..e799c264 100644 --- a/WordPressVIPMinimum/ruleset-test.inc +++ b/WordPressVIPMinimum/ruleset-test.inc @@ -604,15 +604,12 @@ function foo() { $c = compact( $a, $b ); // Warning x 2. } -// WordPressVIPMinimum.VersionControl.MergeConflict -function is_prime( $n ) { - if ( 2 === $n ) { - } -//phpcs:ignore Generic.PHP.Syntax.PHPSyntax -======= // Error. - if ( $n % 2 === 0 ) { - } -} +// Generic.VersionControl.GitMergeConflict +?> +<<<<<<< HEAD // Error. + +>>>>>>> // Error. + diff --git a/WordPressVIPMinimum/ruleset-test.php b/WordPressVIPMinimum/ruleset-test.php index a4dd1efd..a901f9ce 100644 --- a/WordPressVIPMinimum/ruleset-test.php +++ b/WordPressVIPMinimum/ruleset-test.php @@ -206,8 +206,9 @@ 595 => 1, 596 => 1, 597 => 1, - 612 => 1, - 618 => 1, + 609 => 1, + 611 => 1, + 615 => 1, ], 'warnings' => [ 32 => 1, diff --git a/WordPressVIPMinimum/ruleset.xml b/WordPressVIPMinimum/ruleset.xml index 1ff8b2a0..7e0d0d68 100644 --- a/WordPressVIPMinimum/ruleset.xml +++ b/WordPressVIPMinimum/ruleset.xml @@ -42,6 +42,7 @@ + From 2c3090417b042caaf45b19921d6a89bda97387d1 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Fri, 3 Jul 2020 15:26:21 +0100 Subject: [PATCH 17/25] Ruleset Tests: Avoid issue with trigger_error() For some reason, the presence of `trigger_error()` in our ruleset tests (either directly, or via the presence of `_deprecated_file()`, was breaking the tests. --- WordPress-VIP-Go/ruleset-test.inc | 2 +- WordPress-VIP-Go/ruleset-test.php | 2 +- WordPressVIPMinimum/ruleset-test.inc | 4 ++-- WordPressVIPMinimum/ruleset-test.php | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index 11c2b764..8e3be3f1 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -510,7 +510,7 @@ $query_args = array( // WordPressVIPMinimum.Security.EscapingVoidReturnFunctions.Found esc_js( _deprecated_argument() ); // Error. esc_js( _deprecated_constructor() ); // Error. -esc_js( _deprecated_file() ); // Error. +// esc_js( _deprecated_file() ); // Error. esc_js( _deprecated_function() ); // Error. esc_js( _deprecated_hook() ); // Error. esc_js( _doing_it_wrong() ); // Error. diff --git a/WordPress-VIP-Go/ruleset-test.php b/WordPress-VIP-Go/ruleset-test.php index 26be9a5a..b653b83d 100644 --- a/WordPress-VIP-Go/ruleset-test.php +++ b/WordPress-VIP-Go/ruleset-test.php @@ -98,7 +98,7 @@ 507 => 1, 511 => 1, 512 => 1, - 513 => 1, + // 513 => 1, 514 => 1, 515 => 1, 516 => 1, diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc index e799c264..632d0aa4 100644 --- a/WordPressVIPMinimum/ruleset-test.inc +++ b/WordPressVIPMinimum/ruleset-test.inc @@ -180,7 +180,7 @@ $output = shell_exec( 'ls -lart' ); // Error. var_dump(); // Warning. var_export(); // Warning. print_r(); // Warning. -trigger_error(); // Warning. +// trigger_error(); // Warning. set_error_handler(); // Warning. debug_backtrace(); // Warning. debug_print_backtrace(); // Warning. @@ -508,7 +508,7 @@ $query_args = array( // WordPressVIPMinimum.Security.EscapingVoidReturnFunctions.Found esc_js( _deprecated_argument() ); // Error. esc_js( _deprecated_constructor() ); // Error. -esc_js( _deprecated_file() ); // Error. +// esc_js( _deprecated_file() ); // Error. esc_js( _deprecated_function() ); // Error. esc_js( _deprecated_hook() ); // Error. esc_js( _doing_it_wrong() ); // Error. diff --git a/WordPressVIPMinimum/ruleset-test.php b/WordPressVIPMinimum/ruleset-test.php index a901f9ce..b3376134 100644 --- a/WordPressVIPMinimum/ruleset-test.php +++ b/WordPressVIPMinimum/ruleset-test.php @@ -177,7 +177,7 @@ 505 => 1, 509 => 1, 510 => 1, - 511 => 1, + // 511 => 1, 512 => 1, 513 => 1, 514 => 1, @@ -235,7 +235,7 @@ 180 => 1, 181 => 1, 182 => 1, - 183 => 1, + // 183 => 1, 184 => 1, 185 => 1, 186 => 1, From 4a8e464a664cfdcc1ebd6c9cac6b754d592e0b4f Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Sun, 18 Aug 2019 13:35:42 +0100 Subject: [PATCH 18/25] HTMLExecutingFunctions: Add more functions There are different ways to insert variables into the DOM, and our sniffs only covered some of them. The sniff now has an expanded list, which covers functions that have a syntax where the content is in the function arg, and also where the function arg is the target and the content is in the preceding method's arg. Fixes #435. --- .../Sniffs/JS/HTMLExecutingFunctionsSniff.php | 74 ++++++++++++++----- .../JS/HTMLExecutingFunctionsUnitTest.js | 49 +++++++++--- .../JS/HTMLExecutingFunctionsUnitTest.php | 20 ++++- 3 files changed, 116 insertions(+), 27 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php index a06b0aa2..bd4d4240 100644 --- a/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php @@ -22,13 +22,26 @@ class HTMLExecutingFunctionsSniff extends Sniff { /** * List of HTML executing functions. * + * Name of function => content or target. + * Value indicates whether the function's arg is the content to be inserted, or the target where the inserted + * content is to be inserted before/after/replaced. For the latter, the content is in the preceding method's arg. + * * @var array */ public $HTMLExecutingFunctions = [ - 'html', - 'append', - 'write', - 'writeln', + 'after' => 'content', // jQuery. + 'append' => 'content', // jQuery. + 'appendTo' => 'target', // jQuery. + 'before' => 'content', // jQuery. + 'html' => 'content', // jQuery. + 'insertAfter' => 'target', // jQuery. + 'insertBefore' => 'target', // jQuery. + 'prepend' => 'content', // jQuery. + 'prependTo' => 'target', // jQuery. + 'replaceAll' => 'target', // jQuery. + 'replaceWith' => 'content', // jQuery. + 'write' => 'content', + 'writeln' => 'content', ]; /** @@ -58,29 +71,56 @@ public function register() { */ public function process_token( $stackPtr ) { - if ( false === in_array( $this->tokens[ $stackPtr ]['content'], $this->HTMLExecutingFunctions, true ) ) { + if ( ! isset( $this->HTMLExecutingFunctions[ $this->tokens[ $stackPtr ]['content'] ] ) ) { // Looking for specific functions only. return; } - $nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true ); + if ( 'content' === $this->HTMLExecutingFunctions[ $this->tokens[ $stackPtr ]['content'] ] ) { + $nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true ); - if ( T_OPEN_PARENTHESIS !== $this->tokens[ $nextToken ]['code'] ) { - // Not a function. - return; - } + if ( T_OPEN_PARENTHESIS !== $this->tokens[ $nextToken ]['code'] ) { + // Not a function. + return; + } + + $parenthesis_closer = $this->tokens[ $nextToken ]['parenthesis_closer']; - $parenthesis_closer = $this->tokens[ $nextToken ]['parenthesis_closer']; + while ( $nextToken < $parenthesis_closer ) { + $nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true ); + if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) { // Contains a variable. + $message = 'Any HTML passed to `%s` gets executed. Make sure it\'s properly escaped.'; + $data = [ $this->tokens[ $stackPtr ]['content'] ]; + $this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data ); - while ( $nextToken < $parenthesis_closer ) { - $nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true ); - if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) { - $message = 'Any HTML passed to `%s` gets executed. Make sure it\'s properly escaped.'; - $data = [ $this->tokens[ $stackPtr ]['content'] ]; - $this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data ); + return; + } + } + } elseif ( 'target' === $this->HTMLExecutingFunctions[ $this->tokens[ $stackPtr ]['content'] ] ) { + $prevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 1, null, true, null, true ); + if ( T_OBJECT_OPERATOR !== $this->tokens[ $prevToken ]['code'] ) { return; } + + $prevPrevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prevToken - 1, null, true, null, true ); + + if ( T_CLOSE_PARENTHESIS !== $this->tokens[ $prevPrevToken ]['code'] ) { + return; + } + + $parenthesis_opener = $this->tokens[ $prevPrevToken ]['parenthesis_opener']; + + while ( $prevPrevToken > $parenthesis_opener ) { + $prevPrevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prevPrevToken - 1, null, true, null, true ); + if ( T_STRING === $this->tokens[ $prevPrevToken ]['code'] ) { // Contains a variable. + $message = 'Any HTML used with `%s` gets executed. Make sure it\'s properly escaped.'; + $data = [ $this->tokens[ $stackPtr ]['content'] ]; + $this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data ); + + return; + } + } } } diff --git a/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js b/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js index 1a59cc5f..37ce0294 100644 --- a/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js +++ b/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js @@ -1,13 +1,44 @@ (function(){ var el = document.querySelector(".myclass"); - el.html(''); // OK. - el.html('Hand written HTML'); // OK. - el.html( '' + variable + '' ); // NOK. - el.html( variable ); // NOK. - el.append( variable ); // NOK. + el.after(''); // OK. + el.after('Hand written HTML'); // OK. + el.after( '' + variable + '' ); // Warning. + el.after( variable ); // Warning. el.append( 'Hand written HTML' ); // OK. - el.append( '' + variable + '' ); // NOK. + el.append( '' + variable + '' ); // Warning. + el.append( variable ); // Warning. + el.before('Hand written HTML'); // OK. + el.before( '' + variable + '' ); // Warning. + el.before( variable ); // Warning. + el.html('Hand written HTML'); // OK. + el.html( '' + variable + '' ); // Warning. + el.html( variable ); // Warning. + el.prepend('Hand written HTML'); // OK. + el.prepend( '' + variable + '' ); // Warning. + el.prepend( variable ); // Warning. + el.replaceWith('Hand written HTML'); // OK. + el.replaceWith( '' + variable + '' ); // Warning. + el.replaceWith( variable ); // Warning. document.write( '' ); // OK. No variable, conscious. - document.write( hello ); // NOK. - document.writeln( hey ); // NOK. -})(); \ No newline at end of file + document.write( hello ); // Warning. + document + . writeln( hey ); // Warning. + + $('').appendTo(el); // OK. + $('Hand written HTML').appendTo( el ); // OK. + $( '' + variable + '' ).appendTo( el ); // Warning. + $( variable ).appendTo( el ); // Warning. + $('Hand written HTML').insertAfter( el ); // OK. + $( '' + variable + '' ).insertAfter( el ); // Warning. + $( variable ).insertAfter( el ); // Warning. + $('Hand written HTML').insertBefore( el ); // OK. + $( '' + variable + '' ).insertBefore( el ); // Warning. + $( variable ).insertBefore( el ); // Warning. + $('Hand written HTML').prependTo( el ); // OK. + $( '' + variable + '' ).prependTo( el ); // Warning. + $( variable ).prependTo( el ); // Warning. + $('Hand written HTML').replaceAll( el ); // OK. + $( '' + variable + '' ).replaceAll( el ); // Warning. + $( variable ) + . replaceAll( el ); // Warning. +})(); diff --git a/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php index 7fc534a4..19a7c749 100644 --- a/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php @@ -34,10 +34,28 @@ public function getWarningList() { return [ 5 => 1, 6 => 1, - 7 => 1, + 8 => 1, 9 => 1, 11 => 1, 12 => 1, + 14 => 1, + 15 => 1, + 17 => 1, + 18 => 1, + 20 => 1, + 21 => 1, + 23 => 1, + 25 => 1, + 29 => 1, + 30 => 1, + 32 => 1, + 33 => 1, + 35 => 1, + 36 => 1, + 38 => 1, + 39 => 1, + 41 => 1, + 43 => 1, ]; } From a2ea4408800d19c1e2f1ef215641944a6f659a20 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Fri, 3 Jul 2020 20:18:43 +0100 Subject: [PATCH 19/25] HTMLExecutingFunctions: Add further test cases The extra unit test cases: - consider if the contents of a function was not a string but a function call (and update the inline comments to reflect that); this case was already correctly handled. - consider the case when the object that `appendTo()` (and similar) is being called in is a variable, and not a `$()` call (which we already check to see if it contains a simple string to avoid unnecessary violations) or other function call. --- .../Sniffs/JS/HTMLExecutingFunctionsSniff.php | 11 +++++++++-- .../Tests/JS/HTMLExecutingFunctionsUnitTest.js | 4 ++++ .../Tests/JS/HTMLExecutingFunctionsUnitTest.php | 2 ++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php index bd4d4240..dc76da4a 100644 --- a/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php @@ -88,7 +88,7 @@ public function process_token( $stackPtr ) { while ( $nextToken < $parenthesis_closer ) { $nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextToken + 1, null, true, null, true ); - if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) { // Contains a variable. + if ( T_STRING === $this->tokens[ $nextToken ]['code'] ) { // Contains a variable, function call or something else dynamic. $message = 'Any HTML passed to `%s` gets executed. Make sure it\'s properly escaped.'; $data = [ $this->tokens[ $stackPtr ]['content'] ]; $this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data ); @@ -106,14 +106,21 @@ public function process_token( $stackPtr ) { $prevPrevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prevToken - 1, null, true, null, true ); if ( T_CLOSE_PARENTHESIS !== $this->tokens[ $prevPrevToken ]['code'] ) { + // Not a function call, but may be a variable containing an element reference, so just + // flag all remaining instances of these target HTML executing functions. + $message = 'Any HTML used with `%s` gets executed. Make sure it\'s properly escaped.'; + $data = [ $this->tokens[ $stackPtr ]['content'] ]; + $this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data ); + return; } + // Check if it's a function call (typically $() ) that contains a dynamic part. $parenthesis_opener = $this->tokens[ $prevPrevToken ]['parenthesis_opener']; while ( $prevPrevToken > $parenthesis_opener ) { $prevPrevToken = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prevPrevToken - 1, null, true, null, true ); - if ( T_STRING === $this->tokens[ $prevPrevToken ]['code'] ) { // Contains a variable. + if ( T_STRING === $this->tokens[ $prevPrevToken ]['code'] ) { // Contains a variable, function call or something else dynamic. $message = 'Any HTML used with `%s` gets executed. Make sure it\'s properly escaped.'; $data = [ $this->tokens[ $stackPtr ]['content'] ]; $this->phpcsFile->addWarning( $message, $stackPtr, $this->tokens[ $stackPtr ]['content'], $data ); diff --git a/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js b/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js index 37ce0294..ac9d31a0 100644 --- a/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js +++ b/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.js @@ -42,3 +42,7 @@ $( variable ) . replaceAll( el ); // Warning. })(); + + $( foo_that_contains_script_element() ).appendTo( el ); // Warning. + var $foo = $( '.my-selector' ); + $foo.appendTo( el ); // Warning. diff --git a/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php index 19a7c749..794ec61f 100644 --- a/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/JS/HTMLExecutingFunctionsUnitTest.php @@ -56,6 +56,8 @@ public function getWarningList() { 39 => 1, 41 => 1, 43 => 1, + 46 => 1, + 48 => 1, ]; } From d9a7529d3936a1eb36573b7c50eb348a766494b3 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Mon, 9 Dec 2019 14:58:56 +0000 Subject: [PATCH 20/25] Travis: test against PHP 7.4, not snapshot Also test against a `nightly` (PHP 8.0), which is allowed to fail. --- .travis.yml | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index bb2bd8ef..bb2e70c8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,7 +18,8 @@ php: - 7.1 - 7.2 - 7.3 - - "7.4snapshot" + - 7.4 + - "nightly" # Rather than a `matrix` property, we use build stages. This allows early # build failure for basic linting and sniffing issues. @@ -31,7 +32,7 @@ stages: jobs: allow_failures: - - php: "7.4snapshot" + - php: "nightly" include: - stage: lint @@ -73,9 +74,20 @@ before_install: # https://twitter.com/kelunik/status/954242454676475904 - phpenv config-rm xdebug.ini || echo 'No xdebug config.' + # On stable PHPCS versions, allow for PHP deprecation notices. + # Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore. + - | + if [[ "$TRAVIS_BUILD_STAGE_NAME" != "Sniff" && $PHPCS_BRANCH != "dev-master" ]]; then + echo 'error_reporting = E_ALL & ~E_DEPRECATED' >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini + fi + install: - composer require squizlabs/php_codesniffer:"$PHPCS_BRANCH" --update-no-dev --no-suggest --no-scripts - - composer install --dev --no-suggest + - | + # Don't need dev dependencies for running `php -l`. + if [[ "$TRAVIS_BUILD_STAGE_NAME" != "Lint" ]]; then + composer install --dev --no-suggest + fi script: # Run the unit tests. From 94536bae77952418be739dc4103b0e89e0de4977 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Mon, 6 Jul 2020 11:35:49 +0100 Subject: [PATCH 21/25] Add release template --- .github/ISSUE_TEMPLATE/release-template.md | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/release-template.md diff --git a/.github/ISSUE_TEMPLATE/release-template.md b/.github/ISSUE_TEMPLATE/release-template.md new file mode 100644 index 00000000..4d3f1063 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/release-template.md @@ -0,0 +1,24 @@ +--- +name: Release template +about: Internally used for new releases +title: Release 2.x.y +labels: 'Type: Maintenance' +assignees: GaryJones, rebeccahum + +--- + +⚠️ DO NOT MERGE (YET) ⚠️ + +Please do add approvals if you agree. + +PR for tracking changes for the 2.1.0 release. Target release date: Tuesday July 7. + +- [ ] Add changelog for this release. +- [ ] Merge this PR. +- [ ] Add release tag against `master`. +- [ ] Close the current milestone. +- [ ] Open a new milestone for the next release. +- [ ] If any open PRs/issues which were milestoned for this release do not make it into the release, update their milestone. +- [ ] Write a Lobby post. +- [ ] Write an internal P2 post. +- [ ] Open PR to update [Review Bot dependencies](https://github.com/Automattic/vip-go-ci/blob/master/tools-init.sh). From f4774911fbfa283c45e6be24437cb8677a66b056 Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Mon, 6 Jul 2020 11:37:20 +0100 Subject: [PATCH 22/25] Update .github/ISSUE_TEMPLATE/release-template.md --- .github/ISSUE_TEMPLATE/release-template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/release-template.md b/.github/ISSUE_TEMPLATE/release-template.md index 4d3f1063..534e45e4 100644 --- a/.github/ISSUE_TEMPLATE/release-template.md +++ b/.github/ISSUE_TEMPLATE/release-template.md @@ -11,7 +11,7 @@ assignees: GaryJones, rebeccahum Please do add approvals if you agree. -PR for tracking changes for the 2.1.0 release. Target release date: Tuesday July 7. +PR for tracking changes for the 2.x.y release. Target release date: DOW DD MMMM. - [ ] Add changelog for this release. - [ ] Merge this PR. From 9ae36967872efbef52528294b6a84e6e95a5b17f Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Mon, 6 Jul 2020 13:11:54 +0100 Subject: [PATCH 23/25] Travis: Use PHPUnit 7 with PHP 8 By default, tests on PHP 8 use PHPUnit 5.2.7, which includes calls to `each()`, which was removed in PHP 8, and therefore fails. By ignoring the platform requirements, then PHPUnit 7 should be installed. --- .travis.yml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index bb2e70c8..83c5f127 100644 --- a/.travis.yml +++ b/.travis.yml @@ -62,7 +62,7 @@ jobs: php: 7.3 env: PHPCS_BRANCH="dev-master" before_install: phpenv config-rm xdebug.ini || echo 'No xdebug config.' - install: composer install --dev --no-suggest + install: composer install --no-suggest script: # Run PHPCS against VIPCS. - ./bin/phpcs @@ -82,11 +82,14 @@ before_install: fi install: - - composer require squizlabs/php_codesniffer:"$PHPCS_BRANCH" --update-no-dev --no-suggest --no-scripts + - composer require squizlabs/php_codesniffer:"$PHPCS_BRANCH" --no-update --no-suggest --no-scripts - | - # Don't need dev dependencies for running `php -l`. - if [[ "$TRAVIS_BUILD_STAGE_NAME" != "Lint" ]]; then - composer install --dev --no-suggest + if [[ $TRAVIS_PHP_VERSION == "nightly" ]]; then + # PHPUnit 7.x does not allow for installation on PHP 8, so ignore platform + # requirements to get PHPUnit 7.x to install on nightly. + composer install --ignore-platform-reqs --no-suggest + else + composer install --no-suggest fi script: From d61364aa81e49b9085764d6f4486ed55d116d5ae Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Mon, 6 Jul 2020 14:03:04 +0100 Subject: [PATCH 24/25] Travis: Expand supported PHP down to 5.4 PHP 5.4 is the minimum that PHPCS and WPCS support, so it would be nice to be able to say VIPCS supports the same minimum version of PHP. --- .travis.yml | 8 +++++--- README.md | 1 + composer.json | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index bb2e70c8..33d17326 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: php os: linux -dist: xenial +dist: trusty env: # `master` is now 3.x. @@ -13,6 +13,8 @@ cache: - $HOME/.cache/composer/files php: + - 5.4 + - 5.5 - 5.6 - 7.0 - 7.1 @@ -36,7 +38,7 @@ jobs: include: - stage: lint - php: 7.3 + php: 7.4 env: PHPCS_BRANCH="dev-master" before_install: phpenv config-rm xdebug.ini || echo 'No xdebug config.' install: skip @@ -59,7 +61,7 @@ jobs: - libxml2-utils - stage: sniff - php: 7.3 + php: 7.4 env: PHPCS_BRANCH="dev-master" before_install: phpenv config-rm xdebug.ini || echo 'No xdebug config.' install: composer install --dev --no-suggest diff --git a/README.md b/README.md index 4ac0c9c4..9b3626b5 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ Go to https://wpvip.com/documentation/phpcs-review-feedback/ to learn about why ## Minimal requirements +* PHP 5.4+ * [PHPCS 3.3.1+](https://github.com/squizlabs/PHP_CodeSniffer/releases) * [WPCS 2.*](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases) diff --git a/composer.json b/composer.json index d545d818..27133da1 100644 --- a/composer.json +++ b/composer.json @@ -15,14 +15,14 @@ } ], "require": { - "php": ">=5.6", + "php": ">=5.4", "squizlabs/php_codesniffer": "^3.5.5", "wp-coding-standards/wpcs": "^2.3" }, "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7", "phpcompatibility/php-compatibility": "^9", - "phpunit/phpunit": "^5 || ^6 || ^7" + "phpunit/phpunit": "^4 || ^5 || ^6 || ^7" }, "suggest": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7 || This Composer plugin will manage the PHPCS 'installed_paths' automatically." From 4a8cb7dd9421c00d7db2eeace784e8aec7d282ae Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Sun, 5 Jul 2020 07:51:48 +0100 Subject: [PATCH 25/25] Docs: Changelog for 2.1.0 --- CHANGELOG.md | 32 ++++++++++++++++++++++++++++++++ README.md | 6 +++--- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff9762f0..d82cf157 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,37 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.1.0] - 2020-07-07 + +Bumps requirements to PHPCS 3.5.5+ and WPCS 2.3.0+. + +### Added + +- `get_page_by_path()` restricted function warning, to suggest `wpcom_vip_get_page_by_path()` function. +- `stats_get_csv()` restricted function error, since this is a Jetpack-only function. +- Expanded list of HTMLExecutingFunctions to include `after`, `appendTo`, `before`, `insertAfter`, `insertBefore`, `prepend`, `prependTo`, `replaceAll` and `replaceWith`. +- Support PHP 5.4+ (down from 5.6+). +- PHP 8 nightly testing. + +### Changed + +- Expand message for `wp_remote_get()` usage. +- Downgrade `append()` usage violation from Error to Warning for VIP Go, to be consistent with the other HTMLExecutingFunctions. +- Downgrade AdminBarRemoval sniff from Error to Warning for VIP Go. +- Add `get_parent_theme_file_path()` to safelist of path functions for `WordPressVIPMinimum.Files.IncludingFile` sniff. +- Allow short array syntax and fix tests within the VIPCS own coding standards. +- Update issue templates. + +### Fixed + +- Use new `WordPress.DateTime.RestrictedFunctions` sniff instead of deprecated `WordPress.WP.TimezoneChange`. +- Fixed warnings and information items in Travis. + +### Removed + +- `get_super_admins()` restricted function rule for VIP Go. +- `WordPressVIPMinimum.VersionControl.MergeConflict` sniff in favour of `Generic.VersionControl.GitMergeConflict`. + ## [2.0.0] - 2019-07-12 This release switches from having WPCS `1.*` as a dependency, to WPCS `2.*`. It is not compatible with WPCS `1.*`. @@ -367,5 +398,6 @@ This release contains breaking changes. - `wpcom_vip_get_page_by_path` from `WordPressVIPMinimum.VIP.RestrictedFunctions` - Version check for PHP 7 or less in `WordPressVIPMinimum.Variables.VariableAnalysis` unit test since tests are not failing anymore. +[2.1.0]: https://github.com/Automattic/VIP-Coding-Standards/compare/2.0.0...2.1.0 [2.0.0]: https://github.com/Automattic/VIP-Coding-Standards/compare/1.0.0...2.0.0 [1.0.0]: https://github.com/Automattic/VIP-Coding-Standards/compare/0.4.0...1.0.0 diff --git a/README.md b/README.md index 9b3626b5..ee2d7938 100644 --- a/README.md +++ b/README.md @@ -16,8 +16,8 @@ Go to https://wpvip.com/documentation/phpcs-review-feedback/ to learn about why ## Minimal requirements * PHP 5.4+ -* [PHPCS 3.3.1+](https://github.com/squizlabs/PHP_CodeSniffer/releases) -* [WPCS 2.*](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases) +* [PHPCS 3.5.5+](https://github.com/squizlabs/PHP_CodeSniffer/releases) +* [WPCS 2.3.0+](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases) ## Installation @@ -32,7 +32,7 @@ We recommend the [PHP_CodeSniffer Standards Composer Installer Plugin](https://g Alternatively, you should register the standard to PHPCS by appending the VIPCS directory to the end of the installed paths. e.g. -`phpcs --config-set installed_paths [/path/to/wpcsstandard],[path/to/vipcsstandard],etc` +`phpcs --config-set installed_paths /path/to/wpcsstandard,path/to/vipcsstandard,etc.` ## Contribution