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

WPCS 3.0: Fix ruleset tests #736

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cd7e371
Composer: up the minimum PHPCS version to 3.7.1
GaryJones Dec 19, 2022
a3acf35
Composer: rename scripts
GaryJones Dec 20, 2022
b4aa628
Composer: add script descriptions
GaryJones Dec 20, 2022
8fb8805
Composer: use develop branch of WPCS
GaryJones Dec 20, 2022
5a82c31
Composer: Normalize the order
GaryJones Dec 20, 2022
2b893ca
CS: Use pre-increment over post-increment
GaryJones Dec 20, 2022
0b84ea8
CS: Avoid reserved keyword for function param name
GaryJones Dec 20, 2022
b838219
CS: Update WordPress-Extra exclusions
GaryJones Dec 20, 2022
1f6db81
Sniff::strip_quotes(): use PHPCSUtils version
GaryJones Dec 20, 2022
9e6b16e
Sniff::find_array_open_close(): PHPCSUtils version
GaryJones Dec 20, 2022
90a4658
Sniff::get_function_call_parameter(): PHPCSUtils
GaryJones Dec 20, 2022
e948dcb
Add Sniff::merge_custom_array()
GaryJones Dec 20, 2022
2bb34af
Use PHPCSUtils MessageHelper::addMessage
GaryJones Dec 20, 2022
03e7924
CS: fix coding standards for merge_custom_array()
GaryJones Dec 20, 2022
80ca8ab
Composer: Add PHPCSUtils as required dependency
GaryJones Dec 20, 2022
5408a46
CI: Refresh unit tests workflow
GaryJones Dec 20, 2022
8e8792d
Disable ruleset test
GaryJones Dec 20, 2022
5ae2e23
Ruleset tests: update for WPCS 3.0
GaryJones Dec 20, 2022
92e10d3
Ruleset tests: Add label before test runs
GaryJones Dec 20, 2022
d6df86d
Enable ruleset tests
GaryJones Dec 20, 2022
1b968b7
Ruleset tests: Update label
GaryJones Dec 20, 2022
5b33691
Add PHPCSExtras dependency
GaryJones Dec 20, 2022
ce7d0c4
CI: Use WPCS develop branch for quick test
GaryJones Dec 20, 2022
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
32 changes: 15 additions & 17 deletions .github/workflows/quicktest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,14 @@ jobs:
include:
- php: '5.4'
phpcs_version: 'dev-master'
wpcs_version: '2.3.*'
wpcs_version: 'dev-develop'
- php: '5.4'
phpcs_version: '3.5.5'
wpcs_version: '2.3.*'
phpcs_version: '3.7.1'
wpcs_version: 'dev-develop'

- php: 'latest'
phpcs_version: 'dev-master'
wpcs_version: '2.3.*'
- php: 'latest'
# PHPCS 3.6.1 is the lowest version of PHPCS which supports PHP 8.1.
phpcs_version: '3.6.1'
wpcs_version: '2.3.*'
wpcs_version: 'dev-develop'

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

Expand All @@ -54,24 +50,23 @@ jobs:
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 }}" == "latest" ]]; then
echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED' >> $GITHUB_OUTPUT
echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On' >> $GITHUB_OUTPUT
else
echo 'PHP_INI=error_reporting=-1' >> $GITHUB_OUTPUT
echo 'PHP_INI=error_reporting=-1, display_errors=On' >> $GITHUB_OUTPUT
fi

- name: Install PHP
- name: Set up PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
ini-values: ${{ steps.set_ini.outputs.PHP_INI }}
coverage: none

- name: 'Composer: set PHPCS and WPCS versions for tests'
run: |
composer require --no-update --no-scripts squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-interaction
composer require --no-update --no-scripts wp-coding-standards/wpcs:"${{ matrix.wpcs_version }}" --no-interaction
- name: 'Composer: set PHPCS version for tests'
run: composer require squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-update --no-scripts --no-interaction

