From 5d0319f64eaf72c4fc0d4fdb510dba7b7a147eac Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 4 Sep 2023 16:52:11 +0200 Subject: [PATCH 01/24] Issue template: mention PHPCSExtra Noticed this while having a quick look through the release PR. As this is not a file which is included in the release (`export-ignore`d via `.gitattributes`), I'm pulling this to `develop`. --- .github/ISSUE_TEMPLATE/bug_report.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index c089b1d2..f24d6921 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -47,6 +47,7 @@ Use `php -v` and `composer show` to get versions. | PHPCSUtils version | x.y.z | VIPCS version | x.y.z | WordPressCS version | x.y.z +| PHPCSExtra version | x.y.z | VariableAnalysis version | x.y.z ## Additional Context (optional) From d6fe1d4020ed3c1ef8392fd72e622839cfd5b74b Mon Sep 17 00:00:00 2001 From: Gary Jones Date: Wed, 6 Sep 2023 12:07:24 +0100 Subject: [PATCH 02/24] Contributing: add notes for how to checkout the files Quick note on how best to install the dependencies so that `commit check` is ready to go. --- .github/CONTRIBUTING.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 6bc4d107..766f7149 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -29,6 +29,34 @@ Sniff name starts with | Report to ---- +## Getting the source files + +```sh +git clone git@github.com:Automattic/VIP-Coding-Standards.git vipcs +``` + +...or: + +```sh +gh repo clone Automattic/VIP-Coding-Standards vipcs +``` + +Now `cd vipcs` and run: + +```sh +composer install --ignore-platform-req=php+ +``` + +The platform requirements for higher versions of PHP are ignored so that the correct version of PHPUnit (7.x needed by PHPCS) is installed. + +You can now run: + +``` +composer check +``` + +... and all checks should pass. + ## tl;dr Composer Scripts This package contains Composer scripts to quickly run the developer checks which are described (with setups) further below. From 263d13c7a24f140f4b9f1fb8273613dc87924695 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 12 Sep 2023 18:05:24 +0200 Subject: [PATCH 03/24] Add dependabot configuration file This commit adds an initial Dependabot configuration to: * Submit pull requests for security updates and version updates for GH Action runner dependencies. At a later point in time, we could consider enabling it for Composer dependencies as well. The configuration has been set up to: * Run weekly (for now). * The commit messages for PRs submitted by Dependabot will be prefixed according the unofficial conventions used in this repo up to now. * The PRs will automatically be labelled with an appropriate label as already in use in this repo. Refs: * https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file * https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#versioning-strategy --- .github/dependabot.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..3c13fc31 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,16 @@ +# Dependabot configuration. +# +# Please see the documentation for all configuration options: +# https://docs.github.com/github/administering-a-repository/configuration-options-for-dependency-updates + +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + time: "09:00" + commit-message: + prefix: "GH Actions:" + labels: + - "Type: Maintenance" From 910d24a4d5e5599fdb17705aaa97215c339023ad Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 14 Sep 2023 06:55:10 +0000 Subject: [PATCH 04/24] GH Actions: Bump actions/checkout from 3 to 4 Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/basics.yml | 4 ++-- .github/workflows/quicktest.yml | 2 +- .github/workflows/test.yml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index 7e616537..35e57232 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -24,7 +24,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install PHP uses: shivammathur/setup-php@v2 @@ -84,7 +84,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install PHP uses: shivammathur/setup-php@v2 diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml index becaac4c..96131998 100644 --- a/.github/workflows/quicktest.yml +++ b/.github/workflows/quicktest.yml @@ -40,7 +40,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up PHP uses: shivammathur/setup-php@v2 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cc5ed0a6..72dcada6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -35,7 +35,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install PHP uses: shivammathur/setup-php@v2 @@ -92,7 +92,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 # With stable PHPCS dependencies, 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. From ff14488338583cdceff9e08093ebd3c4195f60fa Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 19 Sep 2023 06:15:03 +0200 Subject: [PATCH 05/24] Classes/DeclarationCompatibility: sync with WP Core While looking at this sniff for something unrelated, I started wondering if the signature definitions were still in line with WP Core. Turned out they were not, though with the current checks being done in the sniff, this wasn't necessarily problematic (though it should have been, but that's for another PR). Also see the [additional notes I've added to the review ticket](https://github.com/Automattic/VIP-Coding-Standards/issues/507#issuecomment-1724805522). Refs: * https://developer.wordpress.org/reference/classes/wp_widget/wp_widget/ * https://developer.wordpress.org/reference/classes/walker/start_el/ * https://developer.wordpress.org/reference/classes/walker/end_el/ * https://developer.wordpress.org/reference/classes/walker/unset_children/ --- .../Sniffs/Classes/DeclarationCompatibilitySniff.php | 10 +++++----- .../Classes/DeclarationCompatibilityUnitTest.inc | 12 +++++++++--- .../Classes/DeclarationCompatibilityUnitTest.php | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php b/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php index 712cacd0..a71b4907 100644 --- a/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php +++ b/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php @@ -40,7 +40,7 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'widget_options' => [ 'default' => 'array()', ], - 'constol_options' => [ + 'control_options' => [ 'default' => 'array()', ], ], @@ -103,7 +103,7 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'output' => [ 'pass_by_reference' => true, ], - 'object', + 'data_object', 'depth' => [ 'default' => '0', ], @@ -118,7 +118,7 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'output' => [ 'pass_by_reference' => true, ], - 'object', + 'data_object', 'depth' => [ 'default' => '0', ], @@ -158,7 +158,7 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'elements', ], 'unset_children' => [ - 'el', + 'element', 'children_elements' => [ 'pass_by_reference' => true, ], @@ -215,7 +215,7 @@ protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currSco return; } - // Meed to define the originalParentClassName since we might override the parentClassName due to signature notations grouping. + // Need to define the originalParentClassName since we might override the parentClassName due to signature notations grouping. $originalParentClassName = $parentClassName; if ( array_key_exists( $parentClassName, $this->checkClasses ) === false ) { diff --git a/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.inc b/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.inc index a7ecaacf..740a7883 100644 --- a/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.inc @@ -125,15 +125,21 @@ class MyWalker extends Walker { function display_element( $element, &$children_elements, $max_depth, $depth, $args, &$output ) { } // Ok. - function unset_children( $el, $children_elements ) { + function unset_children( $element, $children_elements ) { } // Bad. $children_elements should be passed by reference - + function walk( $elements, $max_depth, ...$args ) { } // Ok. - + function paged_walk( $elements, $max_depth, $page_num, $per_page ) { } // Bad. Missing $args. function paged_walk( $elements, $max_depth, $page_num, $per_page, $args ) { } // Bad. $args is not variadic. + + function start_el( $output, $data_object, $depth, $args = array(), $current_object_id = 0 ) { + } // Bad. $output should be passed by reference, $depth should have a default value. + + function end_el( &$output, $data_object, $depth = 0, $args = array() ) { + } // Ok. } diff --git a/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.php b/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.php index 1a6b89a7..817bc324 100644 --- a/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.php +++ b/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.php @@ -47,6 +47,7 @@ public function getErrorList() { 128 => 1, 134 => 1, 137 => 1, + 140 => 1, ]; } From 6c9995e56a262816699de14e626bf291db4e8ab8 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 14 Nov 2023 03:09:55 +0100 Subject: [PATCH 06/24] GH Actions: minor tweaks * Update a URL which is no longer valid. * Ensure all steps have a name. --- .github/workflows/basics.yml | 7 ++++--- .github/workflows/quicktest.yml | 2 +- .github/workflows/test.yml | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index 35e57232..50c1d832 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -40,7 +40,8 @@ jobs: # Show XML violations inline in the file diff. # @link https://github.com/marketplace/actions/xmllint-problem-matcher - - uses: korelstar/xmllint-problem-matcher@v1 + - name: Enable showing XML issues inline + uses: korelstar/xmllint-problem-matcher@v1 # Validate the composer.json file. # @link https://getcomposer.org/doc/03-cli.md#validate @@ -52,7 +53,7 @@ jobs: run: composer require --no-update --no-scripts squizlabs/php_codesniffer:"dev-master" --no-interaction # Install dependencies and handle caching in one go. - # @link https://github.com/marketplace/actions/install-composer-dependencies + # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies uses: "ramsey/composer-install@v2" with: @@ -95,7 +96,7 @@ jobs: # Install dependencies and handle caching in one go. # Dependencies need to be installed to make sure the PHPCS and PHPUnit classes are recognized. - # @link https://github.com/marketplace/actions/install-composer-dependencies + # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies uses: "ramsey/composer-install@v2" with: diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml index 96131998..fb001fa6 100644 --- a/.github/workflows/quicktest.yml +++ b/.github/workflows/quicktest.yml @@ -52,7 +52,7 @@ jobs: coverage: none # Install dependencies and handle caching in one go. - # @link https://github.com/marketplace/actions/install-composer-dependencies + # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies - normal if: ${{ startsWith( matrix.php, '8' ) == false && matrix.php != 'latest' }} uses: "ramsey/composer-install@v2" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 72dcada6..09e0c856 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -123,7 +123,7 @@ jobs: wp-coding-standards/wpcs:"dev-develop" # Install dependencies and handle caching in one go. - # @link https://github.com/marketplace/actions/install-composer-dependencies + # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies - normal if: ${{ startsWith( matrix.php, '8' ) == false }} uses: "ramsey/composer-install@v2" From 47909901d72864b1f73d17954fa8d65f414c7ac3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 23 Nov 2023 08:06:22 +0100 Subject: [PATCH 07/24] GH Actions: update for the release of PHP 8.3 ... which is expected later today. * Builds against PHP 8.3 are no longer allowed to fail. * Add _allowed to fail_ build against PHP 8.4. --- .github/workflows/test.yml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 09e0c856..47f1d4f8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,10 +28,10 @@ jobs: strategy: matrix: - php: ['5.4', 'latest', '8.3'] + php: ['5.4', 'latest', '8.4'] name: "Lint: PHP ${{ matrix.php }}" - continue-on-error: ${{ matrix.php == '8.3' }} + continue-on-error: ${{ matrix.php == '8.4' }} steps: - name: Checkout code @@ -67,8 +67,9 @@ jobs: # - PHPCS will run without errors on PHP 5.4 - 7.4 on any supported version. # - PHP 8.0 needs PHPCS 3.5.7+ to run without errors, and we require a higher minimum version. # - PHP 8.1 needs PHPCS 3.6.1+ to run without errors, but works best with 3.7.1+, and we require at least this minimum version. + # - PHP 8.2 and 8.3 need PHPCS 3.8.0+ to run without errors (though the errors don't affect the tests). matrix: - php: ['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] + php: ['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3'] dependencies: ['lowest', 'stable'] include: @@ -79,16 +80,16 @@ jobs: dependencies: 'dev' - php: '7.4' dependencies: 'dev' - - php: '8.2' + - php: '8.3' dependencies: 'dev' # Test against upcoming PHP version. - - php: '8.3' + - php: '8.4' dependencies: 'dev' name: "Test: PHP ${{ matrix.php }} - PHPCS ${{ matrix.dependencies }}" - continue-on-error: ${{ matrix.php == '8.3' }} + continue-on-error: ${{ matrix.php == '8.4' }} steps: - name: Checkout code From d11b30453a6e999da4fd09b2b587f465ed2a8c18 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 7 Nov 2023 01:17:18 +0100 Subject: [PATCH 08/24] Switch to PHPCSStandards/PHP_CodeSniffer The Squizlabs repo has been abandoned. The project continues in a fork in the PHPCSStandards organisation. Ref: * squizlabs/PHP_CodeSniffer 3932 --- .github/CONTRIBUTING.md | 6 +++--- .phpcs.xml.dist | 2 +- LICENSE.md | 2 +- README.md | 4 ++-- WordPress-VIP-Go/ruleset.xml | 2 +- WordPressVIPMinimum/ruleset.xml | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 766f7149..5d6ff2a5 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -19,9 +19,9 @@ To determine where best to report the bug, use the first part of the sniff name: Sniff name starts with | Report to --- | --- -`Generic` | [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer/issues/) -`PSR2` | [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer/issues/) -`Squiz` | [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer/issues/) +`Generic` | [PHP_CodeSniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/) +`PSR2` | [PHP_CodeSniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/) +`Squiz` | [PHP_CodeSniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/) `Universal` | [PHPCSExtra](https://github.com/PHPCSStandards/PHPCSExtra/issues/) `VariableAnalysis` | [VariableAnalysis](https://github.com/sirbrillig/phpcs-variable-analysis/issues/) `WordPress` | [WordPressCS](https://github.com/WordPress/WordPress-Coding-Standards/issues/) diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index a0049b7b..098e5acb 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -1,5 +1,5 @@ - + The custom ruleset for the VIP Coding Standards itself. . diff --git a/LICENSE.md b/LICENSE.md index b6197d02..5517f00d 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -64,4 +64,4 @@ Included Files This project includes: - [WordPress-Coding-Standards](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards), which is Copyright © 2009 John Godley and contributors. Released under the MIT license https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/LICENSE -- [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer), Copyright © 2012, Squiz Pty Ltd (ABN 77 084 670 600). Released under the following license: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt +- [PHP_CodeSniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer), Copyright © 2012, Squiz Pty Ltd (ABN 77 084 670 600). Released under the following license: https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt diff --git a/README.md b/README.md index 1006b175..8e839b00 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # VIP Coding Standards -This project contains [PHP_CodeSniffer (PHPCS) sniffs and rulesets](https://github.com/squizlabs/PHP_CodeSniffer) to validate code developed for [WordPress VIP](https://wpvip.com/). +This project contains [PHP_CodeSniffer (PHPCS) sniffs and rulesets](https://github.com/PHPCSStandards/PHP_CodeSniffer) to validate code developed for [WordPress VIP](https://wpvip.com/). This project contains two rulesets: @@ -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.7.2+](https://github.com/squizlabs/PHP_CodeSniffer/releases) +* [PHPCS 3.7.2+](https://github.com/PHPCSStandards/PHP_CodeSniffer/releases) * [PHPCSUtils 1.0.8+](https://github.com/PHPCSStandards/PHPCSUtils) * [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) diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 190bebc8..f8952dcb 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -1,5 +1,5 @@ - + WordPress VIP Go Coding Standards diff --git a/WordPressVIPMinimum/ruleset.xml b/WordPressVIPMinimum/ruleset.xml index 13670326..6a3f10c1 100644 --- a/WordPressVIPMinimum/ruleset.xml +++ b/WordPressVIPMinimum/ruleset.xml @@ -1,5 +1,5 @@ - + WordPress VIP Minimum Coding Standards