Skip to content

Commit

Permalink
Merge pull request #779 from Automattic/3.0/updates-for-wpcs-3.0
Browse files Browse the repository at this point in the history
  • Loading branch information
GaryJones authored Aug 25, 2023
2 parents 9b5c899 + 36f1684 commit 0fd1c85
Show file tree
Hide file tree
Showing 18 changed files with 109 additions and 64 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/basics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
- name: Install PHP
uses: shivammathur/setup-php@v2
with:
php-version: '7.4'
php-version: 'latest'
coverage: none
tools: cs2pr

Expand Down
13 changes: 6 additions & 7 deletions .github/workflows/quicktest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ jobs:
include:
- php: '5.4'
phpcs_version: 'dev-master'
wpcs_version: '2.3.*'
wpcs_version: '3.0.*'
- php: '5.4'
phpcs_version: '3.7.1'
wpcs_version: '2.3.*'
phpcs_version: '3.7.2'
wpcs_version: '3.0.*'

- php: 'latest'
phpcs_version: 'dev-master'
wpcs_version: '2.3.*'
wpcs_version: '3.0.*'
- php: 'latest'
phpcs_version: '3.7.1'
wpcs_version: '2.3.*'
phpcs_version: '3.7.2'
wpcs_version: '3.0.*'

name: "QTest${{ matrix.phpcs_version == 'dev-master' && ' + Lint' || '' }}: PHP ${{ matrix.php }} - PHPCS ${{ matrix.phpcs_version }}"

Expand All @@ -48,7 +48,6 @@ jobs:

# 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.
# Note: the "elif" condition is temporary and should be removed once VIPCS updates to WPCS 3.0+.
- name: Setup ini config
id: set_ini
run: |
Expand Down
9 changes: 3 additions & 6 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,13 @@ jobs:
# no additional versions are included in the array.
matrix:
php: ['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2']
phpcs_version: ['3.7.1', 'dev-master']
wpcs_version: ['2.3.*']
phpcs_version: ['3.7.2', 'dev-master']
wpcs_version: ['3.0.*']

include:
- php: '8.3'
phpcs_version: 'dev-master'
wpcs_version: '2.3.*'
wpcs_version: '3.0.*'

name: "Test: PHP ${{ matrix.php }} - PHPCS ${{ matrix.phpcs_version }} - WPCS ${{ matrix.wpcs_version }}"

Expand All @@ -90,14 +90,11 @@ jobs:

# 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.
# Note: the "elif" condition is temporary and should be removed once VIPCS updates to WPCS 3.0+.
- name: Setup ini config
id: set_ini
run: |
if [[ "${{ matrix.phpcs_version }}" != "dev-master" ]]; then
echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED' >> $GITHUB_OUTPUT
elif [[ "${{ matrix.php }}" == "8.1" ]]; then
echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED' >> $GITHUB_OUTPUT
else
echo 'PHP_INI=error_reporting=-1' >> $GITHUB_OUTPUT
fi
Expand Down
3 changes: 1 addition & 2 deletions .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
<rule ref="WordPress-Extra">
<exclude name="WordPress.Files.FileName"/>
<exclude name="WordPress.NamingConventions.ValidVariableName"/>
<exclude name="WordPress.NamingConventions.PrefixAllGlobals"/>
<exclude name="Generic.Arrays.DisallowShortArraySyntax"/>
<exclude name="Universal.Arrays.DisallowShortArraySyntax"/>
<exclude name="WordPress.PHP.YodaConditions"/>
</rule>

Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ Go to https://docs.wpvip.com/technical-references/code-review/phpcs-report/ to l
## Minimal requirements