- name: 'Composer: set WPCS version for tests'
run: composer require wp-coding-standards/wpcs:"${{ matrix.wpcs_version }}" --no-update --no-scripts --no-interaction

# Install dependencies and handle caching in one go.
# @link https://github.com/marketplace/actions/install-composer-dependencies
Expand All @@ -91,6 +86,9 @@ jobs:
composer-options: --ignore-platform-reqs
custom-cache-suffix: $(date -u -d "-0 month -$(($(date +%d)-1)) days" "+%F")

- name: Display PHPCS installed standards
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't validate anything. It just shows the standards, it doesn't error out if expected standards would not be registered.

Also, this information is already available at the bottom of the "Install Composer dependencies" step as the Composer PHPCS plugin will display it as part of its user facing output.

run: ./vendor/bin/phpcs -i

- name: Lint against parse errors
if: matrix.phpcs_version == 'dev-master'
run: ./bin/php-lint
Expand Down
92 changes: 35 additions & 57 deletions .github/workflows/test.yml → .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Test
name: Unit Tests

on:
# Run on pushes to `master` and on all pull requests.
Expand Down Expand Up @@ -27,10 +27,10 @@ jobs:

strategy:
matrix:
php: ['5.4', 'latest', '8.2']
php: ['5.4', 'latest', '8.3']

name: "Lint: PHP ${{ matrix.php }}"
continue-on-error: ${{ matrix.php == '8.2' }}
continue-on-error: ${{ matrix.php == '8.3' }}

steps:
- name: Checkout code
Expand All @@ -46,8 +46,8 @@ jobs:
- name: Install Composer dependencies
uses: "ramsey/composer-install@v2"
with:
# Bust the cache at least once a month - output format: YYYY-MM-DD.
custom-cache-suffix: $(date -u -d "-0 month -$(($(date +%d)-1)) days" "+%F")
# Bust the cache at least once a month - output format: YYYY-MM.
custom-cache-suffix: $(date -u "+%Y-%m")

- name: Lint against parse errors
run: ./bin/php-lint --checkstyle | cs2pr
Expand All @@ -70,41 +70,19 @@ jobs:
# no additional versions are included in the array.
# - experimental: Whether the build is "allowed to fail".
matrix:
php: ['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4']
phpcs_version: ['3.5.5', 'dev-master']
wpcs_version: ['2.3.*']
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: ['dev-develop']
experimental: [false]

include:
# Complete the matrix by adding PHP 8.0, but only test against compatible PHPCS versions.
- php: '8.0'
phpcs_version: 'dev-master'
wpcs_version: '2.3.*'
experimental: false
- php: '8.0'
# PHPCS 3.5.7 is the lowest version of PHPCS which supports PHP 8.0.
phpcs_version: '3.5.7'
wpcs_version: '2.3.*'
experimental: false

# Complete the matrix by adding PHP 8.1, but only test against compatible PHPCS versions.
- php: '8.1'
phpcs_version: 'dev-master'
wpcs_version: '2.3.*'
experimental: false
- php: '8.1'
# PHPCS 3.6.1 is the lowest version of PHPCS which supports PHP 8.1.
phpcs_version: '3.6.1'
wpcs_version: '2.3.*'
experimental: false

# Experimental builds. These are allowed to fail.
#- php: '8.2'
# phpcs_version: 'dev-master'
# wpcs_version: '2.3.*'
# experimental: true
- php: '8.3'
phpcs_version: 'dev-master'
wpcs_version: 'dev-develop'
experimental: true

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

continue-on-error: ${{ matrix.experimental }}

Expand All @@ -119,49 +97,49 @@ jobs:
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
echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On' >> $GITHUB_OUTPUT
else
echo 'PHP_INI=error_reporting=-1' >> $GITHUB_OUTPUT
echo 'PHP_INI=error_reporting=-1, display_errors=On' >> $GITHUB_OUTPUT
fi

- name: Install PHP
- name: Set up PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
ini-values: ${{ steps.set_ini.outputs.PHP_INI }}
coverage: none

