Skip to content

Commit

Permalink
Composer/PHPCS: update to YoastCS 3.0.0
Browse files Browse the repository at this point in the history
YoastCS 3.0.0 has been released and is based on WordPressCS 3.0.0.

This commit makes the necessary updates for that:
* Composer: update the requirements.
* PHPCS ruleset:
    - Exclude new WP specific rules which don't apply to this package.
    - Exclude code modernization sniffs which can't be applied to this package yet.
    - Enforce strict PSR-4 for both src and tests.
    - Add a few selective exclusions for specific situations.
* GHA CS workflow: run the CS check on the latest PHP version.
    No need to run on PHP 7.4 any more as the deprecations previously encountered were all fixed.
* Add one selective ignore annotation for a long closure (autoloader in the test bootstrap).

While YoastCS 3.0.0 contains lots of goodies, it also has a downside: a minimum PHP requirement of PHP 7.2, which conflicts with the minimum supported PHP version of this package.

This causes two issues:
1. A plain `composer install` will no longer work on PHP < 7.2.
    This means the YoastCS package will need to be removed for the CI linting and test workfows.
2. As the (Parallel) linting packages are "inherited" from YoastCS, removing YoastCS would break the linting command in CI, so we need to `require-dev` the Parallel Lint packages in PHPUnit Polyfills itself to allow the workflow to continue to work.

With those two work-arounds in place, everything should work again.

Refs:
* https://github.com/Yoast/yoastcs/releases/tag/3.0.0
* https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.0.0
  • Loading branch information
jrfnl committed Dec 14, 2023
1 parent 2d0d527 commit d42ef7f
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
- name: Install PHP
uses: shivammathur/setup-php@v2
with:
php-version: '7.4'
php-version: 'latest'
coverage: none
tools: cs2pr

Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ jobs:
ini-values: zend.assertions=1, error_reporting=-1, display_errors=On
coverage: none

# YoastCS 3.0 has a PHP 7.2 minimum which conflicts with the requirements of this package.
- name: 'Composer: remove YoastCS'
run: composer remove --dev yoast/yoastcs --no-update --no-interaction

- name: 'Composer: remove PHPUnit (not needed for lint)'
run: composer remove phpunit/phpunit --no-update --no-interaction

Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ jobs:
ini-values: zend.assertions=1, error_reporting=-1, display_errors=On
coverage: ${{ matrix.coverage == true && 'xdebug' || 'none' }}

# YoastCS 3.0 has a PHP 7.2 minimum which conflicts with the requirements of this package.
- name: 'Composer: remove YoastCS'
run: composer remove --dev yoast/yoastcs --no-update --no-interaction

- name: 'Composer: set PHPUnit version for tests'
if: ${{ matrix.phpunit != 'auto' }}
run: composer require --no-update phpunit/phpunit:"${{ matrix.phpunit }}" --no-interaction
Expand Down
56 changes: 46 additions & 10 deletions .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<!--
#############################################################################
COMMAND LINE ARGUMENTS
https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-ruleset.xml
https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Annotated-ruleset.xml
#############################################################################
-->

Expand Down Expand Up @@ -49,8 +49,18 @@
<exclude name="WordPress.PHP.DiscouragedPHPFunctions.system_calls_popen"/>
<exclude name="WordPress.Security"/>
<exclude name="WordPress.WP"/>
<exclude name="Yoast.Yoast.AlternativeFunctions"/>
<exclude name="Yoast.Yoast.JsonEncodeAlternative"/>
<exclude name="Yoast.NamingConventions.ObjectNameDepth.MaxExceeded"/>
<exclude name="WordPressVIPMinimum"/>

<!-- Exclude select "modern PHP" sniffs, which conflict with the minimum supported PHP version of this package. -->
<exclude name="SlevomatCodingStandard.Classes.ModernClassNameReference"/><!-- PHP 5.5+ -->
<exclude name="Modernize.FunctionCalls.Dirname.Nested"/><!-- PHP 7.0+. -->
<exclude name="PSR12.Properties.ConstantVisibility"/><!-- PHP 7.1+. -->
<exclude name="SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue"/><!-- PHP 7.1+. -->

<!-- As this repo is about providing assertions, "mixed" is a perfectly valid type. -->
<exclude name="SlevomatCodingStandard.TypeHints.DisallowMixedTypeHint"/>
</rule>

<!-- While PHPCompatibility is already included in the Yoast ruleset, it uses
Expand All @@ -66,7 +76,10 @@
</rule>

<!-- Enforce PSR1 compatible namespaces. -->
<rule ref="PSR1.Classes.ClassDeclaration"/>
<rule ref="PSR1.Classes.ClassDeclaration">
<!-- YoastCS only applies this rule to test files. Overrule it to apply to all files. -->
<include-pattern>*\.php</include-pattern>
</rule>


