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

3.0: support WordPressCS 3.0 #779

Merged
merged 11 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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' ) );
}
}
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';
}
}
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