- name: 'Composer: set PHPCS and WPCS versions for tests'
run: |
composer require --no-update --no-scripts squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-interaction
composer require --no-update --no-scripts wp-coding-standards/wpcs:"${{ matrix.wpcs_version }}" --no-interaction
- name: 'Composer: set PHPCS version for tests'
run: composer require squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-update --no-scripts --no-interaction

- name: 'Composer: set WPCS version for tests'
run: composer require wp-coding-standards/wpcs:"${{ matrix.wpcs_version }}" --no-update --no-scripts --no-interaction

# Install dependencies and handle caching in one go.
# @link https://github.com/marketplace/actions/install-composer-dependencies
- name: Install Composer dependencies - normal
if: ${{ startsWith( matrix.php, '8' ) == false }}
uses: "ramsey/composer-install@v2"
- name: Install Composer dependencies (PHP < 8.0 )
if: ${{ matrix.php < 8.0 }}
uses: ramsey/composer-install@v2
with:
# Bust the cache at least once a month - output format: YYYY-MM-DD.
custom-cache-suffix: $(date -u -d "-0 month -$(($(date +%d)-1)) days" "+%F")
# Bust the cache at least once a month - output format: YYYY-MM.
custom-cache-suffix: $(date -u "+%Y-%m")

# PHPUnit 7.x does not allow for installation on PHP 8, so ignore platform
# requirements to get PHPUnit 7.x to install on nightly.
- name: Install Composer dependencies - with ignore platform
if: ${{ startsWith( matrix.php, '8' ) }}
uses: "ramsey/composer-install@v2"
- name: Install Composer dependencies (PHP >= 8.0)
if: ${{ matrix.php >= 8.0 }}
uses: ramsey/composer-install@v2
with:
composer-options: --ignore-platform-reqs
custom-cache-suffix: $(date -u -d "-0 month -$(($(date +%d)-1)) days" "+%F")
custom-cache-suffix: $(date -u "+%Y-%m")

- name: Run the unit tests - PHP 5.4 - 8.0
if: matrix.php < '8.1'
# Simplify these next two down once bin/unit-tests.yml contains the condition (#735).
- name: Run the unit tests - PHP 5.4 — 8.0
if: ${{ matrix.php < '8.1' }}
run: ./bin/unit-tests

- name: Run the unit tests - PHP > 8.1
if: matrix.php >= '8.1'
- name: Run the unit tests - PHP >= 8.1
if: ${{ matrix.php >= '8.1' }}
run: vendor/bin/phpunit --filter WordPressVIPMinimum ./vendor/squizlabs/php_codesniffer/tests/AllTests.php --no-coverage --no-configuration --bootstrap=./tests/bootstrap.php --dont-report-useless-tests

- name: Run the ruleset tests
Expand Down
2 changes: 1 addition & 1 deletion .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<rule ref="WordPress-Extra">
<exclude name="WordPress.Files.FileName"/>
<exclude name="WordPress.NamingConventions.ValidVariableName"/>
<exclude name="Generic.Arrays.DisallowShortArraySyntax"/>
<exclude name="Universal.Arrays.DisallowShortArraySyntax"/>
<exclude name="WordPress.PHP.YodaConditions"/>
</rule>

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Go to https://docs.wpvip.com/technical-references/code-review/phpcs-report/ to l
## Minimal requirements

