Merge pull request #151 from Yoast/JRF/CS/update-phpcs-ruleset-yoastc…

…s-3.0.0 Composer/PHPCS: update to YoastCS 3.0.0
Yoast · Dec 14, 2023 · f07cf7b · f07cf7b
2 parents 2d0d527 + d42ef7f
commit f07cf7b
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 12 deletions.
diff --git a/.github/workflows/cs.yml b/.github/workflows/cs.yml
@@ -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
 

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -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
 

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -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

diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist
@@ -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
     #############################################################################
     -->
 
@@ -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
@@ -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>
 
 
 	<!--
@@ -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>
@@ -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+. -->
@@ -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>

diff --git a/composer.json b/composer.json
@@ -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,

diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -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.
@@ -32,6 +33,7 @@ static function ( $fqClassName ) {
 			return false;
 		}
 	);
+	// phpcs:enable
 }
 elseif ( \file_exists( \dirname( __DIR__ ) . '/vendor/autoload.php' ) ) {
 	/*