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:
    - Update a few sniff/property names for new names in WordPressCS 3.0.
    - Exclude code modernization sniffs which can't be applied to this package yet.
    - Enforce strict PSR-4 for the tests.
    - Remove a few exclusions which are no longer needed.
    - 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.
    - Change the CS check from a "normal" PHPCS run to a run using the Thresholds report and only reporting on issues found in files changed in the branch.
        There are currently 8 (documentation) issues which still need to be fixed, so hopefully, the threshold can go back to 0 soon.
* Add one selective ignore annotation for a class name containing an acronym.
* Make one more minor CS fix (`@return void`).

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 test workflow.
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 WHIP 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 15, 2023
1 parent e4bae6c commit 1833587
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 34 deletions.
32 changes: 27 additions & 5 deletions .github/workflows/cs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,26 @@ jobs:
- name: Checkout code
uses: actions/checkout@v4

- name: Determine the base branch for the file diff
id: base_branch
env:
BASE_REF: ${{ github.base_ref }}
run: |
if [ "${{ github.event_name }}" == "pull_request" ]; then
echo "NAME=$BASE_REF" >> $GITHUB_OUTPUT
echo "REF=origin/$BASE_REF" >> $GITHUB_OUTPUT
else
echo 'NAME=main' >> $GITHUB_OUTPUT
echo "REF=origin/main" >> $GITHUB_OUTPUT
fi
- name: Fetch base branch
run: git fetch --no-tags --depth=1 origin ${{ steps.base_branch.outputs.NAME }}

- name: Install PHP
uses: shivammathur/setup-php@v2
with:
php-version: '7.4'
php-version: 'latest'
coverage: none
tools: cs2pr

Expand All @@ -60,12 +76,18 @@ jobs:
# Bust the cache at least once a month - output format: YYYY-MM.
custom-cache-suffix: $(date -u "+%Y-%m")

# Check the codestyle of the files.
# The results of the CS check will be shown inline in the PR via the CS2PR tool.
# Check the codestyle of the files against a threshold of expected errors and warnings.
- name: Check PHP code style against the thresholds
run: composer check-cs-thresholds

# Check the codestyle only of the files which were changed in the current branch.
# This step will only be executed if the threshold check exited with a failure status.
# The results of this CS check will be shown inline in the PR via the CS2PR tool.
# @link https://github.com/staabm/annotate-pull-request-from-checkstyle/
- name: Check PHP code style
- name: Check PHP code style for the changes made in the branch only
if: ${{ failure() }}
id: phpcs
run: composer check-cs-warnings -- --no-cache --report-full --report-checkstyle=./phpcs-report.xml
run: composer check-branch-cs -- ${{ steps.base_branch.outputs.REF }}

- name: Show PHPCS results in PR
if: ${{ always() && steps.phpcs.outcome == 'failure' }}
Expand Down
9 changes: 3 additions & 6 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,9 @@ jobs:
coverage: ${{ matrix.coverage == true && 'xdebug' || 'none' }}
tools: cs2pr

# YoastCS has a minimum PHP requirement of 5.4, so remove it and hard require Parallel Lint.
- name: Adjust Composer dependencies (PHP 5.3)
if: matrix.php_version == '5.3'
run: |
composer remove --dev --no-update --no-scripts yoast/yoastcs --no-interaction
composer require --dev --no-update --no-scripts php-parallel-lint/php-parallel-lint --no-interaction
# 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

# Install dependencies and handle caching in one go.
# @link https://github.com/marketplace/actions/install-php-dependencies-with-composer
Expand Down
51 changes: 30 additions & 21 deletions .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,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 @@ -39,7 +39,7 @@
<rule ref="Yoast">
<properties>
<!-- Set the minimum supported WP version for all sniff which use it in one go. -->
<property name="minimum_supported_version" value="3.0"/>
<property name="minimum_wp_version" value="3.0"/>

<!-- Provide the plugin specific prefixes for all naming related sniffs. -->
<property name="prefixes" type="array">
Expand All @@ -52,8 +52,12 @@
<exclude name="WordPress.NamingConventions.ValidVariableName"/>
<exclude name="WordPress.NamingConventions.ValidFunctionName"/>

<!-- Historically, this library uses camelCaps file names. -->
<exclude name="Yoast.Files.FileName"/>
<!-- Exclude select "modern PHP" sniffs, which conflict with the minimum supported PHP version of this package. -->
<exclude name="SlevomatCodingStandard.Functions.StaticClosure"/><!-- PHP 5.4+ -->
<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+. -->
</rule>

