Skip to content

Commit 913cd94

Browse files
swissspidyclaude
swissspidy
and
claude
authored
Add PHPCS linting for PHP blocks in feature files (#340)
Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent d8c33f5 commit 913cd94

12 files changed

Lines changed: 1795 additions & 354 deletions

‎.readme-partials/USING.md‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ To make use of the WP-CLI testing framework, you need to complete the following
7878
```
7979
8080
All other [PHPCS configuration options](https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Annotated-Ruleset) are, of course, available.
81+
The PHP snippets embedded in your feature files are checked along with the rest of the package. See [Checking the code style of the PHP blocks in feature files](#checking-the-code-style-of-the-php-blocks-in-feature-files) below.
82+
8183
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.
8284
8385
7. Update your composer dependencies and regenerate your autoloader and binary folders:
@@ -165,6 +167,56 @@ Two kinds of blocks are left out of the analysis, and are listed at the end of t
165167
Blocks that declare the same class or function as another block are analysed separately from each
166168
other, so that PHPStan does not resolve a name to the wrong block's declaration.
167169

170+
### Checking the code style of the PHP blocks in feature files
171+
172+
`composer phpcs` also checks the PHP snippets that feature files embed in docstrings, and
173+
`composer phpcbf` fixes them in place. No configuration is needed, and like the analysis above the
174+
blocks are padded so that findings are reported against the feature file itself:
175+
176+
```text
177+
FILE: features/command.feature
178+
----------------------------------------------------------------------
179+
FOUND 1 ERROR AFFECTING 1 LINE
180+
----------------------------------------------------------------------
181+
438 | ERROR | [x] Expected 1 space after IF keyword; 0 found
182+
----------------------------------------------------------------------
183+
```
184+
185+
Only a docstring belonging to a step that creates a `.php` file is checked:
186+
187+
```gherkin
188+
Given a wp-content/mu-plugins/test-harness.php file:
189+
"""
190+
<?php
191+
WP_CLI::add_command( 'test-harness', 'Test_Harness' );
192+
"""
193+
```
194+
195+
Unlike the analysis above, a docstring that merely opens with `<?php` does not count. Those are
196+
routinely an expectation about the contents of a file rather than a file, and reformatting one would
197+
make it stop matching what it is checked against.
198+
199+
The defaults leave out the sniffs that look at a block as if it were a file of its own, along with
200+
those that ask of a fixture what is only worth asking of production code. They live in
201+
`phpcs/feature-files.sh` and are shared by the check and the fixer, so that the two cannot disagree
202+
over which sniff applies. A package replaces them wholesale by adding a `phpcs-feature-files.xml`
203+
(or `phpcs-feature-files.xml.dist`) ruleset to its root:
204+
205+
```xml
206+
<?xml version="1.0"?>
207+
<ruleset name="WP-CLI-PROJECT-NAME-feature-files">
208+
<arg name="warning-severity" value="0"/>
209+
210+
<rule ref="WP_CLI_CS">
211+
<exclude name="Generic.Files.InlineHTML"/>
212+
<exclude name="Squiz.Commenting.FileComment"/>
213+
</rule>
214+
</ruleset>
215+
```
216+
217+
The blocks are left alone when a run is narrowed down to a path, as in `composer phpcs -- src/`,
218+
since such an argument is about the files of the package itself.
219+
168220
### Controlling what to test
169221

170222
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-phpcbf-cleanup‎

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,82 @@
11
#!/bin/sh
22

3-
# Run the code style check only if a configuration file exists.
3+
EXIT_CODE=0
4+
5+
# 1. Run standard PHPCBF if configuration file exists.
46
if [ -f ".phpcs.xml" ] || [ -f "phpcs.xml" ] || [ -f ".phpcs.xml.dist" ] || [ -f "phpcs.xml.dist" ]
57
then
6-
vendor/bin/phpcbf "$@"
8+
vendor/bin/phpcbf "$@" || EXIT_CODE=$?
79
fi
10+
11+
# 2. Run PHPCBF over the PHP blocks in .feature files and sync back fixes.
12+
# Composer installs this script as a symlink in the vendor binary directory, so
13+
# it has to be resolved before the root of this package can be derived from it.
14+
SOURCE="$0"
15+
while [ -h "$SOURCE" ]
16+
do
17+
SOURCE_DIR="$(cd -P "$(dirname "$SOURCE")" && pwd)"
18+
SOURCE="$(readlink "$SOURCE")"
19+
# A relative symlink is resolved against the directory holding the symlink.
20+
case "$SOURCE" in
21+
/*) ;;
22+
*) SOURCE="$SOURCE_DIR/$SOURCE" ;;
23+
esac
24+
done
25+
DIR="$(cd -P "$(dirname "$SOURCE")/.." && pwd)"
26+
27+
# A ruleset of the same purpose in the package root replaces the defaults
28+
# wholesale. Both scripts read the defaults from the same file, so that the
29+
# check and the fixer cannot disagree over which sniff applies to a block.
30+
FEATURE_STANDARD=""
31+
for CANDIDATE in "phpcs-feature-files.xml" "phpcs-feature-files.xml.dist"
32+
do
33+
if [ -f "$CANDIDATE" ]
34+
then
35+
FEATURE_STANDARD="$(pwd)/$CANDIDATE"
36+
break
37+
fi
38+
done
39+
40+
FEATURE_ARGS=""
41+
if [ -z "$FEATURE_STANDARD" ] && [ -f "$DIR/phpcs/feature-files.sh" ]
42+
then
43+
. "$DIR/phpcs/feature-files.sh"
44+
FEATURE_STANDARD="$WP_CLI_TESTS_FEATURE_STANDARD"
45+
# Holds no path, so leaving it unquoted below splits it into arguments.
46+
FEATURE_ARGS="$WP_CLI_TESTS_FEATURE_ARGS"
47+
fi
48+
49+
# An argument naming what to fix applies to the files of the package itself, so
50+
# the blocks are left alone once a run has been narrowed down to a path.
51+
FIX_BLOCKS=1
52+
for ARG in "$@"
53+
do
54+
case "$ARG" in
55+
-*) ;;
56+
*) FIX_BLOCKS=0 ;;
57+
esac
58+
done
59+
60+
if [ "$FIX_BLOCKS" -eq 1 ] && [ -d "features" ] && [ -n "$FEATURE_STANDARD" ] \
61+
&& [ -f "$DIR/utils/extract-feature-php.php" ]
62+
then
63+
TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpcbf')
64+
trap 'rm -rf "$TEMP_DIR"' EXIT HUP INT TERM
65+
66+
# Fixes are only synced back when the extraction they are based on succeeded.
67+
if php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR"
68+
then
69+
if [ -n "$(ls -A "$TEMP_DIR" 2>/dev/null)" ]
70+
then
71+
# shellcheck disable=SC2086 # Intentional word splitting.
72+
vendor/bin/phpcbf --standard="$FEATURE_STANDARD" $FEATURE_ARGS \
73+
"$TEMP_DIR" >/dev/null || EXIT_CODE=$?
74+
75+
php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null || EXIT_CODE=$?
76+
fi
77+
else
78+
EXIT_CODE=1
79+
fi
80+
fi
81+
82+
exit $EXIT_CODE

‎bin/run-phpcs-tests‎

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,92 @@
11
#!/bin/sh
22

3-
# Run the code style check only if a configuration file exists.
3+
EXIT_CODE=0
4+
5+
# 1. Run standard PHP code style check if a configuration file exists.
46
if [ -f ".phpcs.xml" ] || [ -f "phpcs.xml" ] || [ -f ".phpcs.xml.dist" ] || [ -f "phpcs.xml.dist" ]
57
then
6-
vendor/bin/phpcs "$@"
8+
vendor/bin/phpcs "$@" || EXIT_CODE=$?
79
fi
10+
11+
# 2. Run PHPCS over the PHP blocks in .feature files.
12+
# Composer installs this script as a symlink in the vendor binary directory, so
13+
# it has to be resolved before the root of this package can be derived from it.
14+
SOURCE="$0"
15+
while [ -h "$SOURCE" ]
16+
do
17+
SOURCE_DIR="$(cd -P "$(dirname "$SOURCE")" && pwd)"
18+
SOURCE="$(readlink "$SOURCE")"
19+
# A relative symlink is resolved against the directory holding the symlink.
20+
case "$SOURCE" in
21+
/*) ;;
22+
*) SOURCE="$SOURCE_DIR/$SOURCE" ;;
23+
esac
24+
done
25+
DIR="$(cd -P "$(dirname "$SOURCE")/.." && pwd)"
26+
27+
# A ruleset of the same purpose in the package root replaces the defaults
28+
# wholesale. Both scripts read the defaults from the same file, so that the
29+
# check and the fixer cannot disagree over which sniff applies to a block.
30+
FEATURE_STANDARD=""
31+
for CANDIDATE in "phpcs-feature-files.xml" "phpcs-feature-files.xml.dist"
32+
do
33+
if [ -f "$CANDIDATE" ]
34+
then
35+
FEATURE_STANDARD="$(pwd)/$CANDIDATE"
36+
break
37+
fi
38+
done
39+
40+
FEATURE_ARGS=""
41+
if [ -z "$FEATURE_STANDARD" ] && [ -f "$DIR/phpcs/feature-files.sh" ]
42+
then
43+
. "$DIR/phpcs/feature-files.sh"
44+
FEATURE_STANDARD="$WP_CLI_TESTS_FEATURE_STANDARD"
45+
# Holds no path, so leaving it unquoted below splits it into arguments.
46+
FEATURE_ARGS="$WP_CLI_TESTS_FEATURE_ARGS"
47+
fi
48+
49+
# An argument naming what to check applies to the files of the package itself,
50+
# so the blocks are left alone once a run has been narrowed down to a path.
51+
CHECK_BLOCKS=1
52+
for ARG in "$@"
53+
do
54+
case "$ARG" in
55+
-*) ;;
56+
*) CHECK_BLOCKS=0 ;;
57+
esac
58+
done
59+
60+
if [ "$CHECK_BLOCKS" -eq 1 ] && [ -d "features" ] && [ -n "$FEATURE_STANDARD" ] \
61+
&& [ -f "$DIR/utils/extract-feature-php.php" ]
62+
then
63+
TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpcs')
64+
PHPCS_OUTPUT=$(mktemp 2>/dev/null || mktemp -t 'feature_phpcs_output')
65+
trap 'rm -rf "$TEMP_DIR" "$PHPCS_OUTPUT"' EXIT HUP INT TERM
66+
67+
# Results are only reported when the extraction they are based on succeeded.
68+
if php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR"
69+
then
70+
if [ -n "$(ls -A "$TEMP_DIR" 2>/dev/null)" ]
71+
then
72+
# The report is written to a file so that the status of PHPCS itself
73+
# is preserved instead of the status of the command rewriting it.
74+
# `--basepath` reduces the reported paths to the part that is worth
75+
# showing, which also keeps PHPCS from truncating them from the left
76+
# once they grow past the width of the report.
77+
# shellcheck disable=SC2086 # Intentional word splitting.
78+
vendor/bin/phpcs --standard="$FEATURE_STANDARD" $FEATURE_ARGS \
79+
--basepath="$TEMP_DIR" "$TEMP_DIR" >"$PHPCS_OUTPUT" 2>&1 || EXIT_CODE=$?
80+
81+
# Findings are reported against the feature files they came from.
82+
sed -E \
83+
-e 's|^FILE: |FILE: features/|' \
84+
-e 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' \
85+
"$PHPCS_OUTPUT"
86+
fi
87+
else
88+
EXIT_CODE=1
89+
fi
90+
fi
91+
92+
exit $EXIT_CODE

‎features/behat-steps.feature‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ Feature: Test that WP-CLI Behat steps work as expected
587587
And a send-email.php file:
588588
"""
589589
<?php
590-
wp_mail('test@example.com', 'Test', 'Body');
590+
wp_mail( 'test@example.com', 'Test', 'Body' );
591591
"""
592592
When I run `wp eval-file send-email.php`
593593
Then an email should be sent

‎features/testing.feature‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Feature: Test that WP-CLI loads.
1818
And a test_cron.php file:
1919
"""
2020
<?php
21-
$cron_disabled = defined( "DISABLE_WP_CRON" ) ? DISABLE_WP_CRON : false;
21+
$cron_disabled = defined( 'DISABLE_WP_CRON' ) ? DISABLE_WP_CRON : false;
2222
echo 'DISABLE_WP_CRON is: ' . ( $cron_disabled ? 'true' : 'false' );
2323
"""
2424

‎phpcs/feature-files.sh‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# Defaults for the code style check of the PHP blocks embedded in Behat feature
2+
# files, shared by `run-phpcs-tests` and `run-phpcbf-cleanup`.
3+
#
4+
# Keeping the list in one place is what makes the check and the fixer agree: a
5+
# sniff excluded for one but not the other would have the fixer rewrite feature
6+
# files over something the check never reports, or have the check report
7+
# something the fixer refuses to touch.
8+
#
9+
# The exclusions are passed on the command line rather than declared in a
10+
# ruleset because a ruleset aborts the whole run over a sniff that the installed
11+
# PHP_CodeSniffer does not know, while `--exclude` passes over it. The list
12+
# spans several major versions of PHP_CodeSniffer and of the standards it
13+
# builds on, and not every entry exists in all of them.
14+
#
15+
# A package replaces these defaults wholesale by adding a
16+
# `phpcs-feature-files.xml` (or `phpcs-feature-files.xml.dist`) ruleset to its
17+
# root, which is then used as the standard instead.
18+
19+
WP_CLI_TESTS_FEATURE_STANDARD="WP_CLI_CS"
20+
21+
# Warnings are advisory, and the fixer must not rewrite a feature file over
22+
# something the check does not report.
23+
WP_CLI_TESTS_FEATURE_ARGS="--warning-severity=0"
24+
25+
# A block is not a file. It is padded with one empty line per preceding line of
26+
# the feature file so that reported line numbers match it, and one that does not
27+
# bring its own opening tag is given one. Neither is part of the snippet, and
28+
# the shared docstring indentation is taken off before the check and put back
29+
# afterwards, so none of the sniffs looking at a file as a whole apply.
30+
WP_CLI_TESTS_FEATURE_EXCLUDES="Generic.Files.InlineHTML"
31+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.PHP.CharacterBeforePHPOpenTag"
32+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.Files.LineEndings"
33+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,PSR2.Files.EndFileNewline"
34+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,PSR12.Files.FileHeader"
35+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Squiz.Commenting.FileComment"
36+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.PHP.RequireStrictTypes"
37+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,WordPress.Files.FileName"
38+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.WhiteSpace.PrecisionAlignment"
39+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Squiz.WhiteSpace.SuperfluousWhitespace"
40+
41+
# A block is a fixture, not production code. Snippets exist to set up a
42+
# scenario, run inside a throwaway WordPress installation, are written to be
43+
# read at a glance, and are routinely a single class or function on their own.
44+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,WordPress.NamingConventions.PrefixAllGlobals"
45+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,WordPress.WP.GlobalVariablesOverride"
46+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,WordPress.PHP.YodaConditions"
47+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.Files.SeparateFunctionsFromOO"
48+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.Files.OneObjectStructurePerFile"
49+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.Namespaces.OneDeclarationPerFile"
50+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.Namespaces.DisallowCurlyBraceSyntax"
51+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.Namespaces.DisallowDeclarationWithoutName"
52+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,PSR2.Methods.FunctionClosingBrace"
53+
54+
# A snippet testing error handling is deliberately incomplete.
55+
WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.CodeAnalysis.EmptyStatement"
56+
57+
WP_CLI_TESTS_FEATURE_ARGS="$WP_CLI_TESTS_FEATURE_ARGS --exclude=$WP_CLI_TESTS_FEATURE_EXCLUDES"

0 commit comments

Comments
 (0)