* PHP 5.4+
* [PHPCS 3.5.5+](https://github.com/squizlabs/PHP_CodeSniffer/releases)
* [PHPCS 3.7.1+](https://github.com/squizlabs/PHP_CodeSniffer/releases)
* [WPCS 2.3.0+](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases)
* [VariableAnalysis 2.11.1+](https://github.com/sirbrillig/phpcs-variable-analysis/releases)

Expand Down
4 changes: 2 additions & 2 deletions WordPress-VIP-Go/ruleset-test.inc
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ rawurlencode(); // Ok.
extract( array( 'a' => 1 ) ); // Error.
$obj->extract(); // Ok.

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

Expand Down Expand Up @@ -546,7 +546,7 @@ str_replace( 'foo', array( 'bar', 'foo' ), 'foobar' ); // Error.

// WordPressVIPMinimum.Security.Underscorejs
echo "<script>
_.templateSettings = {
_.templateSettings = {
Copy link
Collaborator

@jrfnl jrfnl Dec 30, 2022

Choose a reason for hiding this comment

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

Was this an intentional or accidental trailing whitespace ? I.e. are you changing what's being tested now or not ?

(same question for the same change in WordPressVIPMinimum/ruleset-test.inc)

interpolate: /\{\{(.+?)\}\}/g" . // Warning.
"};
</script>";
Expand Down
8 changes: 4 additions & 4 deletions WordPress-VIP-Go/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,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 @@ -193,7 +193,7 @@
<rule ref="WordPress.PHP.DontExtract">
<severity>3</severity>
</rule>
<rule ref="WordPress.PHP.StrictComparisons.LooseComparison">
<rule ref="Universal.Operators.StrictComparisons.LooseComparison">
<severity>3</severity>
</rule>
<rule ref="WordPress.PHP.StrictInArray.MissingTrueStrict">
Expand Down Expand Up @@ -263,10 +263,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 @@ -9,7 +9,7 @@

namespace WordPressVIPMinimum\Sniffs;

use WordPressVIPMinimum\Sniffs\Sniff;
use PHPCSUtils\Utils\MessageHelper;

/**
* Restricts usage of some variables.
Expand Down Expand Up @@ -200,11 +200,13 @@ public function process_token( $stackPtr ) {
continue;
}

$this->addMessage(
$code = MessageHelper::stringToErrorcode( $groupName . '_' . $match[1] );
MessageHelper::addMessage(
$this->phpcsFile,
$group['message'],
$stackPtr,
$group['type'] === 'error',
$this->string_to_errorcode( $groupName . '_' . $match[1] ),
$code,
[ $var ]
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currSco
return;
}
}
$i++;
++$i;
}
}

Expand Down
6 changes: 3 additions & 3 deletions WordPressVIPMinimum/Sniffs/Compatibility/ZoninatorSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ public function process_token( $stackPtr ) {
/**
* Removes the quotation marks around T_CONSTANT_ENCAPSED_STRING.
*
* @param string $string T_CONSTANT_ENCAPSED_STRING containing wrapping quotation marks.
* @param string $text_string T_CONSTANT_ENCAPSED_STRING containing wrapping quotation marks.
*
* @return string String w/o wrapping quotation marks.
*/
public function remove_wrapping_quotation_marks( $string ) {
return trim( str_replace( '"', "'", $string ), "'" );
public function remove_wrapping_quotation_marks( $text_string ) {
return trim( str_replace( '"', "'", $text_string ), "'" );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

namespace WordPressVIPMinimum\Sniffs\Constants;

use WordPressVIPMinimum\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\PassedParameters;
use WordPressVIPMinimum\Sniffs\Sniff;

/**
* Sniff for properly using constant name when checking whether a constant is defined.
Expand Down Expand Up @@ -55,7 +56,7 @@ public function process_token( $stackPtr ) {
return;
}

$param = $this->get_function_call_parameter( $stackPtr, 1 );
$param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1 );
if ( $param === false ) {
// Target parameter not found.
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace WordPressVIPMinimum\Sniffs\Functions;

use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\TextStrings;
use WordPressVIPMinimum\Sniffs\Sniff;

/**
Expand Down Expand Up @@ -139,7 +140,7 @@ private function collect_variables() {
* If we reached the end of the loop and the $value_ptr was set, we know for sure
* this was a plain text string variable assignment.
*/
$current_var_value = $this->strip_quotes( $this->tokens[ $value_ptr ]['content'] );
$current_var_value = TextStrings::stripQuotes( $this->tokens[ $value_ptr ]['content'] );

if ( isset( $this->disallowed_functions[ $current_var_value ] ) === false ) {
// Text string is not one of the ones we're looking for.
Expand Down
Loading