<!-- Check that variable names are in camelCaps. -->
Expand Down Expand Up @@ -83,19 +87,23 @@
#############################################################################
-->

<rule ref="Yoast.NamingConventions.NamespaceName">
<rule ref="Yoast.Files.FileName">
<properties>
<!-- Treat the "src" directory as the project root for path to namespace translations. -->
<property name="src_directory" type="array">
<element value="src"/>
<property name="psr4_paths" type="array">
<element key="Yoast\WHIP\Tests\\" value="tests/"/>
</property>
</properties>

<include-pattern>/tests/*\.php</include-pattern>
</rule>

<rule ref="Yoast.Files.TestDoubles">
<rule ref="Yoast.NamingConventions.NamespaceName">
<properties>
<property name="doubles_path" type="array">
<element value="/tests/Unit/Doubles"/>
<property name="src_directory" type="array">
<element value="src"/>
</property>
<property name="psr4_paths" type="array">
<element key="Yoast\WHIP\Tests\\" value="tests/"/>
</property>
</properties>
</rule>
Expand All @@ -118,15 +126,24 @@
<exclude-pattern>/src/facades/wordpress\.php$</exclude-pattern>
</rule>

<!-- The variable parameters passed to the WHIP exceptions are mostly used to retrieve
the _type_ of the variable. Additionally, we may not be in a WP context. -->
<rule ref="WordPress.Security.EscapeOutput.ExceptionNotEscaped">
<severity>0</severity>
</rule>

<rule ref="Yoast.Files.FileName.InvalidFunctionsFileName">
<exclude-pattern>/tests/Unit/Doubles/WPCoreFunctionsMock\.php$</exclude-pattern>
</rule>

<!--
#############################################################################
TEMPORARY TWEAK
YoastCS demands short arrays, but the WHIP module still supports PHP 5.2.
YoastCS demands short arrays (PHP 5.4), but the WHIP module still supports PHP 5.3.
#############################################################################
-->

<rule ref="Generic.Arrays.DisallowShortArraySyntax">
<rule ref="Universal.Arrays.DisallowShortArraySyntax">
<severity>5</severity>
</rule>
<rule ref="Generic.Arrays.DisallowLongArraySyntax">
Expand Down Expand Up @@ -154,12 +171,4 @@
<exclude-pattern>/src/Whip_Host\.php$</exclude-pattern>
</rule>

<!-- The below two issues will likely be fixed in YoastCS 3.0, after which these excludes can be removed. -->
<rule ref="Yoast.Commenting.FileComment.Unnecessary">
<exclude-pattern>/tests/Unit/bootstrap\.php$</exclude-pattern>
</rule>
<rule ref="Yoast.NamingConventions.ObjectNameDepth.MaxExceeded">
<exclude-pattern>/tests/Unit/WPMessageDismissListenerTest\.php$</exclude-pattern>
</rule>

</ruleset>
6 changes: 4 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
"php": ">=5.3"
},
"require-dev": {
"php-parallel-lint/php-console-highlighter": "^1.0.0",
"php-parallel-lint/php-parallel-lint": "^1.3.2",
"phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.0",
"roave/security-advisories": "dev-master",
"yoast/yoastcs": "^2.3.0"
"yoast/yoastcs": "^3.0"
},
"minimum-stability": "dev",
"prefer-stable": true,
Expand Down Expand Up @@ -54,7 +56,7 @@
"Yoast\\WHIP\\Config\\Composer\\Actions::check_coding_standards"
],
"check-cs-thresholds": [
"@putenv YOASTCS_THRESHOLD_ERRORS=0",
"@putenv YOASTCS_THRESHOLD_ERRORS=8",
"@putenv YOASTCS_THRESHOLD_WARNINGS=0",
"Yoast\\WHIP\\Config\\Composer\\Actions::check_cs_thresholds"
],
Expand Down
2 changes: 2 additions & 0 deletions config/composer/actions.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class Actions {
* Provides a coding standards option choice.
*
* @param Event $event Composer event.
*
* @return void
*/
public static function check_coding_standards( Event $event ) {
$io = $event->getIO();
Expand Down
2 changes: 2 additions & 0 deletions tests/Unit/WPMessageDismissListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
* Message Dismiss Listener unit tests.
*
* @coversDefaultClass Whip_WPMessageDismissListener
*
* @phpcs:disable Yoast.NamingConventions.ObjectNameDepth.MaxExceeded -- Acronym throws the count off.
*/
final class WPMessageDismissListenerTest extends TestCase {

Expand Down

0 comments on commit 1833587

Please sign in to comment.