Add PHPStan analysis for PHP blocks in feature files

.readme-partials/USING.md

@@ -13,11 +13,13 @@ To make use of the WP-CLI testing framework, you need to complete the following
         "lint": "run-linter-tests",
         "phpcs": "run-phpcs-tests",
         "phpcbf": "run-phpcbf-cleanup",
+        "phpstan": "run-phpstan-tests",
         "phpunit": "run-php-unit-tests",
         "prepare-tests": "install-package-tests",
         "test": [
             "@lint",
             "@phpcs",
+            "@phpstan",
             "@phpunit",
             "@behat"
         ]
@@ -76,7 +78,9 @@ To make use of the WP-CLI testing framework, you need to complete the following
     ```
 
     All other [PHPCS configuration options](https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Annotated-Ruleset) are, of course, available.
-6. Update your composer dependencies and regenerate your autoloader and binary folders:
+6. Optionally add a `phpstan-feature-files.neon.dist` file to the package root to also run PHPStan over the PHP snippets embedded in your feature files. See [Analysing the PHP blocks in feature files](#analysing-the-php-blocks-in-feature-files) below.
+
+7. Update your composer dependencies and regenerate your autoloader and binary folders:
     ```bash
     composer update
     ```
@@ -92,9 +96,75 @@ You can use the following commands to control the tests:
 * `composer lint` - Run only the linting test suite.
 * `composer phpcs` - Run only the code sniffer test suite.
 * `composer phpcbf` - Run only the code sniffer cleanup.
+* `composer phpstan` - Run only the static analysis.
 * `composer phpunit` - Run only the unit test suite.
 * `composer behat` - Run only the functional test suite.
 
+### Analysing the PHP blocks in feature files
+
+Feature files embed PHP snippets in docstrings, which none of the static analysis tools normally
+look at:
+
+```gherkin
+Given a wp-content/mu-plugins/test-harness.php file:
+  """
+  <?php
+  WP_CLI::add_command( 'test-harness', 'Test_Harness' );
+  """
+```
+
+Adding a `phpstan-feature-files.neon.dist` file to the package root makes `composer phpstan` analyse
+those snippets as well. The blocks are extracted into standalone PHP files that are padded so their
+line numbers match the feature file, which is what allows errors to be reported against the feature
+file itself:
+
+```text
+ features/command.feature
+  438    Parameter #1 $message of static method WP_CLI::log() expects string, int<0, max> given.
+         🪪  argument.type
+```
+
+The defaults in `phpstan/feature-files.neon` are applied first, so the file only needs to hold what
+it wants to change. An empty file is enough to run with the defaults, and a level of its own looks
+like this:
+
+```neon
+parameters:
+	level: 4
+```
+
+Do note that snippets in feature files are fixtures, not production code, and that they run inside a
+WordPress installation the analysis knows nothing about. Expect to have to ignore errors that are
+not actually wrong, such as functions a scenario deliberately leaves undefined.
+
+An ignore matches against the extracted file rather than the feature file it came from. Those files
+are named `<feature file>_L<first line>_E<last line>.php`, with the feature file relative to the
+`features` directory, so ignoring an error for a whole feature file takes a pattern:
+
+```neon
+parameters:
+	ignoreErrors:
+		-
+			identifier: function.notFound
+			path: */shutdown-handler.feature_L*.php
+```
+
+Since the blocks are analysed in more than one run (see below), an ignore that no run matches is not
+reported. A pattern that matches nothing at all therefore goes unnoticed, so it is worth checking
+that the error it targets is really gone.
+
+Two kinds of blocks are left out of the analysis, and are listed at the end of the run:
+
+* Blocks that are not standalone PHP, such as snippets holding a placeholder that Behat substitutes
+  (`get_the_title( {POST_ID} )`) or code that is deliberately broken to test error handling. PHPStan
+  stops analysing altogether when a single file fails to parse, so these have to be skipped.
+* Docstrings that neither belong to a step creating a `.php` file nor open with `<?php`, since those
+  are not necessarily PHP at all. The second rule covers PHP files that are not named `*.php`, such
+  as the `.maintenance` file of a WordPress installation.
+
+Blocks that declare the same class or function as another block are analysed separately from each
+other, so that PHPStan does not resolve a name to the wrong block's declaration.
+
 ### Controlling what to test
 
 To send one or more arguments to one of the test tools, prepend the argument(s) with a double dash. As an example, here's how to run the functional tests for a specific feature file only:

README.md

@@ -24,11 +24,13 @@ To make use of the WP-CLI testing framework, you need to complete the following
         "lint": "run-linter-tests",
         "phpcs": "run-phpcs-tests",
         "phpcbf": "run-phpcbf-cleanup",
+        "phpstan": "run-phpstan-tests",
         "phpunit": "run-php-unit-tests",
         "prepare-tests": "install-package-tests",
         "test": [
             "@lint",
             "@phpcs",
+            "@phpstan",
             "@phpunit",
             "@behat"
         ]
@@ -87,7 +89,9 @@ To make use of the WP-CLI testing framework, you need to complete the following
     ```
 
     All other [PHPCS configuration options](https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Annotated-Ruleset) are, of course, available.
