Skip to content

fix(phpcs): exclude vendor/ from ruleset to silence third-party errors#271

Open
faisalahammad wants to merge 1 commit into
10up:trunkfrom
faisalahammad:fix/251-phpcs-exclude-vendor
Open

fix(phpcs): exclude vendor/ from ruleset to silence third-party errors#271
faisalahammad wants to merge 1 commit into
10up:trunkfrom
faisalahammad:fix/251-phpcs-exclude-vendor

Conversation

@faisalahammad

Copy link
Copy Markdown

Summary

Fixes #251.

Running phpcs in the repo currently reports errors in third-party dependencies (nikic/php-parser, hamcrest/hamcrest-php) on PHP 8.2+. Those libraries are transitive dependencies pinned by other dev packages, and the project does not own their source.

This PR hardens the phpcs.xml ruleset so vendor is never scanned regardless of how phpcs is invoked, and switches testVersion from exact 7.4 to the open range 7.4- so the standard targets the minimum supported PHP and above.

Changes

phpcs.xml

Before:

<ruleset name="Custom ruleset">
    <file>./php</file>
    <file>./tests</file>
    <arg value="sp"/>
    <rule ref="PHPCompatibility"/>
    <config name="testVersion" value="7.4"/>
</ruleset>

After:

<ruleset name="Custom ruleset">
    <description>WP_Mock PHPCS ruleset. Scans project source only; vendor/ is excluded so third-party code is never reported.</description>
    <file>./php</file>
    <file>./tests</file>
    <exclude-pattern>*/vendor/*</exclude-pattern>
    <arg value="sp"/>
    <rule ref="PHPCompatibility"/>
    <config name="testVersion" value="7.4-"/>
</ruleset>

Why: The original ruleset only restricted scan paths. When phpcs is invoked with a path that includes vendor (or with the project root as a default), third-party code gets scanned and reported. Excluding vendor in the ruleset is the standard PHPCS idiom for "scan only project source". The open-range testVersion matches PHPCompatibility conventions (per their README example) and ensures the standard targets the minimum supported PHP and above.

Testing

Test 1: Default scan (the CI command)

  1. composer install
  2. vendor/bin/phpcs

Result: 43 files scanned in ./php and ./tests, all clean. No vendor noise.

Test 2: Scan project root with the project ruleset

  1. vendor/bin/phpcs .

Result: 46 files scanned (43 above plus 3 config files in .), all clean. No vendor noise.

Test 3: Reproduce the original report

  1. vendor/bin/phpcs --runtime-set testVersion 8.2- .

Result: only project files are scanned. No errors from vendor/nikic/php-parser or vendor/hamcrest/hamcrest-php.

Test 4: Full test suite

  1. composer test:phpunit

Result: 160 tests, 1 skipped (pre-existing final-class limitation on PHP 7.4, unrelated to this change).

Issue 10up#251 reports that running phpcs on the repo flags errors in
vendor/nikic/php-parser and vendor/hamcrest/hamcrest-php on PHP 8.2+.
Those libraries are transitive dependencies pinned by other dev
packages; the project does not own their source.

Harden the phpcs.xml ruleset so vendor is never scanned regardless
of how phpcs is invoked. Also switch testVersion from the exact
'7.4' to the open range '7.4-' so the standard targets the minimum
supported PHP and above, matching PHPCompatibility conventions.

Fixes 10up#251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PHPCS reporting error on dependent library nikic/php-parser, hamcrest/hamcrest-php

1 participant