Merge pull request #368 from Yoast/JRF/add-phpstan

Add PHPStan to QA checks

.gitattributes

@@ -5,13 +5,14 @@
 # https://www.reddit.com/r/PHP/comments/2jzp6k/i_dont_need_your_tests_in_my_production
 # https://blog.madewithlove.be/post/gitattributes/
 #
-/.cache          export-ignore
-/.github         export-ignore
-/Yoast/Tests     export-ignore
-.gitattributes   export-ignore
-.gitignore       export-ignore
-.phpcs.xml.dist  export-ignore
-phpunit.xml.dist export-ignore
+/.cache           export-ignore
+/.github          export-ignore
+/Yoast/Tests      export-ignore
+.gitattributes    export-ignore
+.gitignore        export-ignore
+.phpcs.xml.dist   export-ignore
+phpstan.neon.dist export-ignore
+phpunit.xml.dist  export-ignore
 
 #
 # Auto detect text files and perform LF normalization

.github/workflows/basics.yml

@@ -100,3 +100,31 @@ jobs:
       # Check that the sniffs available are feature complete.
       - name: Check sniff feature completeness
         run: composer check-complete
+
+  phpstan:
+    name: "PHPStan"
+
+    runs-on: "ubuntu-latest"
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v3
+
+      - name: Install PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 'latest'
+          coverage: none
+          tools: phpstan
+
+      # 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-php-dependencies-with-composer
+      - name: Install Composer dependencies
+        uses: "ramsey/composer-install@v2"
+        with:
+          # Bust the cache at least once a month - output format: YYYY-MM.
+          custom-cache-suffix: $(date -u "+%Y-%m")
+
+      - name: Run PHPStan
+        run: phpstan analyse

.gitignore

@@ -6,3 +6,4 @@ phpcs.xml
 phpunit.xml
 .phpunit.result.cache
 build/
+phpstan.neon

phpstan.neon.dist

@@ -0,0 +1,102 @@
+parameters:
+	phpVersion: 70200
+	level: 9
+	paths:
+		- Yoast
+	bootstrapFiles:
+		- phpunit-bootstrap.php
+	scanDirectories:
+		- vendor/wp-coding-standards/wpcs/WordPress
+#	treatPhpDocTypesAsCertain: false
+#	reportUnmatchedIgnoredErrors: false
+	dynamicConstantNames:
+		- YOASTCS_ABOVE_THRESHOLD
+
+	ignoreErrors:
+		# Level 3
+		-
+			# This is an issue with the PHPCS docs. Can't be helped.
+			message: '`^Property PHP_CodeSniffer\\Config::\$basepath \(string\) does not accept null\.$`'
+			path: Yoast/Tests/Utils/PSR4PathsTraitTest.php
+			count: 1
+		-
+			# This is a test specifically checking the handling when an invalid value is passed, so this is okay.
+			message: '`^Property YoastCS\\Yoast\\Tests\\Utils\\PSR4PathsTraitTest::\$psr4_paths \(array\<string, string\>\) does not accept array\<int, string\>\.$`'
+			path: Yoast/Tests/Utils/PSR4PathsTraitTest.php
+			count: 1
+
+		# Level 4
+		-
+			# Bug in PHPStan: PHPStan doesn't seem to like uninitialized properties...
+			message: '`^Property \S+Sniff::\$target_paths \(array<string, string>\) in isset\(\) is not nullable\.$`'
+			path: Yoast/Sniffs/Files/TestDoublesSniff.php
+			count: 1
+		-
+			# Defensive coding in the PSR4PathsTrait as the property value is user provided via the ruleset. This is okay.
+			message: '`^Strict comparison using === between true and false will always evaluate to false\.$`'
+			paths:
+				- Yoast/Sniffs/Files/FileNameSniff.php
+				- Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php
+				- Yoast/Tests/Utils/PSR4PathsTraitTest.php
+
+		# Level 5
+		# We're not using strict types, so this will be juggled without any issues.
+		- '#^Parameter \#3 \$value of method \S+File::recordMetric\(\) expects string, \(?(float|int|bool)(<[^>]+>)?(\|(float|int|bool)(<[^>]+>)?)*\)? given\.$#'
+
+		# Level 7
+		-
+			# False positive: the explode will never return `false` as it is never given an empty string separator.
+			message: '`^Parameter #2 \$array of function array_map expects array, array<int, string>\|false given\.$`'
+			path: Yoast/Sniffs/Commenting/CoversTagSniff.php
+			count: 1
+		-
+			# False positive: the preg_replace will never return `null` as the regex is valid.
+			message: '`^Parameter #1 \$str of function strtolower expects string, string\|null given\.$`'
+			path: Yoast/Sniffs/Files/FileNameSniff.php
+			count: 1
+		-
+			# False positive: the preg_replace will never return `null` as the regex is valid.
+			message: '`^Parameter #2 \$str of function explode expects string, string\|null given\.$`'
+			path: Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php
+			count: 1
+		-
+			# False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array.
+			message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`'
+			count: 2
+			path: Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php
+		-
+			# False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array.
+			message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`'
+			count: 2
+			path: Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php
+		-
+			# False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array.
+			message: '`^Parameter #1 \$stackPtr of method PHP_CodeSniffer\\Fixer::replaceToken\(\) expects int, int\|string given\.$`'
+			path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php
+			count: 1
+		-
+			# This is a test specifically checking type handling, so this is okay.
+			message: '`^Parameter #1 \$path of static method YoastCS\\Yoast\\Utils\\PathHelper::trailingslashit\(\) expects string, bool\|string given\.$`'
+			path: Yoast/Tests/Utils/PathHelperTest.php
+			count: 1
+
+		# Not a real issue (x 4), PHPstan just doesn't know the format of the array well enough.
+		-
+			message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`'
+			count: 2
+			path: Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php
+
+		-
+			message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`'
+			count: 2
+			path: Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php
+
+		-
+			message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`'
+			count: 1
+			path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php
+
+		-
+			message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`'
+			count: 1
+			path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php