-6. Update your composer dependencies and regenerate your autoloader and binary folders:
+6. Optionally add a `phpstan-feature-files.neon.dist` file to the package root to also run PHPStan over the PHP snippets embedded in your feature files. See [Analysing the PHP blocks in feature files](#analysing-the-php-blocks-in-feature-files) below.
+
+7. Update your composer dependencies and regenerate your autoloader and binary folders:
     ```bash
     composer update
     ```
@@ -103,9 +107,75 @@ You can use the following commands to control the tests:
 * `composer lint` - Run only the linting test suite.
 * `composer phpcs` - Run only the code sniffer test suite.
 * `composer phpcbf` - Run only the code sniffer cleanup.
+* `composer phpstan` - Run only the static analysis.
 * `composer phpunit` - Run only the unit test suite.
 * `composer behat` - Run only the functional test suite.
 
+### Analysing the PHP blocks in feature files
+
+Feature files embed PHP snippets in docstrings, which none of the static analysis tools normally
+look at:
+
+```gherkin
+Given a wp-content/mu-plugins/test-harness.php file:
+  """
+  <?php
+  WP_CLI::add_command( 'test-harness', 'Test_Harness' );
+  """
+```
+
+Adding a `phpstan-feature-files.neon.dist` file to the package root makes `composer phpstan` analyse
+those snippets as well. The blocks are extracted into standalone PHP files that are padded so their
+line numbers match the feature file, which is what allows errors to be reported against the feature
+file itself:
+
+```text
+ features/command.feature
+  438    Parameter #1 $message of static method WP_CLI::log() expects string, int<0, max> given.
+         🪪  argument.type
+```
+
+The defaults in `phpstan/feature-files.neon` are applied first, so the file only needs to hold what
+it wants to change. An empty file is enough to run with the defaults, and a level of its own looks
+like this:
+
+```neon
+parameters:
+	level: 4
+```
+
+Do note that snippets in feature files are fixtures, not production code, and that they run inside a
+WordPress installation the analysis knows nothing about. Expect to have to ignore errors that are
+not actually wrong, such as functions a scenario deliberately leaves undefined.
+
+An ignore matches against the extracted file rather than the feature file it came from. Those files
+are named `<feature file>_L<first line>_E<last line>.php`, with the feature file relative to the
+`features` directory, so ignoring an error for a whole feature file takes a pattern:
+
+```neon
+parameters:
+	ignoreErrors:
+		-
+			identifier: function.notFound
+			path: */shutdown-handler.feature_L*.php
+```
+
+Since the blocks are analysed in more than one run (see below), an ignore that no run matches is not
+reported. A pattern that matches nothing at all therefore goes unnoticed, so it is worth checking
+that the error it targets is really gone.
+
+Two kinds of blocks are left out of the analysis, and are listed at the end of the run:
+
+* Blocks that are not standalone PHP, such as snippets holding a placeholder that Behat substitutes
+  (`get_the_title( {POST_ID} )`) or code that is deliberately broken to test error handling. PHPStan
+  stops analysing altogether when a single file fails to parse, so these have to be skipped.
+* Docstrings that neither belong to a step creating a `.php` file nor open with `<?php`, since those
+  are not necessarily PHP at all. The second rule covers PHP files that are not named `*.php`, such
+  as the `.maintenance` file of a WordPress installation.
+
+Blocks that declare the same class or function as another block are analysed separately from each
+other, so that PHPStan does not resolve a name to the wrong block's declaration.
+
 ### Controlling what to test
 
 To send one or more arguments to one of the test tools, prepend the argument(s) with a double dash. As an example, here's how to run the functional tests for a specific feature file only:

bin/run-phpstan-tests

@@ -1,7 +1,96 @@
 #!/bin/sh
 
-# Run the code style check only if a configuration file exists.
+EXIT_CODE=0
+
+# 1. Run the static analysis only if a configuration file exists.
 if [ -f "phpstan.dist.neon" ] || [ -f "phpstan.neon.dist" ] || [ -f "phpstan.neon" ]
 then
-	vendor/bin/phpstan --memory-limit=2048M analyse "$@"
+	vendor/bin/phpstan --memory-limit=2048M analyse "$@" || EXIT_CODE=$?
 fi
+
+# 2. Run PHPStan over the PHP blocks in .feature files if the package opted in.
+# Composer installs this script as a symlink in the vendor binary directory, so
+# it has to be resolved before the root of this package can be derived from it.
+SOURCE="$0"
+while [ -h "$SOURCE" ]
+do
+	SOURCE_DIR="$(cd -P "$(dirname "$SOURCE")" && pwd)"
+	SOURCE="$(readlink "$SOURCE")"
+	# A relative symlink is resolved against the directory holding the symlink.
+	case "$SOURCE" in
+		/*) ;;
+		*) SOURCE="$SOURCE_DIR/$SOURCE" ;;
+	esac
+done
+DIR="$(cd -P "$(dirname "$SOURCE")/.." && pwd)"
+FEATURE_CONFIG=""
+for CANDIDATE in "phpstan-feature-files.neon" "phpstan-feature-files.neon.dist"
+do
+	if [ -f "$CANDIDATE" ]
+	then
+		FEATURE_CONFIG="$(pwd)/$CANDIDATE"
+		break
+	fi
+done
+
+if [ -d "features" ] && [ -n "$FEATURE_CONFIG" ] && [ -f "$DIR/utils/phpstan-feature-files.php" ]
+then
+	TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpstan')
+	trap 'rm -rf "$TEMP_DIR"' EXIT HUP INT TERM
+
+	# Results are only reported when the extraction they are based on succeeded.
+	if php "$DIR/utils/phpstan-feature-files.php" extract features "$TEMP_DIR/blocks"
+	then
+		{
+			echo "includes:"
+			echo "	- $DIR/phpstan/feature-files.neon"
+			echo "	- $FEATURE_CONFIG"
+
+			# The functions in the `WP_CLI\Utils` namespace are pulled in through the
+			# `files` autoloader, which does not make them known to PHPStan.
+			if [ -d "vendor/wp-cli/wp-cli" ]
+			then
+				echo "parameters:"
+				echo "	scanDirectories:"
+				echo "		- $(pwd)/vendor/wp-cli/wp-cli"
+			fi
+		} > "$TEMP_DIR/phpstan-feature-files.neon"
+
+		# Blocks are spread over batches that do not declare the same symbol twice,
+		# so that PHPStan does not resolve a name to another block's declaration.
+		for BATCH in "$TEMP_DIR"/blocks/batch*
+		do
+			[ -d "$BATCH" ] || continue
+
+			NAME="$(basename "$BATCH")"
+
+			vendor/bin/phpstan --memory-limit=2048M analyse \
+				--configuration="$TEMP_DIR/phpstan-feature-files.neon" \
+				--error-format=json \
+				--no-progress \
+				"$BATCH" > "$TEMP_DIR/$NAME.json" 2>"$TEMP_DIR/$NAME.stderr"
+
+			# The findings are reported against the feature files they came from,
+			# so PHPStan's own output is only of interest when it produced none.
+			if [ ! -s "$TEMP_DIR/$NAME.json" ] && [ -s "$TEMP_DIR/$NAME.stderr" ]
+			then
+				cat "$TEMP_DIR/$NAME.stderr" >&2
+			fi
+		done
+
+		# The glob stays unexpanded when no batch produced results, which is the
+		# case for a package whose feature files hold no PHP block at all. The
+		# report is still worth running, as it lists the blocks that were skipped.
+		set -- "$TEMP_DIR"/batch*.json
+		if [ -f "$1" ]
+		then
+			php "$DIR/utils/phpstan-feature-files.php" report "$TEMP_DIR/blocks" "$@" || EXIT_CODE=$?
+		else
+			php "$DIR/utils/phpstan-feature-files.php" report "$TEMP_DIR/blocks" || EXIT_CODE=$?
+		fi
+	else
+		EXIT_CODE=1
+	fi
+fi
+
+exit $EXIT_CODE

composer.json

@@ -10,6 +10,7 @@
     "type": "phpcodesniffer-standard",
     "require": {
         "php": ">=7.2.24",
+        "ext-tokenizer": "*",
         "behat/behat": "^v3.15.0",
         "dealerdirect/phpcodesniffer-composer-installer": "^0.4.3 || ^0.5 || ^0.6.2 || ^0.7.1 || ^1.0.0",
         "php-parallel-lint/php-console-highlighter": "^1.0",

phpstan-feature-files.neon.dist

@@ -0,0 +1,8 @@
+# Opts this package into the static analysis of the PHP blocks in its feature files.
+#
+# The defaults in `phpstan/feature-files.neon` are applied first. Packages using the
+# testing framework pick up `extension.neon` through phpstan/extension-installer, so
+# they do not have to include it themselves.
+
+includes:
+	- extension.neon

phpstan/feature-files.neon

@@ -0,0 +1,27 @@
+# Defaults for the static analysis of the PHP blocks embedded in Behat feature files.
+#
+# A package opts into the analysis by adding a `phpstan-feature-files.neon` (or
+# `phpstan-feature-files.neon.dist`) file to its root. These defaults are always
+# applied first, so that file only needs to hold what it wants to change.
+#
+# The snippets in feature files are fixtures rather than production code, so the
+# level is kept below the one that asks for type declarations everywhere.
+
+parameters:
+	level: 5
+	treatPhpDocTypesAsCertain: false
+
+	# Most packages will not run into all of the errors ignored below.
+	reportUnmatchedIgnoredErrors: false
+
+	ignoreErrors:
+		# `wp eval-file` passes the remaining positional arguments to the file it runs.
+		-
+			identifier: variable.undefined
+			message: '#^Variable \$args might not be defined\.$#'
+
+		# Files a snippet pulls in are created by other steps while the scenario runs.
+		- identifier: include.fileNotFound
+		- identifier: includeOnce.fileNotFound
+		- identifier: require.fileNotFound
+		- identifier: requireOnce.fileNotFound