<!--
Expand All @@ -77,11 +90,9 @@

<rule ref="Yoast.NamingConventions.NamespaceName">
<properties>
<property name="prefixes" type="array">
<element value="Yoast\PHPUnitPolyfills"/>
</property>
<property name="src_directory" type="array">
<element value="src"/>
<property name="psr4_paths" type="array">
<element key="Yoast\PHPUnitPolyfills\\" value="src/"/>
<element key="Yoast\PHPUnitPolyfills\Tests\\" value="tests/"/>
</property>
</properties>
</rule>
Expand All @@ -101,16 +112,24 @@
<exclude-pattern>/src/Exceptions/*Error\.php$</exclude-pattern>
</rule>

<!-- For named parameter support, the parameters in the polyfilled assertions *must*
mirror the parameter name as used in PHPUnit itself.
These cannot be changed until PHPUnit itself changes the names. -->
<rule ref="Universal.NamingConventions.NoReservedKeywordParameterNames.objectFound">
<exclude-pattern>/src/Polyfills/AssertObjectProperty\.php$</exclude-pattern>
</rule>
<rule ref="Universal.NamingConventions.NoReservedKeywordParameterNames.stringFound">
<exclude-pattern>/src/Polyfills/AssertionRenames\.php$</exclude-pattern>
</rule>

<!-- Deliberately empty Catch statements. -->
<rule ref="Generic.CodeAnalysis.EmptyStatement.DetectedCatch">
<exclude-pattern>/src/Helpers/ResourceHelper\.php$</exclude-pattern>
<exclude-pattern>/src/Polyfills/AssertClosedResource*\.php$</exclude-pattern>
</rule>

<!-- Targetted error silencing. This is okay. -->
<rule ref="WordPress.PHP.NoSilencedErrors">
<exclude-pattern>/src/Helpers/ResourceHelper\.php$</exclude-pattern>
<exclude-pattern>/src/Polyfills/AssertClosedResource*\.php$</exclude-pattern>
</rule>

<!-- The TestCase for PHPUnit 8+ will only ever be loaded on PHP 7.2+. -->
Expand All @@ -136,11 +155,28 @@
<exclude-pattern>/src/Polyfills/ExpectException\.php$</exclude-pattern>
</rule>

<!-- TEST CODE -->

<!-- Final classes is irrelevant for test fixtures. -->
<rule ref="Universal.Classes.RequireFinalClass">
<exclude-pattern>/tests/*/Fixtures/*\.php$</exclude-pattern>
</rule>

<!-- The use of `array` annotations/types is intentional and part of the actual test case. -->
<rule ref="SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingTraversableTypeHintSpecification">
<exclude-pattern>/tests/Polyfills/Fixtures/ValueObject*\.php$</exclude-pattern>
</rule>

<!-- Make some allowances for test files. -->
<rule ref="WordPress.PHP.DevelopmentFunctions">
<exclude-pattern>/tests/*\.php$</exclude-pattern>
</rule>

<!-- The use of `static` in the test cases is on purpose to test support. -->
<rule ref="Universal.CodeAnalysis.StaticInFinalClass">
<exclude-pattern>/tests/*</exclude-pattern>
</rule>

<!-- Covers annotations are in the test classes, not the trait. -->
<rule ref="Yoast.Commenting.TestsHaveCoversTag.Missing">
<exclude-pattern>/tests/TestCases/TestCaseTestTrait\.php$</exclude-pattern>
Expand Down
4 changes: 3 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
"phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.0"
},
"require-dev": {
"yoast/yoastcs": "^2.3.0"
"php-parallel-lint/php-console-highlighter": "^1.0.0",
"php-parallel-lint/php-parallel-lint": "^1.3.2",
"yoast/yoastcs": "^3.0.0"
},
"minimum-stability": "dev",
"prefer-stable": true,
Expand Down
2 changes: 2 additions & 0 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
if ( \defined( '__PHPUNIT_PHAR__' ) ) {
require_once \dirname( __DIR__ ) . '/phpunitpolyfills-autoload.php';

// phpcs:disable Universal.FunctionDeclarations.NoLongClosures.ExceedsMaximum
\spl_autoload_register(
/**
* Custom PSR-4 based autoloader for test helper files.
Expand Down Expand Up @@ -32,6 +33,7 @@ static function ( $fqClassName ) {
return false;
}
);
// phpcs:enable
}
elseif ( \file_exists( \dirname( __DIR__ ) . '/vendor/autoload.php' ) ) {
/*
Expand Down

0 comments on commit d42ef7f

Please sign in to comment.