* PHP 5.4+
* [PHPCS 3.7.1+](https://github.com/squizlabs/PHP_CodeSniffer/releases)
* [PHPCS 3.7.2+](https://github.com/squizlabs/PHP_CodeSniffer/releases)
* [PHPCSUtils 1.0.8+](https://github.com/PHPCSStandards/PHPCSUtils)
* [WPCS 2.3.0+](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases)
* [WPCS 3.0.0+](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases)
* [VariableAnalysis 2.11.17+](https://github.com/sirbrillig/phpcs-variable-analysis/releases)

## Installation
Expand All @@ -35,7 +35,7 @@ composer g config allow-plugins.dealerdirect/phpcodesniffer-composer-installer t
composer g require automattic/vipwpcs
```

This will install the latest compatible versions of PHPCS, PHPCSUtils, WPCS and VariableAnalysis and register the external standards with PHP_CodeSniffer.
This will install the latest compatible versions of PHPCS, PHPCSUtils, PHPCSExtra, WPCS and VariableAnalysis and register the external standards with PHP_CodeSniffer.

Please refer to the [installation instructions for installing PHP_CodeSniffer for WordPress.com VIP](https://docs.wpvip.com/how-tos/code-review/php_codesniffer/) for more details.

Expand Down
13 changes: 9 additions & 4 deletions WordPress-VIP-Go/ruleset-test.inc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ if ( isset( $_SERVER['HTTP_USER_AGENT'] ) && $_SERVER['HTTP_USER_AGENT'] === 'so
// Make sure nonce verification is done in global scope to silence notices about use of superglobals without later on in the file.
isset( $_GET['my_nonce'] ) && wp_verify_nonce( sanitize_text_field( $_GET['my_nonce'] ) );

// WordPress.WP.AlternativeFunctions.file_system_read_fopen
// WordPress.WP.AlternativeFunctions.file_system_operations_fopen
fopen( 'file.txt', 'r' ); // Warning + Message.

// WordPressVIPMinimum.Performance.FetchingRemoteData.FileGetContentsUnknown
Expand Down Expand Up @@ -153,7 +153,7 @@ url_to_postid( $url ); // Warning + Message.
wpcom_vip_old_slug_redirect(); // Ok.
wp_old_slug_redirect(); // Warning.

// WordPress.CodeAnalysis.AssignmentInCondition.Found
// Generic.CodeAnalysis.AssignmentInCondition.Found
if ($a = 123) { // Warning.
}

Expand All @@ -165,7 +165,7 @@ rawurlencode(); // Ok.
extract( array( 'a' => 1 ) ); // Error.
$obj->extract(); // Ok.

// WordPress.PHP.StrictComparisons.LooseComparison
// Universal.Operators.StrictComparisons
true == $true; // Warning.
false === $true; // Ok.

Expand Down Expand Up @@ -557,7 +557,7 @@ echo "<script>
</article> <?php

// WordPressVIPMinimum.Variables.RestrictedVariables
foo( $_SESSION['bar'] ); // Error.
foo( $_SESSION['bar'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- Error.

// WordPressVIPMinimum.Variables.ServerVariables
// phpcs:disable WordPress.Security.ValidatedSanitizedInput.InputNotValidated,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
Expand All @@ -572,3 +572,8 @@ $_SERVER["REMOTE_ADDR"]; // Error.
<<<<<<< HEAD // Error.

>>>>>>> // Error.

<?php

// WordPress.CodeAnalysis.AssignmentInTernaryCondition
$var = ($a = 123) ? $a : 0; // Warning.
1 change: 1 addition & 0 deletions WordPress-VIP-Go/ruleset-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@
540 => 1,
550 => 1,
556 => 1,
579 => 1,
],
'messages' => [
7 => [
Expand Down
13 changes: 8 additions & 5 deletions WordPress-VIP-Go/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
This includes potential security holes as well as functions that may bring down sites for performance reasons.
-->
<!-- Should fix all of them but it doesn't need a manual review -->
<rule ref="WordPress.WP.AlternativeFunctions.file_system_read_fopen">
<rule ref="WordPress.WP.AlternativeFunctions.file_system_operations_fopen">
<message>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://docs.wpvip.com/technical-references/vip-go-files-system/local-file-operations/</message>
</rule>
<rule ref="WordPressVIPMinimum.Performance.FetchingRemoteData.FileGetContentsUnknown">
Expand Down Expand Up @@ -179,7 +179,10 @@
<rule ref="Internal.LineEndings.Mixed">
<severity>1</severity>
</rule>
<rule ref="WordPress.CodeAnalysis.AssignmentInCondition.Found">
<rule ref="Generic.CodeAnalysis.AssignmentInCondition">
<severity>1</severity>
</rule>
<rule ref="WordPress.CodeAnalysis.AssignmentInTernaryCondition.FoundInTernaryCondition">
<severity>1</severity>
</rule>
<rule ref="WordPress.PHP.DiscouragedPHPFunctions.urlencode_urlencode">
Expand All @@ -188,7 +191,7 @@
<rule ref="WordPress.PHP.DontExtract">
<severity>3</severity>
</rule>
<rule ref="WordPress.PHP.StrictComparisons.LooseComparison">
<rule ref="Universal.Operators.StrictComparisons">
<severity>3</severity>
</rule>
<rule ref="WordPress.PHP.StrictInArray.MissingTrueStrict">
Expand Down Expand Up @@ -246,10 +249,10 @@
<rule ref="Generic.PHP.DisallowShortOpenTag.EchoFound">
<severity>0</severity>
</rule>
<rule ref="WordPress.WP.AlternativeFunctions.file_system_read_readfile">
<rule ref="WordPress.WP.AlternativeFunctions.file_system_operations_readfile">
<severity>0</severity>
</rule>
<rule ref="WordPress.WP.AlternativeFunctions.file_system_read_fclose">
<rule ref="WordPress.WP.AlternativeFunctions.file_system_operations_fclose">
<severity>0</severity>
</rule>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

use PHPCSUtils\Utils\GetTokensAsString;
use PHPCSUtils\Utils\MessageHelper;
use WordPressCS\WordPress\Helpers\ContextHelper;
use WordPressCS\WordPress\Helpers\RulesetPropertyHelper;

/**
* Restricts usage of some variables.
Expand Down Expand Up @@ -128,7 +130,7 @@ public function process_token( $stackPtr ) {

$token = $this->tokens[ $stackPtr ];

$this->excluded_groups = static::merge_custom_array( $this->exclude );
$this->excluded_groups = RulesetPropertyHelper::merge_custom_array( $this->exclude );
if ( array_diff_key( $this->groups_cache, $this->excluded_groups ) === [] ) {
// All groups have been excluded.
// Don't remove the listener as the exclude property can be changed inline.
Expand All @@ -144,7 +146,7 @@ public function process_token( $stackPtr ) {
}
}

if ( $this->is_in_isset_or_empty( $stackPtr ) === true ) {
if ( ContextHelper::is_in_isset_or_empty( $this->phpcsFile, $stackPtr ) === true ) {
// Checking whether a variable exists is not the same as using it.
return;
}
Expand Down
2 changes: 1 addition & 1 deletion WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ public function getGroups() {
public function callback( $key, $val, $line, $group ) {
$key = strtolower( $key );

return ( $key === 'nopaging' && ( $val === 'true' || $val === 1 ) );
return ( $key === 'nopaging' && ( $val === 'true' || $val === '1' ) );
}
}
4 changes: 3 additions & 1 deletion WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace WordPressVIPMinimum\Sniffs\Performance;

use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;

/**
Expand All @@ -31,7 +32,7 @@ public function getGroups() {
return [
'orderby' => [
'type' => 'error',
'message' => 'Detected forbidden query_var "%s" of "%s". Use vip_get_random_posts() instead.',
'message' => 'Detected forbidden query_var "%s" of %s. Use vip_get_random_posts() instead.',
'keys' => [
'orderby',
],
Expand All @@ -50,6 +51,7 @@ public function getGroups() {
* @return bool FALSE if no match, TRUE if matches.
*/
public function callback( $key, $val, $line, $group ) {
$val = TextStrings::stripQuotes( $val );
return strtolower( $val ) === 'rand';
}
}
2 changes: 2 additions & 0 deletions WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

namespace WordPressVIPMinimum\Sniffs\Performance;

use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;

/**
Expand Down Expand Up @@ -45,6 +46,7 @@ public function getGroups() {
* @return bool FALSE if no match, TRUE if matches.
*/
public function callback( $key, $val, $line, $group ) {
$val = TextStrings::stripQuotes( $val );
return ( strpos( $val, 'NOT REGEXP' ) === 0
|| strpos( $val, 'REGEXP' ) === 0
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,22 @@
namespace WordPressVIPMinimum\Sniffs\Security;

use PHP_CodeSniffer\Util\Tokens;
use WordPressCS\WordPress\Helpers\PrintingFunctionsTrait;
use WordPressVIPMinimum\Sniffs\Sniff;

/**
* Flag functions that don't return anything, yet are wrapped in an escaping function call.
*
* E.g. esc_html( _e( 'foo' ) );
*
* @package VIPCS\WordPressVIPMinimum
* @package VIPCS\WordPressVIPMinimum
*
* @uses \WordPressCS\WordPress\Helpers\PrintingFunctionsTrait::$customPrintingFunctions
*/
class EscapingVoidReturnFunctionsSniff extends Sniff {

use PrintingFunctionsTrait;

/**
* Returns an array of tokens this test wants to listen for.
*
Expand Down Expand Up @@ -59,7 +64,7 @@ public function process_token( $stackPtr ) {
return;
}

if ( isset( $this->printingFunctions[ $this->tokens[ $next_token ]['content'] ] ) ) {
if ( $this->is_printing_function( $this->tokens[ $next_token ]['content'] ) ) {
$message = 'Attempting to escape `%s()` which is printing its output.';
$data = [ $this->tokens[ $next_token ]['content'] ];
$this->phpcsFile->addError( $message, $stackPtr, 'Found', $data );
Expand Down
Loading

0 comments on commit 0fd1c85

Please sign in to comment.