From 6de99d4ec3ce6687e6c486a0b5977332db745186 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 14:20:39 +0200 Subject: [PATCH 01/20] Add PHPCS linting for PHP blocks in feature files --- bin/run-phpcs-tests | 25 +++- utils/extract-feature-php.php | 236 ++++++++++++++++++++++++++++++++++ 2 files changed, 259 insertions(+), 2 deletions(-) create mode 100644 utils/extract-feature-php.php diff --git a/bin/run-phpcs-tests b/bin/run-phpcs-tests index 82d96b4e5..12e42efdb 100755 --- a/bin/run-phpcs-tests +++ b/bin/run-phpcs-tests @@ -1,7 +1,28 @@ #!/bin/sh -# Run the code style check only if a configuration file exists. +EXIT_CODE=0 + +# 1. Run standard PHP code style check if a configuration file exists. if [ -f ".phpcs.xml" ] || [ -f "phpcs.xml" ] || [ -f ".phpcs.xml.dist" ] || [ -f "phpcs.xml.dist" ] then - vendor/bin/phpcs "$@" + vendor/bin/phpcs "$@" || EXIT_CODE=$? fi + +# 2. Run PHPCS over extracted PHP blocks in .feature files if features/ directory exists. +DIR="$(cd "$(dirname "$0")/.." && pwd)" +if [ -d "features" ] && [ -f "$DIR/utils/extract-feature-php.php" ] +then + TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpcs') + trap 'rm -rf "$TEMP_DIR"' EXIT HUP INT TERM + + php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" >/dev/null 2>&1 + + if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] + then + vendor/bin/phpcs --standard=WP_CLI_CS --warning-severity=0 \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Generic.WhiteSpace.DisallowSpaceIndent,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace \ + "$TEMP_DIR" 2>&1 | sed -E 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' | sed -E "s|FILE: .*/([^/]+\.feature)|FILE: features/\1|g" || EXIT_CODE=$? + fi +fi + +exit $EXIT_CODE diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php new file mode 100644 index 000000000..8d3683cbd --- /dev/null +++ b/utils/extract-feature-php.php @@ -0,0 +1,236 @@ +isDir() ? 'rmdir' : 'unlink' ); + $todo( $fileinfo->getRealPath() ); + } + } + + $directory = new RecursiveDirectoryIterator( $source_dir ); + $iterator = new RecursiveIteratorIterator( $directory ); + + foreach ( $iterator as $file ) { + if ( $file->isFile() && 'feature' === $file->getExtension() ) { + $filepath = $file->getPathname(); + $relative = substr( $filepath, strlen( $source_dir ) + 1 ); + $lines = file( $filepath ); + + $in_docstring = false; + $is_php_block = false; + $docstring_lines = []; + $start_line = 0; + + foreach ( $lines as $index => $line ) { + $trimmed = trim( $line ); + + if ( 0 === strpos( $trimmed, '"""' ) || 0 === strpos( $trimmed, "'''" ) ) { + if ( ! $in_docstring ) { + $in_docstring = true; + $is_php_block = false; + $docstring_lines = []; + $start_line = $index; + + if ( $index > 0 && preg_match( '/\b[\w\/-]+\.php\b/i', $lines[ $index - 1 ] ) ) { + $is_php_block = true; + } + } else { + $in_docstring = false; + if ( $is_php_block && ! empty( $docstring_lines ) ) { + $min_indent = PHP_INT_MAX; + foreach ( $docstring_lines as $code_line ) { + if ( '' !== trim( $code_line ) ) { + preg_match( '/^\s*/', $code_line, $m ); + $min_indent = min( $min_indent, strlen( $m[0] ) ); + } + } + if ( PHP_INT_MAX === $min_indent ) { + $min_indent = 0; + } + + $has_php_tag = false; + foreach ( $docstring_lines as $code_line ) { + if ( '' !== trim( $code_line ) ) { + if ( 0 === strpos( trim( $code_line ), ' $code_line ) { + $out_lines[ $line_idx ] = substr( $code_line, $min_indent ); + } + + $end_line = $index; + $php_flag = $has_php_tag ? 'HASPHP' : 'NOPHP'; + $target_file = $target_dir . '/' . $relative . '_L' . ( $start_line + 1 ) . '_E' . ( $end_line + 1 ) . '_' . $php_flag . '.php'; + + $target_subdir = dirname( $target_file ); + if ( ! is_dir( $target_subdir ) ) { + mkdir( $target_subdir, 0777, true ); + } + file_put_contents( $target_file, implode( '', $out_lines ) ); + } + } + continue; + } + + if ( $in_docstring ) { + $docstring_count = count( $docstring_lines ); + if ( 0 === $docstring_count && 0 === strpos( $trimmed, 'isFile() && 'php' === $file->getExtension() ) { + $temp_filepath = $file->getPathname(); + $temp_filename = $file->getFilename(); + + if ( ! preg_match( '/^(.*\.feature)_L(\d+)_E(\d+)_(HASPHP|NOPHP)\.php$/', $temp_filename, $matches ) ) { + continue; + } + + $sub_path = substr( dirname( $temp_filepath ), strlen( $target_dir ) ); + $feature_rel_path = ( '' !== $sub_path ? $sub_path . '/' : '' ) . $matches[1]; + $feature_path = $source_dir . '/' . ltrim( $feature_rel_path, '/' ); + + $files_by_feature[ $feature_path ][] = [ + 'temp_filepath' => $temp_filepath, + 'docstring_start' => (int) $matches[2] - 1, + 'docstring_end' => (int) $matches[3] - 1, + 'had_php_tag' => 'HASPHP' === $matches[4], + ]; + } + } + + foreach ( $files_by_feature as $feature_path => $blocks ) { + if ( ! file_exists( $feature_path ) ) { + continue; + } + + usort( + $blocks, + function ( $a, $b ) { + return $b['docstring_start'] <=> $a['docstring_start']; + } + ); + + $feature_lines = file( $feature_path ); + + foreach ( $blocks as $block ) { + $code_start = $block['docstring_start'] + 1; + $code_end = $block['docstring_end'] - 1; + $had_php_tag = $block['had_php_tag']; + $temp_lines = file( $block['temp_filepath'] ); + + if ( ! isset( $feature_lines[ $code_start ] ) || $code_start > $code_end ) { + continue; + } + + preg_match( '/^\s*/', $feature_lines[ $code_start ], $m ); + $indent = $m[0] ?? ' '; + + $code_lines = []; + foreach ( $temp_lines as $temp_line ) { + if ( ! $had_php_tag && false !== strpos( $temp_line, 'added_php_tag' ) ) { + continue; + } + $code_lines[] = $temp_line; + } + + while ( ! empty( $code_lines ) && '' === trim( reset( $code_lines ) ) ) { + array_shift( $code_lines ); + } + while ( ! empty( $code_lines ) && '' === trim( end( $code_lines ) ) ) { + array_pop( $code_lines ); + } + + $fixed_lines = []; + foreach ( $code_lines as $line_content ) { + if ( '' === trim( $line_content ) ) { + $fixed_lines[] = "\n"; + } else { + $fixed_lines[] = $indent . ltrim( $line_content ); + } + } + + $num_code_lines = ( $code_end - $code_start + 1 ); + array_splice( $feature_lines, $code_start, $num_code_lines, $fixed_lines ); + } + + file_put_contents( $feature_path, implode( '', $feature_lines ) ); + } +} + +$wp_cli_tests_action = $argv[1] ?? 'extract'; +if ( 'update' === $wp_cli_tests_action ) { + update_feature_php( $argv[2] ?? '', $argv[3] ?? '' ); +} else { + extract_feature_php( $argv[2] ?? $argv[1] ?? '', $argv[3] ?? $argv[2] ?? '' ); +} From 1f954c8976f401ab118e702668e5ef965c0a4b0d Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 14:20:44 +0200 Subject: [PATCH 02/20] Add PHPCBF support for feature files --- bin/run-phpcbf-cleanup | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/bin/run-phpcbf-cleanup b/bin/run-phpcbf-cleanup index f7a4d8fb1..ec2fd8134 100755 --- a/bin/run-phpcbf-cleanup +++ b/bin/run-phpcbf-cleanup @@ -1,7 +1,30 @@ #!/bin/sh -# Run the code style check only if a configuration file exists. +EXIT_CODE=0 + +# 1. Run standard PHPCBF if configuration file exists. if [ -f ".phpcs.xml" ] || [ -f "phpcs.xml" ] || [ -f ".phpcs.xml.dist" ] || [ -f "phpcs.xml.dist" ] then - vendor/bin/phpcbf "$@" + vendor/bin/phpcbf "$@" || EXIT_CODE=$? +fi + +# 2. Run PHPCBF over extracted PHP blocks in .feature files and sync back fixes. +DIR="$(cd "$(dirname "$0")/.." && pwd)" +if [ -d "features" ] && [ -f "$DIR/utils/extract-feature-php.php" ] +then + TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpcbf') + trap 'rm -rf "$TEMP_DIR"' EXIT HUP INT TERM + + php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" >/dev/null 2>&1 + + if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] + then + vendor/bin/phpcbf --standard=WP_CLI_CS \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Generic.WhiteSpace.DisallowSpaceIndent,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace \ + "$TEMP_DIR" >/dev/null 2>&1 + + php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null 2>&1 + fi fi + +exit $EXIT_CODE From af59387eff30a288c9f996d6b830fc204c0c3cc7 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 14:20:50 +0200 Subject: [PATCH 03/20] Fix PHP code style violations in feature files --- features/behat-steps.feature | 2 +- features/testing.feature | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/features/behat-steps.feature b/features/behat-steps.feature index 431f56e9c..6dc5874bd 100644 --- a/features/behat-steps.feature +++ b/features/behat-steps.feature @@ -549,7 +549,7 @@ Feature: Test that WP-CLI Behat steps work as expected And a send-email.php file: """ Date: Thu, 23 Jul 2026 14:33:37 +0200 Subject: [PATCH 04/20] Preserve relative PHP code indentation during PHPCBF update --- utils/extract-feature-php.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index 8d3683cbd..72f002710 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -216,7 +216,7 @@ function ( $a, $b ) { if ( '' === trim( $line_content ) ) { $fixed_lines[] = "\n"; } else { - $fixed_lines[] = $indent . ltrim( $line_content ); + $fixed_lines[] = $indent . $line_content; } } From f7e89ee4d7c0d817a385b4abf43b527a63b073be Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 16:08:47 +0200 Subject: [PATCH 05/20] Preserve empty lines inside feature PHP blocks during extraction --- utils/extract-feature-php.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index 72f002710..30cb7d5a0 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -96,7 +96,11 @@ function extract_feature_php( $source_dir, $target_dir ) { } foreach ( $docstring_lines as $line_idx => $code_line ) { - $out_lines[ $line_idx ] = substr( $code_line, $min_indent ); + if ( '' === trim( $code_line ) ) { + $out_lines[ $line_idx ] = "\n"; + } else { + $out_lines[ $line_idx ] = substr( $code_line, $min_indent ); + } } $end_line = $index; From 9d3728ca8b700688578fdc424d180c4104bb9028 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 16:17:35 +0200 Subject: [PATCH 06/20] Enforce standard tab indentation inside feature PHP snippets --- bin/run-phpcbf-cleanup | 2 +- bin/run-phpcs-tests | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/run-phpcbf-cleanup b/bin/run-phpcbf-cleanup index ec2fd8134..8ffcdea83 100755 --- a/bin/run-phpcbf-cleanup +++ b/bin/run-phpcbf-cleanup @@ -20,7 +20,7 @@ then if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then vendor/bin/phpcbf --standard=WP_CLI_CS \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Generic.WhiteSpace.DisallowSpaceIndent,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace \ "$TEMP_DIR" >/dev/null 2>&1 php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null 2>&1 diff --git a/bin/run-phpcs-tests b/bin/run-phpcs-tests index 12e42efdb..3a2a4cafe 100755 --- a/bin/run-phpcs-tests +++ b/bin/run-phpcs-tests @@ -20,7 +20,7 @@ then if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then vendor/bin/phpcs --standard=WP_CLI_CS --warning-severity=0 \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Generic.WhiteSpace.DisallowSpaceIndent,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace \ "$TEMP_DIR" 2>&1 | sed -E 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' | sed -E "s|FILE: .*/([^/]+\.feature)|FILE: features/\1|g" || EXIT_CODE=$? fi fi From d6304a348399848a633304d2c83f684dfdd78b0a Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 16:41:44 +0200 Subject: [PATCH 07/20] Exclude WordPress.NamingConventions.PrefixAllGlobals from feature file sniffs --- bin/run-phpcbf-cleanup | 2 +- bin/run-phpcs-tests | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/run-phpcbf-cleanup b/bin/run-phpcbf-cleanup index 8ffcdea83..61095f4f2 100755 --- a/bin/run-phpcbf-cleanup +++ b/bin/run-phpcbf-cleanup @@ -20,7 +20,7 @@ then if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then vendor/bin/phpcbf --standard=WP_CLI_CS \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals \ "$TEMP_DIR" >/dev/null 2>&1 php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null 2>&1 diff --git a/bin/run-phpcs-tests b/bin/run-phpcs-tests index 3a2a4cafe..72250d4fd 100755 --- a/bin/run-phpcs-tests +++ b/bin/run-phpcs-tests @@ -20,7 +20,7 @@ then if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then vendor/bin/phpcs --standard=WP_CLI_CS --warning-severity=0 \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals \ "$TEMP_DIR" 2>&1 | sed -E 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' | sed -E "s|FILE: .*/([^/]+\.feature)|FILE: features/\1|g" || EXIT_CODE=$? fi fi From 780a76b4b19e674ebdde45bb4140a6d65e95b93c Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 17:54:59 +0200 Subject: [PATCH 08/20] Exclude OO structure and global override rules from feature file sniffs --- bin/run-phpcbf-cleanup | 2 +- bin/run-phpcs-tests | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/run-phpcbf-cleanup b/bin/run-phpcbf-cleanup index 61095f4f2..eacfab3a2 100755 --- a/bin/run-phpcbf-cleanup +++ b/bin/run-phpcbf-cleanup @@ -20,7 +20,7 @@ then if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then vendor/bin/phpcbf --standard=WP_CLI_CS \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax \ "$TEMP_DIR" >/dev/null 2>&1 php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null 2>&1 diff --git a/bin/run-phpcs-tests b/bin/run-phpcs-tests index 72250d4fd..fa4614ee0 100755 --- a/bin/run-phpcs-tests +++ b/bin/run-phpcs-tests @@ -20,7 +20,7 @@ then if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then vendor/bin/phpcs --standard=WP_CLI_CS --warning-severity=0 \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax \ "$TEMP_DIR" 2>&1 | sed -E 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' | sed -E "s|FILE: .*/([^/]+\.feature)|FILE: features/\1|g" || EXIT_CODE=$? fi fi From b32a615f1f03a9d0c603b237644a016b121e4800 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 18:03:52 +0200 Subject: [PATCH 09/20] Exclude YodaConditions, empty catch, unnamed namespaces, and file header sniffs from feature PHP blocks --- bin/run-phpcbf-cleanup | 2 +- bin/run-phpcs-tests | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/run-phpcbf-cleanup b/bin/run-phpcbf-cleanup index eacfab3a2..bf1e38823 100755 --- a/bin/run-phpcbf-cleanup +++ b/bin/run-phpcbf-cleanup @@ -20,7 +20,7 @@ then if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then vendor/bin/phpcbf --standard=WP_CLI_CS \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax,WordPress.PHP.YodaConditions,Universal.Namespaces.DisallowDeclarationWithoutName,PSR12.Files.FileHeader,Generic.CodeAnalysis.EmptyStatement \ "$TEMP_DIR" >/dev/null 2>&1 php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null 2>&1 diff --git a/bin/run-phpcs-tests b/bin/run-phpcs-tests index fa4614ee0..d8d86025d 100755 --- a/bin/run-phpcs-tests +++ b/bin/run-phpcs-tests @@ -20,7 +20,7 @@ then if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then vendor/bin/phpcs --standard=WP_CLI_CS --warning-severity=0 \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax,WordPress.PHP.YodaConditions,Universal.Namespaces.DisallowDeclarationWithoutName,PSR12.Files.FileHeader,Generic.CodeAnalysis.EmptyStatement \ "$TEMP_DIR" 2>&1 | sed -E 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' | sed -E "s|FILE: .*/([^/]+\.feature)|FILE: features/\1|g" || EXIT_CODE=$? fi fi From 6a3fbc89a88263c84541f07364c8fa1d76e36217 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 09:59:40 +0000 Subject: [PATCH 10/20] Address review feedback on feature file PHP checks Fixes for the PHP block extraction and synchronization: * Place an added ` Claude-Session: https://claude.ai/code/session_01KVnFXuhCGs4NT2A7FmDo4e --- bin/run-phpcbf-cleanup | 18 +- bin/run-phpcs-tests | 29 +- tests/tests/TestExtractFeaturePhp.php | 526 ++++++++++++++++++++++++++ utils/extract-feature-php.php | 324 +++++++++++++--- 4 files changed, 833 insertions(+), 64 deletions(-) create mode 100644 tests/tests/TestExtractFeaturePhp.php diff --git a/bin/run-phpcbf-cleanup b/bin/run-phpcbf-cleanup index bf1e38823..97df3f445 100755 --- a/bin/run-phpcbf-cleanup +++ b/bin/run-phpcbf-cleanup @@ -15,15 +15,19 @@ then TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpcbf') trap 'rm -rf "$TEMP_DIR"' EXIT HUP INT TERM - php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" >/dev/null 2>&1 - - if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] + # Fixes are only synced back when the extraction they are based on succeeded. + if php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" >/dev/null then - vendor/bin/phpcbf --standard=WP_CLI_CS \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax,WordPress.PHP.YodaConditions,Universal.Namespaces.DisallowDeclarationWithoutName,PSR12.Files.FileHeader,Generic.CodeAnalysis.EmptyStatement \ - "$TEMP_DIR" >/dev/null 2>&1 + if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] + then + vendor/bin/phpcbf --standard=WP_CLI_CS \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax,WordPress.PHP.YodaConditions,Universal.Namespaces.DisallowDeclarationWithoutName,PSR12.Files.FileHeader,Generic.CodeAnalysis.EmptyStatement \ + "$TEMP_DIR" >/dev/null || EXIT_CODE=$? - php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null 2>&1 + php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null || EXIT_CODE=$? + fi + else + EXIT_CODE=1 fi fi diff --git a/bin/run-phpcs-tests b/bin/run-phpcs-tests index d8d86025d..b58f70e72 100755 --- a/bin/run-phpcs-tests +++ b/bin/run-phpcs-tests @@ -13,15 +13,30 @@ DIR="$(cd "$(dirname "$0")/.." && pwd)" if [ -d "features" ] && [ -f "$DIR/utils/extract-feature-php.php" ] then TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpcs') - trap 'rm -rf "$TEMP_DIR"' EXIT HUP INT TERM + PHPCS_OUTPUT=$(mktemp 2>/dev/null || mktemp -t 'feature_phpcs_output') + trap 'rm -rf "$TEMP_DIR" "$PHPCS_OUTPUT"' EXIT HUP INT TERM - php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" >/dev/null 2>&1 - - if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] + if php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" >/dev/null then - vendor/bin/phpcs --standard=WP_CLI_CS --warning-severity=0 \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax,WordPress.PHP.YodaConditions,Universal.Namespaces.DisallowDeclarationWithoutName,PSR12.Files.FileHeader,Generic.CodeAnalysis.EmptyStatement \ - "$TEMP_DIR" 2>&1 | sed -E 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' | sed -E "s|FILE: .*/([^/]+\.feature)|FILE: features/\1|g" || EXIT_CODE=$? + if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] + then + # The report is written to a file so that the status of PHPCS itself + # is preserved instead of the status of the commands rewriting it. + vendor/bin/phpcs --standard=WP_CLI_CS --warning-severity=0 \ + --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax,WordPress.PHP.YodaConditions,Universal.Namespaces.DisallowDeclarationWithoutName,PSR12.Files.FileHeader,Generic.CodeAnalysis.EmptyStatement \ + "$TEMP_DIR" >"$PHPCS_OUTPUT" 2>&1 || EXIT_CODE=$? + + # The temporary directory is reported through its resolved path. + TEMP_DIR_REAL=$(cd "$TEMP_DIR" && pwd -P) + + sed -E \ + -e 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' \ + -e "s|$TEMP_DIR_REAL/|features/|g" \ + -e "s|$TEMP_DIR/|features/|g" \ + "$PHPCS_OUTPUT" + fi + else + EXIT_CODE=1 fi fi diff --git a/tests/tests/TestExtractFeaturePhp.php b/tests/tests/TestExtractFeaturePhp.php new file mode 100644 index 000000000..d64fbe9f3 --- /dev/null +++ b/tests/tests/TestExtractFeaturePhp.php @@ -0,0 +1,526 @@ +temp_dir = Utils\get_temp_dir() . uniqid( 'wp-cli-test-extract-feature-php-', true ); + $this->features_dir = $this->temp_dir . '/features'; + $this->target_dir = $this->temp_dir . '/extracted'; + + mkdir( $this->temp_dir ); + mkdir( $this->features_dir ); + } + + protected function tear_down(): void { + if ( is_dir( $this->temp_dir ) ) { + $this->remove_dir( $this->temp_dir ); + } + + parent::tear_down(); + } + + /** + * Recursively removes a directory and its contents. + * + * @param string $dir The directory to remove. + */ + private function remove_dir( $dir ): void { + if ( ! is_dir( $dir ) ) { + return; + } + + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator( $dir, \FilesystemIterator::SKIP_DOTS ), + \RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ( $iterator as $file ) { + if ( $file->isDir() ) { + rmdir( $file->getPathname() ); + } else { + unlink( $file->getPathname() ); + } + } + + rmdir( $dir ); + } + + /** + * Runs the extract-feature-php.php script from within the temporary directory. + * + * @param string[] $args Arguments to pass to the script. + * @return array{output: string, exit_code: int} Combined output and exit code of the script. + */ + private function run_script( array $args ): array { + $script = dirname( dirname( __DIR__ ) ) . DIRECTORY_SEPARATOR . 'utils' . DIRECTORY_SEPARATOR . 'extract-feature-php.php'; + + // Use the `-n` flag to disable loading of `php.ini` and ensure a clean environment. + $command = escapeshellarg( PHP_BINARY ) . ' -n ' . escapeshellarg( $script ); + + foreach ( $args as $arg ) { + $command .= ' ' . escapeshellarg( $arg ); + } + + $command = 'cd ' . escapeshellarg( $this->temp_dir ) . ' && ' . $command . ' 2>&1'; + + $output = array(); + $exit_code = 0; + + exec( $command, $output, $exit_code ); + + return array( + 'output' => implode( "\n", $output ), + 'exit_code' => $exit_code, + ); + } + + /** + * Creates a feature file in the features directory. + * + * @param string $relative_path Path relative to the features directory. + * @param string $contents Contents of the feature file. + * @return string Full path to the created file. + */ + private function create_feature_file( $relative_path, $contents ): string { + $path = $this->features_dir . '/' . $relative_path; + + $directory = dirname( $path ); + if ( ! is_dir( $directory ) ) { + mkdir( $directory, 0777, true ); + } + + file_put_contents( $path, $contents ); + + return $path; + } + + /** + * Returns the paths of all extracted files, relative to the target directory. + * + * @return string[] Sorted list of relative file paths. + */ + private function get_extracted_files(): array { + if ( ! is_dir( $this->target_dir ) ) { + return array(); + } + + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator( $this->target_dir, \FilesystemIterator::SKIP_DOTS ) + ); + + $files = array(); + + foreach ( $iterator as $file ) { + if ( $file->isFile() ) { + $files[] = str_replace( '\\', '/', substr( $file->getPathname(), strlen( $this->target_dir ) + 1 ) ); + } + } + + sort( $files ); + + return $files; + } + + /** + * Returns the contents of an extracted file. + * + * @param string $relative_path Path relative to the target directory. + * @return string Contents of the file. + */ + private function get_extracted_contents( $relative_path ): string { + $contents = file_get_contents( $this->target_dir . '/' . $relative_path ); + + return false === $contents ? '' : $contents; + } + + public function test_extracts_block_with_opening_tag(): void { + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . "\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( array( 'example.feature_L5_E8_HASPHP.php' ), $this->get_extracted_files() ); + + // The block is padded with one empty line per preceding line of the + // feature file, so that reported line numbers keep matching. + $this->assertSame( + "\n\n\n\n\nget_extracted_contents( 'example.feature_L5_E8_HASPHP.php' ) + ); + } + + public function test_extracts_block_without_opening_tag(): void { + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . "\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " \$foo = 'bar';\n" + . " \"\"\"\n" + ); + + $result = $this->run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( array( 'example.feature_L5_E7_NOPHP.php' ), $this->get_extracted_files() ); + + // The added opening tag takes the place of the docstring delimiter, so + // that it is not overwritten by the first line of code. + $this->assertSame( + "\n\n\n\nget_extracted_contents( 'example.feature_L5_E7_NOPHP.php' ) + ); + } + + public function test_extracts_multiple_blocks_from_one_feature_file(): void { + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . "\n" + . " Scenario: Two PHP blocks\n" + . " Given a first.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( + array( + 'example.feature_L10_E13_HASPHP.php', + 'example.feature_L5_E8_HASPHP.php', + ), + $this->get_extracted_files() + ); + } + + public function test_extracts_from_nested_directories(): void { + $this->create_feature_file( + 'sub/nested.feature', + "Feature: Nested\n" + . "\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( array( 'sub/nested.feature_L5_E8_HASPHP.php' ), $this->get_extracted_files() ); + } + + public function test_extraction_preserves_relative_indentation_and_empty_lines(): void { + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( + "\n\n\n\nget_extracted_contents( 'example.feature_L4_E10_HASPHP.php' ) + ); + } + + public function test_extraction_skips_docstrings_that_are_not_php_files(): void { + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: An expectation about a file\n" + . " Then the wp-config.php file should contain:\n" + . " \"\"\"\n" + . " if ( defined( 'X' ) === false ) { define( 'X', true ); }\n" + . " \"\"\"\n" + ); + + $result = $this->run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( array(), $this->get_extracted_files() ); + } + + public function test_extraction_keeps_empty_lines_before_the_opening_tag(): void { + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . "\n" + . " create_feature_file( 'example.feature', $contents ); + + $result = $this->run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( + "\n\n\n\n\nget_extracted_contents( 'example.feature_L4_E8_HASPHP.php' ) + ); + + $result = $this->run_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + + public function test_extraction_keeps_unrelated_files_in_target_directory(): void { + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " target_dir ); + file_put_contents( $this->target_dir . '/keep-me.txt', 'important' ); + file_put_contents( $this->target_dir . '/stale.feature_L1_E2_HASPHP.php', 'run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertFileExists( $this->target_dir . '/keep-me.txt' ); + $this->assertSame( 'important', file_get_contents( $this->target_dir . '/keep-me.txt' ) ); + $this->assertFileDoesNotExist( $this->target_dir . '/stale.feature_L1_E2_HASPHP.php' ); + } + + public function test_extraction_refuses_to_use_the_source_directory_as_target(): void { + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $result = $this->run_script( array( 'extract', 'features', 'features' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertFileExists( $feature_file ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + + public function test_extraction_refuses_to_use_the_current_directory_as_target(): void { + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', '.' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertDirectoryExists( $this->features_dir ); + } + + public function test_directories_are_not_mistaken_for_an_action(): void { + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( array( 'example.feature_L4_E7_HASPHP.php' ), $this->get_extracted_files() ); + } + + public function test_missing_arguments_are_reported(): void { + $result = $this->run_script( array( 'extract' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertStringContainsString( 'Usage:', $result['output'] ); + } + + public function test_update_syncs_fixes_back_into_the_feature_file(): void { + $feature_file = $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', 'extracted' ) ); + + $extracted = $this->target_dir . '/example.feature_L4_E7_HASPHP.php'; + file_put_contents( $extracted, "\n\n\n\nrun_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " \$foo='bar';\n" + . " \"\"\"\n" + ); + + $this->run_script( array( 'extract', 'features', 'extracted' ) ); + + $extracted = $this->target_dir . '/example.feature_L4_E6_NOPHP.php'; + file_put_contents( $extracted, "\n\n\nrun_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " \$foo = 'bar';\n" + . " \"\"\"\n", + file_get_contents( $feature_file ) + ); + } + + public function test_update_without_changes_leaves_the_feature_file_untouched(): void { + // Includes a block starting and ending with an empty line, nested + // directories, and code that is indented relative to the block. + $contents = "Feature: Example\n" + . "\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . "\n" + . " create_feature_file( 'sub/example.feature', $contents ); + + $this->run_script( array( 'extract', 'features', 'extracted' ) ); + $result = $this->run_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + + public function test_update_reports_unexpected_content_without_changing_the_feature_file(): void { + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $this->run_script( array( 'extract', 'features', 'extracted' ) ); + + // Shift the whole block, so the padding no longer lines up. + $extracted = $this->target_dir . '/example.feature_L4_E7_HASPHP.php'; + file_put_contents( $extracted, "\$shifted = true;\n" . (string) file_get_contents( $extracted ) ); + + $result = $this->run_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } +} diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index 30cb7d5a0..4da4667ce 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -6,35 +6,136 @@ namespace WP_CLI\Tests; +use FilesystemIterator; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; +/** + * Pattern matching the file names created during extraction. + */ +const EXTRACTED_FILE_PATTERN = '/^(.*\.feature)_L(\d+)_E(\d+)_(HASPHP|NOPHP)\.php$/'; + +/** + * Determine whether a directory can be used as extraction target. + * + * Extraction removes previously extracted files from the target directory, + * so guard against pointing it at a directory holding actual project files. + * + * @param string $target_dir Target directory to output extracted .php files. + * @param string $source_dir Source directory containing .feature files. + * @return bool Whether the target directory can be used. + */ +function is_valid_target_dir( $target_dir, $source_dir ) { + if ( '' === $target_dir || '.' === $target_dir || '..' === $target_dir ) { + return false; + } + + // A Windows drive root, such as `C:` or `C:\`. + if ( preg_match( '/^[a-z]:\\\\?$/i', $target_dir ) ) { + return false; + } + + $target_real = realpath( $target_dir ); + + // A directory that does not exist yet gets created during extraction. + if ( false === $target_real ) { + return true; + } + + $cwd = getcwd(); + if ( false !== $cwd && realpath( $cwd ) === $target_real ) { + return false; + } + + $source_real = realpath( $source_dir ); + if ( false === $source_real ) { + return true; + } + + if ( $source_real === $target_real ) { + return false; + } + + // The target directory contains the feature files themselves. + if ( 0 === strpos( $source_real . DIRECTORY_SEPARATOR, $target_real . DIRECTORY_SEPARATOR ) ) { + return false; + } + + return true; +} + +/** + * Remove files of a previous extraction from the target directory. + * + * Only files created by this script and the directories that held them are + * removed, so that an unrelated file in the target directory is never lost. + * + * @param string $target_dir Target directory containing extracted .php files. + * @return void + */ +function remove_extracted_files( $target_dir ) { + if ( ! is_dir( $target_dir ) ) { + return; + } + + $files = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator( $target_dir, RecursiveDirectoryIterator::SKIP_DOTS ), + RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ( $files as $fileinfo ) { + $pathname = $fileinfo->getPathname(); + + if ( $fileinfo->isDir() ) { + $contents = new FilesystemIterator( $pathname ); + if ( ! $contents->valid() ) { + rmdir( $pathname ); + } + } elseif ( preg_match( EXTRACTED_FILE_PATTERN, $fileinfo->getFilename() ) ) { + unlink( $pathname ); + } + } +} + +/** + * Determine whether a step creates a PHP file. + * + * The docstring following such a step holds the contents of a PHP file, while + * docstrings following other steps -- an expectation about the contents of a + * file, for example -- are not necessarily PHP code and must not be touched. + * + * @param string $line Line preceding a docstring. + * @return bool Whether the line is a step creating a PHP file. + */ +function is_php_file_step( $line ) { + return 1 === preg_match( '/^\s*(?:Given|When|Then|And|But|\*)\s+an?\s+[\w\/.-]+\.php\s+(?:cache\s+)?file:\s*$/i', $line ); +} + /** * Extract PHP blocks from a source directory of feature files to a target directory. * * @param string $source_dir Source directory containing .feature files. * @param string $target_dir Target directory to output extracted .php files. - * @return void + * @return bool Whether extraction completed successfully. */ function extract_feature_php( $source_dir, $target_dir ) { $source_dir = rtrim( $source_dir, '/' ); $target_dir = rtrim( $target_dir, '/' ); if ( ! is_dir( $source_dir ) ) { - return; + fwrite( STDERR, sprintf( 'Source directory "%s" does not exist.', $source_dir ) . PHP_EOL ); + return false; } - if ( is_dir( $target_dir ) ) { - $files = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator( $target_dir, RecursiveDirectoryIterator::SKIP_DOTS ), - RecursiveIteratorIterator::CHILD_FIRST - ); - foreach ( $files as $fileinfo ) { - $todo = ( $fileinfo->isDir() ? 'rmdir' : 'unlink' ); - $todo( $fileinfo->getRealPath() ); - } + if ( ! is_valid_target_dir( $target_dir, $source_dir ) ) { + fwrite( STDERR, sprintf( 'Refusing to use "%s" as target directory.', $target_dir ) . PHP_EOL ); + return false; } + remove_extracted_files( $target_dir ); + + $success = true; + $directory = new RecursiveDirectoryIterator( $source_dir ); $iterator = new RecursiveIteratorIterator( $directory ); @@ -44,10 +145,17 @@ function extract_feature_php( $source_dir, $target_dir ) { $relative = substr( $filepath, strlen( $source_dir ) + 1 ); $lines = file( $filepath ); + if ( false === $lines ) { + fwrite( STDERR, sprintf( 'Could not read "%s".', $filepath ) . PHP_EOL ); + $success = false; + continue; + } + $in_docstring = false; $is_php_block = false; - $docstring_lines = []; + $has_content = false; $start_line = 0; + $docstring_lines = []; foreach ( $lines as $index => $line ) { $trimmed = trim( $line ); @@ -56,10 +164,11 @@ function extract_feature_php( $source_dir, $target_dir ) { if ( ! $in_docstring ) { $in_docstring = true; $is_php_block = false; + $has_content = false; $docstring_lines = []; $start_line = $index; - if ( $index > 0 && preg_match( '/\b[\w\/-]+\.php\b/i', $lines[ $index - 1 ] ) ) { + if ( $index > 0 && is_php_file_step( $lines[ $index - 1 ] ) ) { $is_php_block = true; } } else { @@ -68,7 +177,7 @@ function extract_feature_php( $source_dir, $target_dir ) { $min_indent = PHP_INT_MAX; foreach ( $docstring_lines as $code_line ) { if ( '' !== trim( $code_line ) ) { - preg_match( '/^\s*/', $code_line, $m ); + preg_match( '/^[ \t]*/', $code_line, $m ); $min_indent = min( $min_indent, strlen( $m[0] ) ); } } @@ -91,8 +200,10 @@ function extract_feature_php( $source_dir, $target_dir ) { $out_lines[ $i ] = "\n"; } + // The docstring delimiter is the line right before the first line of + // code, so an added opening tag goes there to keep line numbers intact. if ( ! $has_php_tag ) { - $out_lines[ $start_line + 1 ] = " $code_line ) { @@ -108,27 +219,104 @@ function extract_feature_php( $source_dir, $target_dir ) { $target_file = $target_dir . '/' . $relative . '_L' . ( $start_line + 1 ) . '_E' . ( $end_line + 1 ) . '_' . $php_flag . '.php'; $target_subdir = dirname( $target_file ); - if ( ! is_dir( $target_subdir ) ) { - mkdir( $target_subdir, 0777, true ); + if ( ! is_dir( $target_subdir ) && ! mkdir( $target_subdir, 0777, true ) && ! is_dir( $target_subdir ) ) { + fwrite( STDERR, sprintf( 'Could not create directory "%s".', $target_subdir ) . PHP_EOL ); + $success = false; + continue; + } + + if ( false === file_put_contents( $target_file, implode( '', $out_lines ) ) ) { + fwrite( STDERR, sprintf( 'Could not write "%s".', $target_file ) . PHP_EOL ); + $success = false; } - file_put_contents( $target_file, implode( '', $out_lines ) ); } } continue; } if ( $in_docstring ) { - $docstring_count = count( $docstring_lines ); - if ( 0 === $docstring_count && 0 === strpos( $trimmed, ' $line ) { + $trimmed = trim( $line ); + + if ( '' === $trimmed ) { + continue; + } + + // The opening tag added during extraction sits right before the code. + if ( ! $had_php_tag && $index === $code_start - 1 && 0 === strpos( $trimmed, 'getPathname(); $temp_filename = $file->getFilename(); - if ( ! preg_match( '/^(.*\.feature)_L(\d+)_E(\d+)_(HASPHP|NOPHP)\.php$/', $temp_filename, $matches ) ) { + if ( ! preg_match( EXTRACTED_FILE_PATTERN, $temp_filename, $matches ) ) { continue; } @@ -173,6 +362,8 @@ function update_feature_php( $source_dir, $target_dir ) { } } + $success = true; + foreach ( $files_by_feature as $feature_path => $blocks ) { if ( ! file_exists( $feature_path ) ) { continue; @@ -187,54 +378,87 @@ function ( $a, $b ) { $feature_lines = file( $feature_path ); + if ( false === $feature_lines ) { + fwrite( STDERR, sprintf( 'Could not read "%s".', $feature_path ) . PHP_EOL ); + $success = false; + continue; + } + foreach ( $blocks as $block ) { - $code_start = $block['docstring_start'] + 1; - $code_end = $block['docstring_end'] - 1; - $had_php_tag = $block['had_php_tag']; - $temp_lines = file( $block['temp_filepath'] ); + $code_start = $block['docstring_start'] + 1; + $code_end = $block['docstring_end'] - 1; + $temp_lines = file( $block['temp_filepath'] ); + + if ( false === $temp_lines ) { + fwrite( STDERR, sprintf( 'Could not read "%s".', $block['temp_filepath'] ) . PHP_EOL ); + $success = false; + continue; + } if ( ! isset( $feature_lines[ $code_start ] ) || $code_start > $code_end ) { continue; } - preg_match( '/^\s*/', $feature_lines[ $code_start ], $m ); - $indent = $m[0] ?? ' '; + $code_lines = strip_extraction_padding( $temp_lines, $code_start, $block['had_php_tag'] ); - $code_lines = []; - foreach ( $temp_lines as $temp_line ) { - if ( ! $had_php_tag && false !== strpos( $temp_line, 'added_php_tag' ) ) { - continue; - } - $code_lines[] = $temp_line; + if ( null === $code_lines ) { + fwrite( + STDERR, + sprintf( 'Unexpected content in "%s", not syncing this block.', $block['temp_filepath'] ) . PHP_EOL + ); + $success = false; + continue; } - while ( ! empty( $code_lines ) && '' === trim( reset( $code_lines ) ) ) { - array_shift( $code_lines ); - } - while ( ! empty( $code_lines ) && '' === trim( end( $code_lines ) ) ) { - array_pop( $code_lines ); - } + $indent = get_block_indent( $feature_lines, $code_start, $code_end, $block['docstring_start'] ); $fixed_lines = []; foreach ( $code_lines as $line_content ) { if ( '' === trim( $line_content ) ) { $fixed_lines[] = "\n"; - } else { - $fixed_lines[] = $indent . $line_content; + continue; + } + + $fixed_line = $indent . $line_content; + if ( "\n" !== substr( $fixed_line, -1 ) ) { + $fixed_line .= "\n"; } + + $fixed_lines[] = $fixed_line; } $num_code_lines = ( $code_end - $code_start + 1 ); array_splice( $feature_lines, $code_start, $num_code_lines, $fixed_lines ); } - file_put_contents( $feature_path, implode( '', $feature_lines ) ); + if ( false === file_put_contents( $feature_path, implode( '', $feature_lines ) ) ) { + fwrite( STDERR, sprintf( 'Could not write "%s".', $feature_path ) . PHP_EOL ); + $success = false; + } } + + return $success; +} + +$wp_cli_tests_args = array_slice( $argv, 1 ); +$wp_cli_tests_action = 'extract'; + +// Only treat the first argument as an action if it actually is one, so that +// a source directory does not accidentally end up being used as target. +if ( isset( $wp_cli_tests_args[0] ) && in_array( $wp_cli_tests_args[0], [ 'extract', 'update' ], true ) ) { + $wp_cli_tests_action = array_shift( $wp_cli_tests_args ); +} + +$wp_cli_tests_source = $wp_cli_tests_args[0] ?? ''; +$wp_cli_tests_target = $wp_cli_tests_args[1] ?? ''; + +if ( '' === $wp_cli_tests_source || '' === $wp_cli_tests_target ) { + fwrite( STDERR, 'Usage: extract-feature-php.php [extract|update] ' . PHP_EOL ); + exit( 1 ); } -$wp_cli_tests_action = $argv[1] ?? 'extract'; if ( 'update' === $wp_cli_tests_action ) { - update_feature_php( $argv[2] ?? '', $argv[3] ?? '' ); -} else { - extract_feature_php( $argv[2] ?? $argv[1] ?? '', $argv[3] ?? $argv[2] ?? '' ); + exit( update_feature_php( $wp_cli_tests_source, $wp_cli_tests_target ) ? 0 : 1 ); } + +exit( extract_feature_php( $wp_cli_tests_source, $wp_cli_tests_target ) ? 0 : 1 ); From 2c962f335b5b58be9d6995c6bcb647b60e4fbbf7 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 6 Aug 2026 15:39:46 +0200 Subject: [PATCH 11/20] Address some code review feedback --- tests/tests/TestExtractFeaturePhp.php | 46 +++++++++++++++++++++++++++ utils/extract-feature-php.php | 42 +++++++++++++++++------- 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/tests/tests/TestExtractFeaturePhp.php b/tests/tests/TestExtractFeaturePhp.php index d64fbe9f3..0ecaaaf10 100644 --- a/tests/tests/TestExtractFeaturePhp.php +++ b/tests/tests/TestExtractFeaturePhp.php @@ -523,4 +523,50 @@ public function test_update_reports_unexpected_content_without_changing_the_feat $this->assertSame( 1, $result['exit_code'] ); $this->assertSame( $contents, file_get_contents( $feature_file ) ); } + + public function test_update_reports_missing_feature_file(): void { + $feature_file = $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', 'extracted' ) ); + + unlink( $feature_file ); + + $result = $this->run_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertStringContainsString( 'does not exist', $result['output'] ); + } + + public function test_update_skips_block_when_source_coordinates_mismatch(): void { + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $this->run_script( array( 'extract', 'features', 'extracted' ) ); + + // Modify the feature file so the step preceding the docstring no longer creates a PHP file. + $modified_contents = str_replace( 'Given a test.php file:', 'Given a non-php step:', $contents ); + file_put_contents( $feature_file, $modified_contents ); + + $result = $this->run_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertStringContainsString( 'Unexpected content in', $result['output'] ); + $this->assertSame( $modified_contents, file_get_contents( $feature_file ) ); + } } diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index 4da4667ce..32262fa53 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -30,8 +30,8 @@ function is_valid_target_dir( $target_dir, $source_dir ) { return false; } - // A Windows drive root, such as `C:` or `C:\`. - if ( preg_match( '/^[a-z]:\\\\?$/i', $target_dir ) ) { + // A Windows drive root, such as `C:`, `C:\`, or `C:/`. + if ( preg_match( '/^[a-z]:[\\\\\/]?$/i', $target_dir ) ) { return false; } @@ -119,8 +119,8 @@ function is_php_file_step( $line ) { * @return bool Whether extraction completed successfully. */ function extract_feature_php( $source_dir, $target_dir ) { - $source_dir = rtrim( $source_dir, '/' ); - $target_dir = rtrim( $target_dir, '/' ); + $source_dir = rtrim( str_replace( '\\', '/', $source_dir ), '/' ); + $target_dir = rtrim( str_replace( '\\', '/', $target_dir ), '/' ); if ( ! is_dir( $source_dir ) ) { fwrite( STDERR, sprintf( 'Source directory "%s" does not exist.', $source_dir ) . PHP_EOL ); @@ -141,7 +141,7 @@ function extract_feature_php( $source_dir, $target_dir ) { foreach ( $iterator as $file ) { if ( $file->isFile() && 'feature' === $file->getExtension() ) { - $filepath = $file->getPathname(); + $filepath = str_replace( '\\', '/', $file->getPathname() ); $relative = substr( $filepath, strlen( $source_dir ) + 1 ); $lines = file( $filepath ); @@ -327,8 +327,8 @@ function strip_extraction_padding( $temp_lines, $code_start, $had_php_tag ) { * @return bool Whether all blocks were synced successfully. */ function update_feature_php( $source_dir, $target_dir ) { - $source_dir = rtrim( $source_dir, '/' ); - $target_dir = rtrim( $target_dir, '/' ); + $source_dir = rtrim( str_replace( '\\', '/', $source_dir ), '/' ); + $target_dir = rtrim( str_replace( '\\', '/', $target_dir ), '/' ); if ( ! is_dir( $target_dir ) ) { fwrite( STDERR, sprintf( 'Target directory "%s" does not exist.', $target_dir ) . PHP_EOL ); @@ -342,7 +342,7 @@ function update_feature_php( $source_dir, $target_dir ) { foreach ( $iterator as $file ) { if ( $file->isFile() && 'php' === $file->getExtension() ) { - $temp_filepath = $file->getPathname(); + $temp_filepath = str_replace( '\\', '/', $file->getPathname() ); $temp_filename = $file->getFilename(); if ( ! preg_match( EXTRACTED_FILE_PATTERN, $temp_filename, $matches ) ) { @@ -366,6 +366,8 @@ function update_feature_php( $source_dir, $target_dir ) { foreach ( $files_by_feature as $feature_path => $blocks ) { if ( ! file_exists( $feature_path ) ) { + fwrite( STDERR, sprintf( 'Feature file "%s" does not exist.', $feature_path ) . PHP_EOL ); + $success = false; continue; } @@ -385,9 +387,11 @@ function ( $a, $b ) { } foreach ( $blocks as $block ) { - $code_start = $block['docstring_start'] + 1; - $code_end = $block['docstring_end'] - 1; - $temp_lines = file( $block['temp_filepath'] ); + $docstring_start = $block['docstring_start']; + $docstring_end = $block['docstring_end']; + $code_start = $docstring_start + 1; + $code_end = $docstring_end - 1; + $temp_lines = file( $block['temp_filepath'] ); if ( false === $temp_lines ) { fwrite( STDERR, sprintf( 'Could not read "%s".', $block['temp_filepath'] ) . PHP_EOL ); @@ -395,7 +399,21 @@ function ( $a, $b ) { continue; } - if ( ! isset( $feature_lines[ $code_start ] ) || $code_start > $code_end ) { + if ( + $code_start > $code_end + || ! isset( $feature_lines[ $docstring_start ] ) + || ! isset( $feature_lines[ $docstring_end ] ) + || ( 0 !== strpos( trim( $feature_lines[ $docstring_start ] ), '"""' ) && 0 !== strpos( trim( $feature_lines[ $docstring_start ] ), "'''" ) ) + || ( 0 !== strpos( trim( $feature_lines[ $docstring_end ] ), '"""' ) && 0 !== strpos( trim( $feature_lines[ $docstring_end ] ), "'''" ) ) + || 0 === $docstring_start + || ! isset( $feature_lines[ $docstring_start - 1 ] ) + || ! is_php_file_step( $feature_lines[ $docstring_start - 1 ] ) + ) { + fwrite( + STDERR, + sprintf( 'Unexpected content in "%s", not syncing this block.', $feature_path ) . PHP_EOL + ); + $success = false; continue; } From 9c79335707ad17bcee9b24f6b98f6b095383e42b Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 6 Aug 2026 16:00:07 +0200 Subject: [PATCH 12/20] Address code review feedback --- tests/tests/TestExtractFeaturePhp.php | 50 +++++++++++++++++++++++---- utils/extract-feature-php.php | 5 +++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/tests/tests/TestExtractFeaturePhp.php b/tests/tests/TestExtractFeaturePhp.php index 0ecaaaf10..0a9e07cb8 100644 --- a/tests/tests/TestExtractFeaturePhp.php +++ b/tests/tests/TestExtractFeaturePhp.php @@ -84,7 +84,8 @@ private function run_script( array $args ): array { $command .= ' ' . escapeshellarg( $arg ); } - $command = 'cd ' . escapeshellarg( $this->temp_dir ) . ' && ' . $command . ' 2>&1'; + $cd_command = Utils\is_windows() ? 'cd /d ' : 'cd '; + $command = $cd_command . escapeshellarg( $this->temp_dir ) . ' && ' . $command . ' 2>&1'; $output = array(); $exit_code = 0; @@ -235,6 +236,14 @@ public function test_extracts_multiple_blocks_from_one_feature_file(): void { ), $this->get_extracted_files() ); + $this->assertSame( + "\n\n\n\n\nget_extracted_contents( 'example.feature_L5_E8_HASPHP.php' ) + ); + $this->assertSame( + "\n\n\n\n\n\n\n\n\n\nget_extracted_contents( 'example.feature_L10_E13_HASPHP.php' ) + ); } public function test_extracts_from_nested_directories(): void { @@ -366,21 +375,50 @@ public function test_extraction_refuses_to_use_the_source_directory_as_target(): } public function test_extraction_refuses_to_use_the_current_directory_as_target(): void { - $this->create_feature_file( - 'example.feature', - "Feature: Example\n" + $contents = "Feature: Example\n" . " Scenario: A PHP block\n" . " Given a test.php file:\n" . " \"\"\"\n" . " create_feature_file( 'example.feature', $contents ); $result = $this->run_script( array( 'extract', 'features', '.' ) ); $this->assertSame( 1, $result['exit_code'] ); $this->assertDirectoryExists( $this->features_dir ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + + $extracted_files = array(); + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator( $this->temp_dir, \FilesystemIterator::SKIP_DOTS ) + ); + foreach ( $iterator as $file ) { + if ( $file->isFile() && 'php' === $file->getExtension() ) { + $extracted_files[] = $file->getPathname(); + } + } + $this->assertSame( array(), $extracted_files ); + } + + public function test_extraction_reports_unterminated_docstring(): void { + $this->create_feature_file( + 'unterminated.feature', + "Feature: Unterminated\n" + . " Scenario: Unterminated docstring\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertStringContainsString( 'Unterminated docstring', $result['output'] ); + $this->assertSame( array(), $this->get_extracted_files() ); } public function test_directories_are_not_mistaken_for_an_action(): void { diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index 32262fa53..dcecc6918 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -248,6 +248,11 @@ function extract_feature_php( $source_dir, $target_dir ) { $docstring_lines[ $index ] = $line; } } + + if ( $in_docstring ) { + fwrite( STDERR, sprintf( 'Unterminated docstring in "%s".', $filepath ) . PHP_EOL ); + $success = false; + } } } From 3c60271f7422ccf84ed5f76a93b929bcff3cc3cf Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 26 Aug 2026 16:11:58 +0000 Subject: [PATCH 13/20] Keep the feature file round trip lossless Extraction took a number of leading whitespace characters off every line of a block and syncing put the indentation of the block's first line back, which is only the same thing when that line is the least indented one. A block whose opening tag sits deeper than the code below it drifted further to the right on every `composer phpcbf`, and one mixing tabs and spaces came back with one swapped for the other, both without a single sniff having fired. Extraction now takes off the indentation prefix that all lines of a block share, and syncing puts exactly that prefix back. Extraction also counted a docstring opening with `run_script( array( 'update', 'features', 'extracted' ) ); $this->assertSame( 1, $result['exit_code'] ); - $this->assertStringContainsString( 'Unexpected content in', $result['output'] ); + $this->assertStringContainsString( 'is no longer the one that was checked', $result['output'] ); $this->assertSame( $modified_contents, file_get_contents( $feature_file ) ); } + + public function test_extraction_skips_php_docstrings_that_do_not_belong_to_a_php_file_step(): void { + // The block opens with `create_feature_file( 'example.feature', $contents ); + + $result = $this->run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( array(), $this->get_extracted_files() ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + + public function test_update_preserves_a_block_indented_below_its_opening_tag(): void { + // The first line of the block is not the one carrying the least + // indentation, so the indentation to restore cannot be read off it. + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $this->run_script( array( 'extract', 'features', 'extracted' ) ); + $result = $this->run_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + + public function test_update_preserves_mixed_tab_and_space_indentation(): void { + // Extraction takes a shared prefix off the block rather than a number of + // characters, so a tab never comes back as a space or the other way round. + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $this->run_script( array( 'extract', 'features', 'extracted' ) ); + $result = $this->run_script( array( 'update', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + + public function test_extraction_refuses_to_use_a_root_directory_as_target(): void { + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $result = $this->run_script( array( 'extract', 'features', '/' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertStringContainsString( 'Refusing to use', $result['output'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + + public function test_extraction_refuses_a_target_that_only_resolves_to_a_root(): void { + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $result = $this->run_script( array( 'extract', 'features', str_repeat( '../', 64 ) ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertStringContainsString( 'Refusing to use', $result['output'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } } diff --git a/tests/tests/TestPhpStanFeatureFiles.php b/tests/tests/TestPhpStanFeatureFiles.php index 275957a3c..9a807bb7c 100644 --- a/tests/tests/TestPhpStanFeatureFiles.php +++ b/tests/tests/TestPhpStanFeatureFiles.php @@ -546,6 +546,42 @@ public function test_extraction_refuses_to_use_the_source_directory_as_target(): $this->assertSame( $contents, file_get_contents( $feature_file ) ); } + public function test_extraction_refuses_to_use_a_root_directory_as_target(): void { + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $result = $this->run_script( array( 'extract', 'features', '/' ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertStringContainsString( 'Refusing to use', $result['output'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + + public function test_extraction_refuses_a_target_that_only_resolves_to_a_root(): void { + $contents = "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " create_feature_file( 'example.feature', $contents ); + + $result = $this->run_script( array( 'extract', 'features', str_repeat( '../', 64 ) ) ); + + $this->assertSame( 1, $result['exit_code'] ); + $this->assertStringContainsString( 'Refusing to use', $result['output'] ); + $this->assertSame( $contents, file_get_contents( $feature_file ) ); + } + public function test_extraction_refuses_to_use_the_current_directory_as_target(): void { $contents = "Feature: Example\n" . " Scenario: A PHP block\n" diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index dcecc6918..5a8380abd 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -15,6 +15,25 @@ */ const EXTRACTED_FILE_PATTERN = '/^(.*\.feature)_L(\d+)_E(\d+)_(HASPHP|NOPHP)\.php$/'; +/** + * Determine whether a path is the root of a filesystem or of a drive. + * + * @param string $path Path to check. + * @return bool Whether the path is a root directory. + */ +function is_root_dir( $path ) { + if ( '' === $path ) { + return false; + } + + // A Windows drive root, such as `C:`, `C:\`, or `C:/`. + if ( preg_match( '/^[a-z]:[\\\\\/]?$/i', $path ) ) { + return true; + } + + return '' === rtrim( $path, '/\\' ); +} + /** * Determine whether a directory can be used as extraction target. * @@ -30,13 +49,17 @@ function is_valid_target_dir( $target_dir, $source_dir ) { return false; } - // A Windows drive root, such as `C:`, `C:\`, or `C:/`. - if ( preg_match( '/^[a-z]:[\\\\\/]?$/i', $target_dir ) ) { + if ( is_root_dir( $target_dir ) ) { return false; } $target_real = realpath( $target_dir ); + // Also covers a path that only resolves to a root, such as `features/../..`. + if ( false !== $target_real && is_root_dir( $target_real ) ) { + return false; + } + // A directory that does not exist yet gets created during extraction. if ( false === $target_real ) { return true; @@ -56,8 +79,11 @@ function is_valid_target_dir( $target_dir, $source_dir ) { return false; } - // The target directory contains the feature files themselves. - if ( 0 === strpos( $source_real . DIRECTORY_SEPARATOR, $target_real . DIRECTORY_SEPARATOR ) ) { + // The target directory contains the feature files themselves. A root + // directory already ends in a separator, so appending another one would + // keep the comparison below from ever matching it. + $target_prefix = rtrim( $target_real, '/\\' ) . DIRECTORY_SEPARATOR; + if ( 0 === strpos( $source_real . DIRECTORY_SEPARATOR, $target_prefix ) ) { return false; } @@ -67,8 +93,9 @@ function is_valid_target_dir( $target_dir, $source_dir ) { /** * Remove files of a previous extraction from the target directory. * - * Only files created by this script and the directories that held them are - * removed, so that an unrelated file in the target directory is never lost. + * Only files created by this script are removed, so that an unrelated file in + * the target directory is never lost. Directories are removed once they are + * empty, which includes an empty directory that was already there. * * @param string $target_dir Target directory containing extracted .php files. * @return void @@ -78,6 +105,14 @@ function remove_extracted_files( $target_dir ) { return; } + // The caller is expected to have rejected such a directory already, but + // the walk below is not something to start on a whole filesystem by + // accident. + $target_real = realpath( $target_dir ); + if ( is_root_dir( $target_dir ) || ( false !== $target_real && is_root_dir( $target_real ) ) ) { + return; + } + $files = new RecursiveIteratorIterator( new RecursiveDirectoryIterator( $target_dir, RecursiveDirectoryIterator::SKIP_DOTS ), RecursiveIteratorIterator::CHILD_FIRST @@ -97,6 +132,47 @@ function remove_extracted_files( $target_dir ) { } } +/** + * Determine the indentation that all lines holding code share. + * + * This is what extraction takes off a block and what syncing puts back, so it + * is determined as an actual prefix rather than as a number of characters: a + * block mixing tabs and spaces would otherwise come back with one swapped for + * the other. Blank lines carry no indentation of their own and are left out. + * + * @param string[] $lines Lines to compare. + * @return string|null Shared indentation, or null if no line holds code. + */ +function get_common_indent( array $lines ) { + $common = null; + + foreach ( $lines as $line ) { + if ( '' === trim( $line ) ) { + continue; + } + + preg_match( '/^[ \t]*/', $line, $matches ); + + if ( null === $common ) { + $common = $matches[0]; + continue; + } + + $length = min( strlen( $common ), strlen( $matches[0] ) ); + while ( $length > 0 && substr( $common, 0, $length ) !== substr( $matches[0], 0, $length ) ) { + --$length; + } + + $common = substr( $common, 0, $length ); + + if ( '' === $common ) { + break; + } + } + + return $common; +} + /** * Determine whether a step creates a PHP file. * @@ -104,6 +180,12 @@ function remove_extracted_files( $target_dir ) { * docstrings following other steps -- an expectation about the contents of a * file, for example -- are not necessarily PHP code and must not be touched. * + * This is the only thing that makes a docstring a PHP block here. The analysis + * in `phpstan-feature-files.php` also takes one that opens with ` Date: Wed, 26 Aug 2026 16:12:12 +0000 Subject: [PATCH 14/20] Make the feature file code style check work outside this package The root of this package was derived from `dirname "$0"`, but Composer installs these scripts as a symlink in the vendor binary directory, where that resolves to `vendor` instead. `vendor/utils/extract-feature-php.php` does not exist, so the check was skipped in every package using the testing framework and only ever ran here, where `bin` and `utils` are siblings. The symlink is now resolved first, the way `run-phpstan-tests` already does. PHPCS truncates a reported path from the left once it grows past the width of the report, which happened before the temporary directory was rewritten out of it and left findings pointing at `...N1dTe6Y/some/feature.feature`. Passing `--basepath` reduces the paths to the part worth showing, so the report needs a single anchored substitution and no longer depends on the length of a path it does not control. The list of sniffs that do not apply to a block was spelled out in both scripts. A future edit to one of them would have had the fixer rewrite feature files over something the check never reports, so the list moves to `phpcs/feature-files.sh`, which both read. That also gets the fixer the `--warning-severity=0` only the check was passing. A package replaces the defaults by adding a `phpcs-feature-files.xml` ruleset to its root. The blocks are left alone when a run is narrowed down to a path, as in `composer phpcs -- src/`, since such an argument is about the files of the package itself rather than about its feature files. --- bin/run-phpcbf-cleanup | 62 ++++++++++++++++++++++++++++++---- bin/run-phpcs-tests | 77 ++++++++++++++++++++++++++++++++++-------- phpcs/feature-files.sh | 57 +++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 21 deletions(-) create mode 100644 phpcs/feature-files.sh diff --git a/bin/run-phpcbf-cleanup b/bin/run-phpcbf-cleanup index 97df3f445..677a13ec3 100755 --- a/bin/run-phpcbf-cleanup +++ b/bin/run-phpcbf-cleanup @@ -8,20 +8,68 @@ then vendor/bin/phpcbf "$@" || EXIT_CODE=$? fi -# 2. Run PHPCBF over extracted PHP blocks in .feature files and sync back fixes. -DIR="$(cd "$(dirname "$0")/.." && pwd)" -if [ -d "features" ] && [ -f "$DIR/utils/extract-feature-php.php" ] +# 2. Run PHPCBF over the PHP blocks in .feature files and sync back fixes. +# 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)" + +# A ruleset of the same purpose in the package root replaces the defaults +# wholesale. Both scripts read the defaults from the same file, so that the +# check and the fixer cannot disagree over which sniff applies to a block. +FEATURE_STANDARD="" +for CANDIDATE in "phpcs-feature-files.xml" "phpcs-feature-files.xml.dist" +do + if [ -f "$CANDIDATE" ] + then + FEATURE_STANDARD="$(pwd)/$CANDIDATE" + break + fi +done + +FEATURE_ARGS="" +if [ -z "$FEATURE_STANDARD" ] && [ -f "$DIR/phpcs/feature-files.sh" ] +then + . "$DIR/phpcs/feature-files.sh" + FEATURE_STANDARD="$WP_CLI_TESTS_FEATURE_STANDARD" + # Holds no path, so leaving it unquoted below splits it into arguments. + FEATURE_ARGS="$WP_CLI_TESTS_FEATURE_ARGS" +fi + +# An argument naming what to fix applies to the files of the package itself, so +# the blocks are left alone once a run has been narrowed down to a path. +FIX_BLOCKS=1 +for ARG in "$@" +do + case "$ARG" in + -*) ;; + *) FIX_BLOCKS=0 ;; + esac +done + +if [ "$FIX_BLOCKS" -eq 1 ] && [ -d "features" ] && [ -n "$FEATURE_STANDARD" ] \ + && [ -f "$DIR/utils/extract-feature-php.php" ] then TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpcbf') trap 'rm -rf "$TEMP_DIR"' EXIT HUP INT TERM # Fixes are only synced back when the extraction they are based on succeeded. - if php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" >/dev/null + if php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" then - if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] + if [ -n "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then - vendor/bin/phpcbf --standard=WP_CLI_CS \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax,WordPress.PHP.YodaConditions,Universal.Namespaces.DisallowDeclarationWithoutName,PSR12.Files.FileHeader,Generic.CodeAnalysis.EmptyStatement \ + # shellcheck disable=SC2086 # Intentional word splitting. + vendor/bin/phpcbf --standard="$FEATURE_STANDARD" $FEATURE_ARGS \ "$TEMP_DIR" >/dev/null || EXIT_CODE=$? php "$DIR/utils/extract-feature-php.php" update features "$TEMP_DIR" >/dev/null || EXIT_CODE=$? diff --git a/bin/run-phpcs-tests b/bin/run-phpcs-tests index b58f70e72..e973e85fa 100755 --- a/bin/run-phpcs-tests +++ b/bin/run-phpcs-tests @@ -8,31 +8,80 @@ then vendor/bin/phpcs "$@" || EXIT_CODE=$? fi -# 2. Run PHPCS over extracted PHP blocks in .feature files if features/ directory exists. -DIR="$(cd "$(dirname "$0")/.." && pwd)" -if [ -d "features" ] && [ -f "$DIR/utils/extract-feature-php.php" ] +# 2. Run PHPCS over the PHP blocks in .feature files. +# 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)" + +# A ruleset of the same purpose in the package root replaces the defaults +# wholesale. Both scripts read the defaults from the same file, so that the +# check and the fixer cannot disagree over which sniff applies to a block. +FEATURE_STANDARD="" +for CANDIDATE in "phpcs-feature-files.xml" "phpcs-feature-files.xml.dist" +do + if [ -f "$CANDIDATE" ] + then + FEATURE_STANDARD="$(pwd)/$CANDIDATE" + break + fi +done + +FEATURE_ARGS="" +if [ -z "$FEATURE_STANDARD" ] && [ -f "$DIR/phpcs/feature-files.sh" ] +then + . "$DIR/phpcs/feature-files.sh" + FEATURE_STANDARD="$WP_CLI_TESTS_FEATURE_STANDARD" + # Holds no path, so leaving it unquoted below splits it into arguments. + FEATURE_ARGS="$WP_CLI_TESTS_FEATURE_ARGS" +fi + +# An argument naming what to check applies to the files of the package itself, +# so the blocks are left alone once a run has been narrowed down to a path. +CHECK_BLOCKS=1 +for ARG in "$@" +do + case "$ARG" in + -*) ;; + *) CHECK_BLOCKS=0 ;; + esac +done + +if [ "$CHECK_BLOCKS" -eq 1 ] && [ -d "features" ] && [ -n "$FEATURE_STANDARD" ] \ + && [ -f "$DIR/utils/extract-feature-php.php" ] then TEMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'feature_phpcs') PHPCS_OUTPUT=$(mktemp 2>/dev/null || mktemp -t 'feature_phpcs_output') trap 'rm -rf "$TEMP_DIR" "$PHPCS_OUTPUT"' EXIT HUP INT TERM - if php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" >/dev/null + # Results are only reported when the extraction they are based on succeeded. + if php "$DIR/utils/extract-feature-php.php" extract features "$TEMP_DIR" then - if [ -d "$TEMP_DIR" ] && [ "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] + if [ -n "$(ls -A "$TEMP_DIR" 2>/dev/null)" ] then # The report is written to a file so that the status of PHPCS itself - # is preserved instead of the status of the commands rewriting it. - vendor/bin/phpcs --standard=WP_CLI_CS --warning-severity=0 \ - --exclude=Generic.Files.InlineHTML,Generic.Files.LineEndings,WordPress.Files.FileName,Squiz.Commenting.FileComment,Universal.WhiteSpace.PrecisionAlignment,PSR2.Files.EndFileNewline,PSR2.Methods.FunctionClosingBrace,Generic.PHP.CharacterBeforePHPOpenTag,Generic.PHP.RequireStrictTypes,Squiz.WhiteSpace.SuperfluousWhitespace,WordPress.NamingConventions.PrefixAllGlobals,Universal.Files.SeparateFunctionsFromOO,Generic.Files.OneObjectStructurePerFile,WordPress.WP.GlobalVariablesOverride,Universal.Namespaces.OneDeclarationPerFile,Universal.Namespaces.DisallowCurlyBraceSyntax,WordPress.PHP.YodaConditions,Universal.Namespaces.DisallowDeclarationWithoutName,PSR12.Files.FileHeader,Generic.CodeAnalysis.EmptyStatement \ - "$TEMP_DIR" >"$PHPCS_OUTPUT" 2>&1 || EXIT_CODE=$? - - # The temporary directory is reported through its resolved path. - TEMP_DIR_REAL=$(cd "$TEMP_DIR" && pwd -P) + # is preserved instead of the status of the command rewriting it. + # `--basepath` reduces the reported paths to the part that is worth + # showing, which also keeps PHPCS from truncating them from the left + # once they grow past the width of the report. + # shellcheck disable=SC2086 # Intentional word splitting. + vendor/bin/phpcs --standard="$FEATURE_STANDARD" $FEATURE_ARGS \ + --basepath="$TEMP_DIR" "$TEMP_DIR" >"$PHPCS_OUTPUT" 2>&1 || EXIT_CODE=$? + # Findings are reported against the feature files they came from. sed -E \ + -e 's|^FILE: |FILE: features/|' \ -e 's/\.feature_L[0-9]+_E[0-9]+_(HASPHP|NOPHP)\.php/.feature/g' \ - -e "s|$TEMP_DIR_REAL/|features/|g" \ - -e "s|$TEMP_DIR/|features/|g" \ "$PHPCS_OUTPUT" fi else diff --git a/phpcs/feature-files.sh b/phpcs/feature-files.sh new file mode 100644 index 000000000..0d8cba7e5 --- /dev/null +++ b/phpcs/feature-files.sh @@ -0,0 +1,57 @@ +# Defaults for the code style check of the PHP blocks embedded in Behat feature +# files, shared by `run-phpcs-tests` and `run-phpcbf-cleanup`. +# +# Keeping the list in one place is what makes the check and the fixer agree: a +# sniff excluded for one but not the other would have the fixer rewrite feature +# files over something the check never reports, or have the check report +# something the fixer refuses to touch. +# +# The exclusions are passed on the command line rather than declared in a +# ruleset because a ruleset aborts the whole run over a sniff that the installed +# PHP_CodeSniffer does not know, while `--exclude` passes over it. The list +# spans several major versions of PHP_CodeSniffer and of the standards it +# builds on, and not every entry exists in all of them. +# +# A package replaces these defaults wholesale by adding a +# `phpcs-feature-files.xml` (or `phpcs-feature-files.xml.dist`) ruleset to its +# root, which is then used as the standard instead. + +WP_CLI_TESTS_FEATURE_STANDARD="WP_CLI_CS" + +# Warnings are advisory, and the fixer must not rewrite a feature file over +# something the check does not report. +WP_CLI_TESTS_FEATURE_ARGS="--warning-severity=0" + +# A block is not a file. It is padded with one empty line per preceding line of +# the feature file so that reported line numbers match it, and one that does not +# bring its own opening tag is given one. Neither is part of the snippet, and +# the shared docstring indentation is taken off before the check and put back +# afterwards, so none of the sniffs looking at a file as a whole apply. +WP_CLI_TESTS_FEATURE_EXCLUDES="Generic.Files.InlineHTML" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.PHP.CharacterBeforePHPOpenTag" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.Files.LineEndings" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,PSR2.Files.EndFileNewline" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,PSR12.Files.FileHeader" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Squiz.Commenting.FileComment" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.PHP.RequireStrictTypes" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,WordPress.Files.FileName" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.WhiteSpace.PrecisionAlignment" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Squiz.WhiteSpace.SuperfluousWhitespace" + +# A block is a fixture, not production code. Snippets exist to set up a +# scenario, run inside a throwaway WordPress installation, are written to be +# read at a glance, and are routinely a single class or function on their own. +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,WordPress.NamingConventions.PrefixAllGlobals" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,WordPress.WP.GlobalVariablesOverride" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,WordPress.PHP.YodaConditions" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.Files.SeparateFunctionsFromOO" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.Files.OneObjectStructurePerFile" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.Namespaces.OneDeclarationPerFile" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.Namespaces.DisallowCurlyBraceSyntax" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Universal.Namespaces.DisallowDeclarationWithoutName" +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,PSR2.Methods.FunctionClosingBrace" + +# A snippet testing error handling is deliberately incomplete. +WP_CLI_TESTS_FEATURE_EXCLUDES="$WP_CLI_TESTS_FEATURE_EXCLUDES,Generic.CodeAnalysis.EmptyStatement" + +WP_CLI_TESTS_FEATURE_ARGS="$WP_CLI_TESTS_FEATURE_ARGS --exclude=$WP_CLI_TESTS_FEATURE_EXCLUDES" From 8b6fc76f4c6301588d008b72b5733a9ee7cf3544 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 26 Aug 2026 16:12:12 +0000 Subject: [PATCH 15/20] Document the code style check of the PHP blocks in feature files Adds a section alongside the one for the static analysis, covering which docstrings are checked and why a docstring merely opening with ` + + + + + + + + +``` + +The blocks are left alone when a run is narrowed down to a path, as in `composer phpcs -- src/`, +since such an argument is about the files of the package itself. + ### 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: From 6b61bd4fc967b4f72eb97094d9abc147f0fc77e5 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 26 Aug 2026 16:47:59 +0000 Subject: [PATCH 16/20] Share the parts both feature file tools agree on `extract-feature-php.php` and `phpstan-feature-files.php` each carried their own copy of the checks deciding which docstrings hold a PHP block and where a block may be extracted to, in the same namespace and byte for byte the same. Nothing catches such a pair drifting apart: the two run as separate processes, `utils` is not autoloaded, and the static analysis only looks at `src` and `tests`. The copies had already started to diverge, with a fix for an extraction target resolving to the filesystem root landing in one of them first. They move to `utils/feature-php-blocks.php`, which both scripts pull in themselves. It stays out of the autoloader, as loading a set of functions into every package using the framework buys nothing. `remove_extracted_files()` takes the pattern matching the extracted file names as an argument, since the two tools name their files differently, and the manifest that only the analysis writes is removed by the analysis. --- utils/extract-feature-php.php | 142 +-------------------------- utils/feature-php-blocks.php | 169 ++++++++++++++++++++++++++++++++ utils/phpstan-feature-files.php | 159 ++---------------------------- 3 files changed, 181 insertions(+), 289 deletions(-) create mode 100644 utils/feature-php-blocks.php diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index 5a8380abd..c42d02cf0 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -6,132 +6,16 @@ namespace WP_CLI\Tests; -use FilesystemIterator; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; +require_once __DIR__ . '/feature-php-blocks.php'; + /** * Pattern matching the file names created during extraction. */ const EXTRACTED_FILE_PATTERN = '/^(.*\.feature)_L(\d+)_E(\d+)_(HASPHP|NOPHP)\.php$/'; -/** - * Determine whether a path is the root of a filesystem or of a drive. - * - * @param string $path Path to check. - * @return bool Whether the path is a root directory. - */ -function is_root_dir( $path ) { - if ( '' === $path ) { - return false; - } - - // A Windows drive root, such as `C:`, `C:\`, or `C:/`. - if ( preg_match( '/^[a-z]:[\\\\\/]?$/i', $path ) ) { - return true; - } - - return '' === rtrim( $path, '/\\' ); -} - -/** - * Determine whether a directory can be used as extraction target. - * - * Extraction removes previously extracted files from the target directory, - * so guard against pointing it at a directory holding actual project files. - * - * @param string $target_dir Target directory to output extracted .php files. - * @param string $source_dir Source directory containing .feature files. - * @return bool Whether the target directory can be used. - */ -function is_valid_target_dir( $target_dir, $source_dir ) { - if ( '' === $target_dir || '.' === $target_dir || '..' === $target_dir ) { - return false; - } - - if ( is_root_dir( $target_dir ) ) { - return false; - } - - $target_real = realpath( $target_dir ); - - // Also covers a path that only resolves to a root, such as `features/../..`. - if ( false !== $target_real && is_root_dir( $target_real ) ) { - return false; - } - - // A directory that does not exist yet gets created during extraction. - if ( false === $target_real ) { - return true; - } - - $cwd = getcwd(); - if ( false !== $cwd && realpath( $cwd ) === $target_real ) { - return false; - } - - $source_real = realpath( $source_dir ); - if ( false === $source_real ) { - return true; - } - - if ( $source_real === $target_real ) { - return false; - } - - // The target directory contains the feature files themselves. A root - // directory already ends in a separator, so appending another one would - // keep the comparison below from ever matching it. - $target_prefix = rtrim( $target_real, '/\\' ) . DIRECTORY_SEPARATOR; - if ( 0 === strpos( $source_real . DIRECTORY_SEPARATOR, $target_prefix ) ) { - return false; - } - - return true; -} - -/** - * Remove files of a previous extraction from the target directory. - * - * Only files created by this script are removed, so that an unrelated file in - * the target directory is never lost. Directories are removed once they are - * empty, which includes an empty directory that was already there. - * - * @param string $target_dir Target directory containing extracted .php files. - * @return void - */ -function remove_extracted_files( $target_dir ) { - if ( ! is_dir( $target_dir ) ) { - return; - } - - // The caller is expected to have rejected such a directory already, but - // the walk below is not something to start on a whole filesystem by - // accident. - $target_real = realpath( $target_dir ); - if ( is_root_dir( $target_dir ) || ( false !== $target_real && is_root_dir( $target_real ) ) ) { - return; - } - - $files = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator( $target_dir, RecursiveDirectoryIterator::SKIP_DOTS ), - RecursiveIteratorIterator::CHILD_FIRST - ); - - foreach ( $files as $fileinfo ) { - $pathname = $fileinfo->getPathname(); - - if ( $fileinfo->isDir() ) { - $contents = new FilesystemIterator( $pathname ); - if ( ! $contents->valid() ) { - rmdir( $pathname ); - } - } elseif ( preg_match( EXTRACTED_FILE_PATTERN, $fileinfo->getFilename() ) ) { - unlink( $pathname ); - } - } -} - /** * Determine the indentation that all lines holding code share. * @@ -173,26 +57,6 @@ function get_common_indent( array $lines ) { return $common; } -/** - * Determine whether a step creates a PHP file. - * - * The docstring following such a step holds the contents of a PHP file, while - * docstrings following other steps -- an expectation about the contents of a - * file, for example -- are not necessarily PHP code and must not be touched. - * - * This is the only thing that makes a docstring a PHP block here. The analysis - * in `phpstan-feature-files.php` also takes one that opens with `getPathname(); + + if ( $fileinfo->isDir() ) { + $contents = new FilesystemIterator( $pathname ); + if ( ! $contents->valid() ) { + rmdir( $pathname ); + } + } elseif ( preg_match( $pattern, $fileinfo->getFilename() ) ) { + unlink( $pathname ); + } + } +} diff --git a/utils/phpstan-feature-files.php b/utils/phpstan-feature-files.php index f112536a1..d62483a05 100644 --- a/utils/phpstan-feature-files.php +++ b/utils/phpstan-feature-files.php @@ -17,6 +17,8 @@ use RecursiveDirectoryIterator; use RecursiveIteratorIterator; +require_once __DIR__ . '/feature-php-blocks.php'; + /** * Pattern matching the file names created during extraction. */ @@ -27,155 +29,6 @@ */ const MANIFEST_FILE = 'manifest.json'; -/** - * Bring a path into the form used to compare it against another path. - * - * @param string $path Path to normalize. - * @return string Normalized path. - */ -function normalize_path( $path ) { - $path = rtrim( str_replace( '\\', '/', $path ), '/' ); - - // Windows paths are not case sensitive. - return DIRECTORY_SEPARATOR === '\\' ? strtolower( $path ) : $path; -} - -/** - * Determine whether a path is the root of a filesystem or of a drive. - * - * @param string $path Path to check. - * @return bool Whether the path is a root directory. - */ -function is_root_dir( $path ) { - if ( '' === $path ) { - return false; - } - - // A Windows drive root, such as `C:`, `C:\`, or `C:/`. - if ( preg_match( '/^[a-z]:[\\\\\/]?$/i', $path ) ) { - return true; - } - - return '' === rtrim( $path, '/\\' ); -} - -/** - * Determine whether a directory can be used as extraction target. - * - * Extraction removes previously extracted files from the target directory, - * so guard against pointing it at a directory holding actual project files. - * - * @param string $target_dir Target directory to output extracted .php files. - * @param string $source_dir Source directory containing .feature files. - * @return bool Whether the target directory can be used. - */ -function is_valid_target_dir( $target_dir, $source_dir ) { - if ( '' === $target_dir || '.' === $target_dir || '..' === $target_dir ) { - return false; - } - - if ( is_root_dir( $target_dir ) ) { - return false; - } - - $target_real = realpath( $target_dir ); - - // Also covers a path that only resolves to a root, such as `features/../..`. - if ( false !== $target_real && is_root_dir( $target_real ) ) { - return false; - } - - // A directory that does not exist yet gets created during extraction. - if ( false === $target_real ) { - return true; - } - - $cwd = getcwd(); - if ( false !== $cwd && realpath( $cwd ) === $target_real ) { - return false; - } - - $source_real = realpath( $source_dir ); - if ( false === $source_real ) { - return true; - } - - if ( $source_real === $target_real ) { - return false; - } - - // The target directory contains the feature files themselves. A root - // directory already ends in a separator, so appending another one would - // keep the comparison below from ever matching it. - $target_prefix = rtrim( $target_real, '/\\' ) . DIRECTORY_SEPARATOR; - if ( 0 === strpos( $source_real . DIRECTORY_SEPARATOR, $target_prefix ) ) { - return false; - } - - return true; -} - -/** - * Remove files of a previous extraction from the target directory. - * - * Only files created by this script and the directories that held them are - * removed, so that an unrelated file in the target directory is never lost. - * - * @param string $target_dir Target directory containing extracted .php files. - * @return void - */ -function remove_extracted_files( $target_dir ) { - if ( ! is_dir( $target_dir ) ) { - return; - } - - // The caller is expected to have rejected such a directory already, but - // the walk below is not something to start on a whole filesystem by - // accident. - $target_real = realpath( $target_dir ); - if ( is_root_dir( $target_dir ) || ( false !== $target_real && is_root_dir( $target_real ) ) ) { - return; - } - - $manifest = $target_dir . '/' . MANIFEST_FILE; - if ( is_file( $manifest ) ) { - unlink( $manifest ); - } - - $files = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator( $target_dir, RecursiveDirectoryIterator::SKIP_DOTS ), - RecursiveIteratorIterator::CHILD_FIRST - ); - - foreach ( $files as $fileinfo ) { - $pathname = $fileinfo->getPathname(); - - if ( $fileinfo->isDir() ) { - $contents = new FilesystemIterator( $pathname ); - if ( ! $contents->valid() ) { - rmdir( $pathname ); - } - } elseif ( preg_match( EXTRACTED_FILE_PATTERN, $fileinfo->getFilename() ) ) { - unlink( $pathname ); - } - } -} - -/** - * Determine whether a step creates a PHP file. - * - * The docstring following such a step holds the contents of a PHP file, while - * docstrings following other steps -- an expectation about the contents of a - * file, for example -- are not necessarily PHP code. A docstring that opens - * with ` Date: Wed, 26 Aug 2026 16:50:47 +0000 Subject: [PATCH 17/20] Share how the blocks of a feature file are found The loop walking a feature file for docstrings existed twice: as `collect_blocks()` in the analysis and inlined in the extractor of the code style check, which is where the two had already come to disagree about what counts as a PHP block. The check needs the narrower rule, since it writes a block back and reformatting an expectation about the contents of a file would make it stop matching, while the analysis wants every block it can read. Rather than a flag deciding that inside the shared loop, a block now comes back saying which rule recognised it and whether it brings its own opening tag, and each tool takes it from there. That also drops the separate pass the check made over a block to answer the second question. The walk over the feature files of a directory is shared along with it. The code style check gains the stable order the analysis already relied on. --- utils/extract-feature-php.php | 157 ++++++++++++++------------------ utils/feature-php-blocks.php | 95 +++++++++++++++++++ utils/phpstan-feature-files.php | 79 +--------------- 3 files changed, 166 insertions(+), 165 deletions(-) diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index c42d02cf0..3572ad331 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -57,6 +57,45 @@ function get_common_indent( array $lines ) { return $common; } +/** + * Turn a PHP block into the source of a standalone PHP file. + * + * The block is padded with one empty line per preceding line of the feature + * file, so that the line numbers PHP_CodeSniffer reports are the line numbers + * of the feature file. A block that does not bring its own opening tag is + * given one on the line of the docstring delimiter, which is the line right + * before the first line of code and therefore free. + * + * Unlike the analysis in `phpstan-feature-files.php`, an opening tag that the + * block brings along stays where it is. A fix is written back into the feature + * file, so the checked copy has to line up with the block it came from. + * + * @param array{start: int, lines: array, has_php_tag: bool} $block Block to render. + * @return string Source of the standalone PHP file. + */ +function render_fixable_block( array $block ) { + $indent_length = strlen( (string) get_common_indent( $block['lines'] ) ); + + $out_lines = []; + for ( $i = 0; $i < $block['start'] + 1; $i++ ) { + $out_lines[ $i ] = "\n"; + } + + if ( ! $block['has_php_tag'] ) { + $out_lines[ $block['start'] ] = " $code_line ) { + if ( '' === trim( $code_line ) ) { + $out_lines[ $line_idx ] = "\n"; + } else { + $out_lines[ $line_idx ] = substr( $code_line, $indent_length ); + } + } + + return implode( '', $out_lines ); +} + /** * Extract PHP blocks from a source directory of feature files to a target directory. * @@ -82,102 +121,44 @@ function extract_feature_php( $source_dir, $target_dir ) { $success = true; - $directory = new RecursiveDirectoryIterator( $source_dir ); - $iterator = new RecursiveIteratorIterator( $directory ); + foreach ( find_feature_files( $source_dir ) as $filepath ) { + $relative = substr( $filepath, strlen( $source_dir ) + 1 ); + $lines = file( $filepath ); - foreach ( $iterator as $file ) { - if ( $file->isFile() && 'feature' === $file->getExtension() ) { - $filepath = str_replace( '\\', '/', $file->getPathname() ); - $relative = substr( $filepath, strlen( $source_dir ) + 1 ); - $lines = file( $filepath ); + if ( false === $lines ) { + fwrite( STDERR, sprintf( 'Could not read "%s".', $filepath ) . PHP_EOL ); + $success = false; + continue; + } - if ( false === $lines ) { - fwrite( STDERR, sprintf( 'Could not read "%s".', $filepath ) . PHP_EOL ); - $success = false; + $blocks = collect_blocks( $lines ); + + if ( null === $blocks ) { + fwrite( STDERR, sprintf( 'Unterminated docstring in "%s".', $filepath ) . PHP_EOL ); + $success = false; + continue; + } + + foreach ( $blocks as $block ) { + // A docstring that merely opens with ` $line ) { - $trimmed = trim( $line ); - - if ( 0 === strpos( $trimmed, '"""' ) || 0 === strpos( $trimmed, "'''" ) ) { - if ( ! $in_docstring ) { - $in_docstring = true; - $is_php_block = false; - $docstring_lines = []; - $start_line = $index; - - if ( $index > 0 && is_php_file_step( $lines[ $index - 1 ] ) ) { - $is_php_block = true; - } - } else { - $in_docstring = false; - if ( $is_php_block && ! empty( $docstring_lines ) ) { - $indent_length = strlen( (string) get_common_indent( $docstring_lines ) ); - - $has_php_tag = false; - foreach ( $docstring_lines as $code_line ) { - if ( '' !== trim( $code_line ) ) { - if ( 0 === strpos( trim( $code_line ), ' $code_line ) { - if ( '' === trim( $code_line ) ) { - $out_lines[ $line_idx ] = "\n"; - } else { - $out_lines[ $line_idx ] = substr( $code_line, $indent_length ); - } - } - - $end_line = $index; - $php_flag = $has_php_tag ? 'HASPHP' : 'NOPHP'; - $target_file = $target_dir . '/' . $relative . '_L' . ( $start_line + 1 ) . '_E' . ( $end_line + 1 ) . '_' . $php_flag . '.php'; - - $target_subdir = dirname( $target_file ); - if ( ! is_dir( $target_subdir ) && ! mkdir( $target_subdir, 0777, true ) && ! is_dir( $target_subdir ) ) { - fwrite( STDERR, sprintf( 'Could not create directory "%s".', $target_subdir ) . PHP_EOL ); - $success = false; - continue; - } - - if ( false === file_put_contents( $target_file, implode( '', $out_lines ) ) ) { - fwrite( STDERR, sprintf( 'Could not write "%s".', $target_file ) . PHP_EOL ); - $success = false; - } - } - } - continue; - } + $php_flag = $block['has_php_tag'] ? 'HASPHP' : 'NOPHP'; + $target_file = $target_dir . '/' . $relative . '_L' . ( $block['start'] + 1 ) . '_E' . ( $block['end'] + 1 ) . '_' . $php_flag . '.php'; - if ( $in_docstring ) { - // Every line is kept, including the empty ones leading up to - // an opening tag, so that line numbers keep matching. - $docstring_lines[ $index ] = $line; - } + $target_subdir = dirname( $target_file ); + if ( ! is_dir( $target_subdir ) && ! mkdir( $target_subdir, 0777, true ) && ! is_dir( $target_subdir ) ) { + fwrite( STDERR, sprintf( 'Could not create directory "%s".', $target_subdir ) . PHP_EOL ); + $success = false; + continue; } - if ( $in_docstring ) { - fwrite( STDERR, sprintf( 'Unterminated docstring in "%s".', $filepath ) . PHP_EOL ); + if ( false === file_put_contents( $target_file, render_fixable_block( $block ) ) ) { + fwrite( STDERR, sprintf( 'Could not write "%s".', $target_file ) . PHP_EOL ); $success = false; } } diff --git a/utils/feature-php-blocks.php b/utils/feature-php-blocks.php index 9c0a75501..bdc5b857a 100644 --- a/utils/feature-php-blocks.php +++ b/utils/feature-php-blocks.php @@ -125,6 +125,101 @@ function is_php_file_step( $line ) { return 1 === preg_match( '/^\s*(?:Given|When|Then|And|But|\*)\s+an?\s+[\w\/.-]+\.php\s+(?:cache\s+)?file:\s*$/i', $line ); } +/** + * Find the feature files of a source directory. + * + * @param string $source_dir Source directory containing .feature files. + * @return string[] Sorted paths of the feature files, with forward slashes. + */ +function find_feature_files( $source_dir ) { + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator( $source_dir, FilesystemIterator::SKIP_DOTS ) + ); + + $feature_files = []; + + foreach ( $iterator as $file ) { + if ( $file->isFile() && 'feature' === $file->getExtension() ) { + $feature_files[] = str_replace( '\\', '/', $file->getPathname() ); + } + } + + // A stable order keeps the results of a run comparable to those of the next. + sort( $feature_files ); + + return $feature_files; +} + +/** + * Collect the PHP blocks contained in a single feature file. + * + * A docstring holds a PHP block when the step it belongs to creates a `.php` + * file, and also when it opens with `, from_step: bool, has_php_tag: bool}>|null Blocks, or null on an unterminated docstring. + */ +function collect_blocks( array $lines ) { + $blocks = []; + $in_docstring = false; + $from_step = false; + $has_php_tag = false; + $has_content = false; + $start_line = 0; + $docstring_lines = []; + + foreach ( $lines as $index => $line ) { + $trimmed = trim( $line ); + + if ( 0 === strpos( $trimmed, '"""' ) || 0 === strpos( $trimmed, "'''" ) ) { + if ( ! $in_docstring ) { + $in_docstring = true; + $from_step = $index > 0 && is_php_file_step( $lines[ $index - 1 ] ); + $has_php_tag = false; + $has_content = false; + $docstring_lines = []; + $start_line = $index; + } else { + $in_docstring = false; + + if ( ( $from_step || $has_php_tag ) && ! empty( $docstring_lines ) ) { + $blocks[] = [ + 'start' => $start_line, + 'end' => $index, + 'lines' => $docstring_lines, + 'from_step' => $from_step, + 'has_php_tag' => $has_php_tag, + ]; + } + } + continue; + } + + if ( $in_docstring ) { + if ( ! $has_content && 0 === strpos( $trimmed, '}>|null Blocks, or null on an unterminated docstring. - */ -function collect_blocks( array $lines ) { - $blocks = []; - $in_docstring = false; - $is_php_block = false; - $has_content = false; - $start_line = 0; - $docstring_lines = []; - - foreach ( $lines as $index => $line ) { - $trimmed = trim( $line ); - - if ( 0 === strpos( $trimmed, '"""' ) || 0 === strpos( $trimmed, "'''" ) ) { - if ( ! $in_docstring ) { - $in_docstring = true; - $is_php_block = $index > 0 && is_php_file_step( $lines[ $index - 1 ] ); - $has_content = false; - $docstring_lines = []; - $start_line = $index; - } else { - $in_docstring = false; - - if ( $is_php_block && ! empty( $docstring_lines ) ) { - $blocks[] = [ - 'start' => $start_line, - 'end' => $index, - 'lines' => $docstring_lines, - ]; - } - } - continue; - } - - if ( $in_docstring ) { - // A block opening with `isFile() && 'feature' === $file->getExtension() ) { - $feature_files[] = str_replace( '\\', '/', $file->getPathname() ); - } - } - - // The order determines the batch a block ends up in, so keep it stable. - sort( $feature_files ); - - foreach ( $feature_files as $filepath ) { + // The order determines the batch a block ends up in, so it has to be stable. + foreach ( find_feature_files( $source_dir ) as $filepath ) { $relative = substr( $filepath, strlen( $source_dir ) + 1 ); $lines = file( $filepath ); From 49d2f819c41e94aca45daa63730d54c3ffa1b673 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 26 Aug 2026 16:53:50 +0000 Subject: [PATCH 18/20] Share how much indentation comes off a block Both tools took the indentation a block shares off its lines before handing them to PHP_CodeSniffer or PHPStan, and each did it its own way. The analysis removed a number of characters, namely the smallest number of leading whitespace characters any line of the block carried, which is only the same thing as removing the shared prefix while every line is indented with the same characters. It now uses the same `get_common_indent()` the code style check does. For a well-formed docstring the two agree, as every line of a block then starts with the indentation of the docstring, and the analysis of every feature file in config-command, entity-command and scaffold-command comes out byte for byte the same. They part ways over a line indented with a tab where the rest of the block uses spaces: the old computation would take the tab off one line and a single space off another, leaving the block with an indentation it never had. Nothing is written back on this side, so unlike the same defect on the code style side this was not corrupting anything, but it is one implementation now rather than two, and the one that is left cannot lose the distinction. --- tests/tests/TestPhpStanFeatureFiles.php | 24 +++++++++++++++ utils/extract-feature-php.php | 41 ------------------------- utils/feature-php-blocks.php | 41 +++++++++++++++++++++++++ utils/phpstan-feature-files.php | 13 ++------ 4 files changed, 67 insertions(+), 52 deletions(-) diff --git a/tests/tests/TestPhpStanFeatureFiles.php b/tests/tests/TestPhpStanFeatureFiles.php index 9a807bb7c..55c7c21ad 100644 --- a/tests/tests/TestPhpStanFeatureFiles.php +++ b/tests/tests/TestPhpStanFeatureFiles.php @@ -546,6 +546,30 @@ public function test_extraction_refuses_to_use_the_source_directory_as_target(): $this->assertSame( $contents, file_get_contents( $feature_file ) ); } + public function test_extraction_preserves_indentation_that_mixes_tabs_and_spaces(): void { + // The shared indentation is taken off a block as a prefix rather than as + // a number of characters, so a line indented with a tab where the rest of + // the block uses spaces keeps its tab instead of trading it for a space. + $this->create_feature_file( + 'example.feature', + "Feature: Example\n" + . " Scenario: A PHP block\n" + . " Given a test.php file:\n" + . " \"\"\"\n" + . " run_script( array( 'extract', 'features', 'extracted' ) ); + + $this->assertSame( 0, $result['exit_code'], $result['output'] ); + $this->assertSame( + "get_extracted_contents( 'batch0/example.feature_L4_E7.php' ) + ); + } + public function test_extraction_refuses_to_use_a_root_directory_as_target(): void { $contents = "Feature: Example\n" . " Scenario: A PHP block\n" diff --git a/utils/extract-feature-php.php b/utils/extract-feature-php.php index 3572ad331..c52fe2243 100644 --- a/utils/extract-feature-php.php +++ b/utils/extract-feature-php.php @@ -16,47 +16,6 @@ */ const EXTRACTED_FILE_PATTERN = '/^(.*\.feature)_L(\d+)_E(\d+)_(HASPHP|NOPHP)\.php$/'; -/** - * Determine the indentation that all lines holding code share. - * - * This is what extraction takes off a block and what syncing puts back, so it - * is determined as an actual prefix rather than as a number of characters: a - * block mixing tabs and spaces would otherwise come back with one swapped for - * the other. Blank lines carry no indentation of their own and are left out. - * - * @param string[] $lines Lines to compare. - * @return string|null Shared indentation, or null if no line holds code. - */ -function get_common_indent( array $lines ) { - $common = null; - - foreach ( $lines as $line ) { - if ( '' === trim( $line ) ) { - continue; - } - - preg_match( '/^[ \t]*/', $line, $matches ); - - if ( null === $common ) { - $common = $matches[0]; - continue; - } - - $length = min( strlen( $common ), strlen( $matches[0] ) ); - while ( $length > 0 && substr( $common, 0, $length ) !== substr( $matches[0], 0, $length ) ) { - --$length; - } - - $common = substr( $common, 0, $length ); - - if ( '' === $common ) { - break; - } - } - - return $common; -} - /** * Turn a PHP block into the source of a standalone PHP file. * diff --git a/utils/feature-php-blocks.php b/utils/feature-php-blocks.php index bdc5b857a..219598ab2 100644 --- a/utils/feature-php-blocks.php +++ b/utils/feature-php-blocks.php @@ -150,6 +150,47 @@ function find_feature_files( $source_dir ) { return $feature_files; } +/** + * Determine the indentation that all lines holding code share. + * + * This is what extraction takes off a block and what syncing puts back, so it + * is determined as an actual prefix rather than as a number of characters: a + * block mixing tabs and spaces would otherwise come back with one swapped for + * the other. Blank lines carry no indentation of their own and are left out. + * + * @param string[] $lines Lines to compare. + * @return string|null Shared indentation, or null if no line holds code. + */ +function get_common_indent( array $lines ) { + $common = null; + + foreach ( $lines as $line ) { + if ( '' === trim( $line ) ) { + continue; + } + + preg_match( '/^[ \t]*/', $line, $matches ); + + if ( null === $common ) { + $common = $matches[0]; + continue; + } + + $length = min( strlen( $common ), strlen( $matches[0] ) ); + while ( $length > 0 && substr( $common, 0, $length ) !== substr( $matches[0], 0, $length ) ) { + --$length; + } + + $common = substr( $common, 0, $length ); + + if ( '' === $common ) { + break; + } + } + + return $common; +} + /** * Collect the PHP blocks contained in a single feature file. * diff --git a/utils/phpstan-feature-files.php b/utils/phpstan-feature-files.php index 686f5a566..9228ac43e 100644 --- a/utils/phpstan-feature-files.php +++ b/utils/phpstan-feature-files.php @@ -40,16 +40,7 @@ * @return string Source of the standalone PHP file. */ function render_block( array $block ) { - $min_indent = PHP_INT_MAX; - foreach ( $block['lines'] as $code_line ) { - if ( '' !== trim( $code_line ) ) { - preg_match( '/^[ \t]*/', $code_line, $matches ); - $min_indent = min( $min_indent, strlen( $matches[0] ) ); - } - } - if ( PHP_INT_MAX === $min_indent ) { - $min_indent = 0; - } + $indent_length = strlen( (string) get_common_indent( $block['lines'] ) ); $out_lines = []; for ( $i = 0; $i <= $block['start']; $i++ ) { @@ -64,7 +55,7 @@ function render_block( array $block ) { continue; } - $code_line = substr( $code_line, $min_indent ); + $code_line = substr( $code_line, $indent_length ); if ( ! $tag_dropped ) { $tag_dropped = true; From 736b3c0e441422b5be550008333950f55119c961 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 26 Aug 2026 16:56:01 +0000 Subject: [PATCH 19/20] Share the scaffolding around the feature file script tests Both scripts are exercised the way they are used, by running them over a directory of feature files written into a temporary directory, so both test classes carried the same setup: creating and removing that directory, writing a feature file into it, running the script from within it, and reading back what it wrote. The scaffolding moves to `FeatureFilesTestCase`, which the two classes extend. Each names the script it is about and the flags to run it with, and the rest follows: the temporary directory is still named after the script, and the code style check still runs it without loading `php.ini` while the analysis needs one loaded for ext-tokenizer. The listing of what extraction wrote now leaves out files that are not PHP in both, rather than only in the one whose target directory holds a manifest. Extraction only ever writes `.php` files, so anything else in there came from somewhere else. --- tests/tests/FeatureFilesTestCase.php | 190 ++++++++++++++++++++++++ tests/tests/TestExtractFeaturePhp.php | 154 +------------------ tests/tests/TestPhpStanFeatureFiles.php | 153 +------------------ 3 files changed, 203 insertions(+), 294 deletions(-) create mode 100644 tests/tests/FeatureFilesTestCase.php diff --git a/tests/tests/FeatureFilesTestCase.php b/tests/tests/FeatureFilesTestCase.php new file mode 100644 index 000000000..f136b39e8 --- /dev/null +++ b/tests/tests/FeatureFilesTestCase.php @@ -0,0 +1,190 @@ +get_script_name(), '.php' ) . '-'; + + $this->temp_dir = Utils\get_temp_dir() . uniqid( $prefix, true ); + $this->features_dir = $this->temp_dir . '/features'; + $this->target_dir = $this->temp_dir . '/extracted'; + + mkdir( $this->temp_dir ); + mkdir( $this->features_dir ); + } + + protected function tear_down(): void { + if ( is_dir( $this->temp_dir ) ) { + $this->remove_dir( $this->temp_dir ); + } + + parent::tear_down(); + } + + /** + * Recursively removes a directory and its contents. + * + * @param string $dir The directory to remove. + */ + private function remove_dir( $dir ): void { + if ( ! is_dir( $dir ) ) { + return; + } + + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator( $dir, \FilesystemIterator::SKIP_DOTS ), + \RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ( $iterator as $file ) { + if ( $file->isDir() ) { + rmdir( $file->getPathname() ); + } else { + unlink( $file->getPathname() ); + } + } + + rmdir( $dir ); + } + + /** + * Runs the script under test from within the temporary directory. + * + * @param string[] $args Arguments to pass to the script. + * @return array{output: string, exit_code: int} Combined output and exit code of the script. + */ + protected function run_script( array $args ): array { + $script = dirname( dirname( __DIR__ ) ) . DIRECTORY_SEPARATOR . 'utils' . DIRECTORY_SEPARATOR . $this->get_script_name(); + + $command = escapeshellarg( PHP_BINARY ); + + foreach ( $this->get_php_flags() as $flag ) { + $command .= ' ' . $flag; + } + + $command .= ' ' . escapeshellarg( $script ); + + foreach ( $args as $arg ) { + $command .= ' ' . escapeshellarg( $arg ); + } + + $cd_command = Utils\is_windows() ? 'cd /d ' : 'cd '; + $command = $cd_command . escapeshellarg( $this->temp_dir ) . ' && ' . $command . ' 2>&1'; + + $output = array(); + $exit_code = 0; + + exec( $command, $output, $exit_code ); + + return array( + 'output' => implode( "\n", $output ), + 'exit_code' => $exit_code, + ); + } + + /** + * Creates a feature file in the features directory. + * + * @param string $relative_path Path relative to the features directory. + * @param string $contents Contents of the feature file. + * @return string Full path to the created file. + */ + protected function create_feature_file( $relative_path, $contents ): string { + $path = $this->features_dir . '/' . $relative_path; + + $directory = dirname( $path ); + if ( ! is_dir( $directory ) ) { + mkdir( $directory, 0777, true ); + } + + file_put_contents( $path, $contents ); + + return $path; + } + + /** + * Returns the paths of all extracted files, relative to the target directory. + * + * Extraction only ever writes `.php` files, so anything else in the target + * directory was put there by something other than the script under test. + * + * @return string[] Sorted list of relative file paths. + */ + protected function get_extracted_files(): array { + if ( ! is_dir( $this->target_dir ) ) { + return array(); + } + + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator( $this->target_dir, \FilesystemIterator::SKIP_DOTS ) + ); + + $files = array(); + + foreach ( $iterator as $file ) { + if ( $file->isFile() && 'php' === $file->getExtension() ) { + $files[] = str_replace( '\\', '/', substr( $file->getPathname(), strlen( $this->target_dir ) + 1 ) ); + } + } + + sort( $files ); + + return $files; + } + + /** + * Returns the contents of an extracted file. + * + * @param string $relative_path Path relative to the target directory. + * @return string Contents of the file. + */ + protected function get_extracted_contents( $relative_path ): string { + $contents = file_get_contents( $this->target_dir . '/' . $relative_path ); + + return false === $contents ? '' : $contents; + } +} diff --git a/tests/tests/TestExtractFeaturePhp.php b/tests/tests/TestExtractFeaturePhp.php index d964c0961..2e37a31b2 100644 --- a/tests/tests/TestExtractFeaturePhp.php +++ b/tests/tests/TestExtractFeaturePhp.php @@ -2,158 +2,18 @@ namespace WP_CLI\Tests\Tests; -use WP_CLI\Tests\TestCase; -use WP_CLI\Utils; +class TestExtractFeaturePhp extends FeatureFilesTestCase { -class TestExtractFeaturePhp extends TestCase { - - /** - * @var string - */ - public $temp_dir; - - /** - * @var string - */ - public $features_dir; - - /** - * @var string - */ - public $target_dir; - - protected function set_up(): void { - parent::set_up(); - - $this->temp_dir = Utils\get_temp_dir() . uniqid( 'wp-cli-test-extract-feature-php-', true ); - $this->features_dir = $this->temp_dir . '/features'; - $this->target_dir = $this->temp_dir . '/extracted'; - - mkdir( $this->temp_dir ); - mkdir( $this->features_dir ); - } - - protected function tear_down(): void { - if ( is_dir( $this->temp_dir ) ) { - $this->remove_dir( $this->temp_dir ); - } - - parent::tear_down(); + protected function get_script_name(): string { + return 'extract-feature-php.php'; } /** - * Recursively removes a directory and its contents. - * - * @param string $dir The directory to remove. + * The script needs nothing beyond the PHP core, so `php.ini` is left out of + * the run to keep the environment it is exercised in predictable. */ - private function remove_dir( $dir ): void { - if ( ! is_dir( $dir ) ) { - return; - } - - $iterator = new \RecursiveIteratorIterator( - new \RecursiveDirectoryIterator( $dir, \FilesystemIterator::SKIP_DOTS ), - \RecursiveIteratorIterator::CHILD_FIRST - ); - - foreach ( $iterator as $file ) { - if ( $file->isDir() ) { - rmdir( $file->getPathname() ); - } else { - unlink( $file->getPathname() ); - } - } - - rmdir( $dir ); - } - - /** - * Runs the extract-feature-php.php script from within the temporary directory. - * - * @param string[] $args Arguments to pass to the script. - * @return array{output: string, exit_code: int} Combined output and exit code of the script. - */ - private function run_script( array $args ): array { - $script = dirname( dirname( __DIR__ ) ) . DIRECTORY_SEPARATOR . 'utils' . DIRECTORY_SEPARATOR . 'extract-feature-php.php'; - - // Use the `-n` flag to disable loading of `php.ini` and ensure a clean environment. - $command = escapeshellarg( PHP_BINARY ) . ' -n ' . escapeshellarg( $script ); - - foreach ( $args as $arg ) { - $command .= ' ' . escapeshellarg( $arg ); - } - - $cd_command = Utils\is_windows() ? 'cd /d ' : 'cd '; - $command = $cd_command . escapeshellarg( $this->temp_dir ) . ' && ' . $command . ' 2>&1'; - - $output = array(); - $exit_code = 0; - - exec( $command, $output, $exit_code ); - - return array( - 'output' => implode( "\n", $output ), - 'exit_code' => $exit_code, - ); - } - - /** - * Creates a feature file in the features directory. - * - * @param string $relative_path Path relative to the features directory. - * @param string $contents Contents of the feature file. - * @return string Full path to the created file. - */ - private function create_feature_file( $relative_path, $contents ): string { - $path = $this->features_dir . '/' . $relative_path; - - $directory = dirname( $path ); - if ( ! is_dir( $directory ) ) { - mkdir( $directory, 0777, true ); - } - - file_put_contents( $path, $contents ); - - return $path; - } - - /** - * Returns the paths of all extracted files, relative to the target directory. - * - * @return string[] Sorted list of relative file paths. - */ - private function get_extracted_files(): array { - if ( ! is_dir( $this->target_dir ) ) { - return array(); - } - - $iterator = new \RecursiveIteratorIterator( - new \RecursiveDirectoryIterator( $this->target_dir, \FilesystemIterator::SKIP_DOTS ) - ); - - $files = array(); - - foreach ( $iterator as $file ) { - if ( $file->isFile() ) { - $files[] = str_replace( '\\', '/', substr( $file->getPathname(), strlen( $this->target_dir ) + 1 ) ); - } - } - - sort( $files ); - - return $files; - } - - /** - * Returns the contents of an extracted file. - * - * @param string $relative_path Path relative to the target directory. - * @return string Contents of the file. - */ - private function get_extracted_contents( $relative_path ): string { - $contents = file_get_contents( $this->target_dir . '/' . $relative_path ); - - return false === $contents ? '' : $contents; + protected function get_php_flags(): array { + return array( '-n' ); } public function test_extracts_block_with_opening_tag(): void { diff --git a/tests/tests/TestPhpStanFeatureFiles.php b/tests/tests/TestPhpStanFeatureFiles.php index 55c7c21ad..90ddc9b17 100644 --- a/tests/tests/TestPhpStanFeatureFiles.php +++ b/tests/tests/TestPhpStanFeatureFiles.php @@ -2,158 +2,17 @@ namespace WP_CLI\Tests\Tests; -use WP_CLI\Tests\TestCase; -use WP_CLI\Utils; +class TestPhpStanFeatureFiles extends FeatureFilesTestCase { -class TestPhpStanFeatureFiles extends TestCase { - - /** - * @var string - */ - public $temp_dir; - - /** - * @var string - */ - public $features_dir; - - /** - * @var string - */ - public $target_dir; - - protected function set_up(): void { - parent::set_up(); - - $this->temp_dir = Utils\get_temp_dir() . uniqid( 'wp-cli-test-phpstan-feature-files-', true ); - $this->features_dir = $this->temp_dir . '/features'; - $this->target_dir = $this->temp_dir . '/extracted'; - - mkdir( $this->temp_dir ); - mkdir( $this->features_dir ); - } - - protected function tear_down(): void { - if ( is_dir( $this->temp_dir ) ) { - $this->remove_dir( $this->temp_dir ); - } - - parent::tear_down(); - } - - /** - * Recursively removes a directory and its contents. - * - * @param string $dir The directory to remove. - */ - private function remove_dir( $dir ): void { - if ( ! is_dir( $dir ) ) { - return; - } - - $iterator = new \RecursiveIteratorIterator( - new \RecursiveDirectoryIterator( $dir, \FilesystemIterator::SKIP_DOTS ), - \RecursiveIteratorIterator::CHILD_FIRST - ); - - foreach ( $iterator as $file ) { - if ( $file->isDir() ) { - rmdir( $file->getPathname() ); - } else { - unlink( $file->getPathname() ); - } - } - - rmdir( $dir ); + protected function get_script_name(): string { + return 'phpstan-feature-files.php'; } /** - * Runs the phpstan-feature-files.php script from within the temporary directory. - * - * @param string[] $args Arguments to pass to the script. - * @return array{output: string, exit_code: int} Combined output and exit code of the script. + * `php.ini` is loaded as usual here, as the script needs ext-tokenizer. */ - private function run_script( array $args ): array { - $script = dirname( dirname( __DIR__ ) ) . DIRECTORY_SEPARATOR . 'utils' . DIRECTORY_SEPARATOR . 'phpstan-feature-files.php'; - - // `php.ini` is loaded as usual here, as the script needs ext-tokenizer. - $command = escapeshellarg( PHP_BINARY ) . ' ' . escapeshellarg( $script ); - - foreach ( $args as $arg ) { - $command .= ' ' . escapeshellarg( $arg ); - } - - $cd_command = Utils\is_windows() ? 'cd /d ' : 'cd '; - $command = $cd_command . escapeshellarg( $this->temp_dir ) . ' && ' . $command . ' 2>&1'; - - $output = array(); - $exit_code = 0; - - exec( $command, $output, $exit_code ); - - return array( - 'output' => implode( "\n", $output ), - 'exit_code' => $exit_code, - ); - } - - /** - * Creates a feature file in the features directory. - * - * @param string $relative_path Path relative to the features directory. - * @param string $contents Contents of the feature file. - * @return string Full path to the created file. - */ - private function create_feature_file( $relative_path, $contents ): string { - $path = $this->features_dir . '/' . $relative_path; - - $directory = dirname( $path ); - if ( ! is_dir( $directory ) ) { - mkdir( $directory, 0777, true ); - } - - file_put_contents( $path, $contents ); - - return $path; - } - - /** - * Returns the paths of all extracted files, relative to the target directory. - * - * @return string[] Sorted list of relative file paths. - */ - private function get_extracted_files(): array { - if ( ! is_dir( $this->target_dir ) ) { - return array(); - } - - $iterator = new \RecursiveIteratorIterator( - new \RecursiveDirectoryIterator( $this->target_dir, \FilesystemIterator::SKIP_DOTS ) - ); - - $files = array(); - - foreach ( $iterator as $file ) { - if ( $file->isFile() && 'php' === $file->getExtension() ) { - $files[] = str_replace( '\\', '/', substr( $file->getPathname(), strlen( $this->target_dir ) + 1 ) ); - } - } - - sort( $files ); - - return $files; - } - - /** - * Returns the contents of an extracted file. - * - * @param string $relative_path Path relative to the target directory. - * @return string Contents of the file. - */ - private function get_extracted_contents( $relative_path ): string { - $contents = file_get_contents( $this->target_dir . '/' . $relative_path ); - - return false === $contents ? '' : $contents; + protected function get_php_flags(): array { + return array(); } /** From 77737d8bc6a2dbdfb683961c5c16270e05cdc195 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Wed, 26 Aug 2026 16:56:29 +0000 Subject: [PATCH 20/20] Annotate the return types of the test scaffolding hooks --- tests/tests/TestExtractFeaturePhp.php | 5 +++++ tests/tests/TestPhpStanFeatureFiles.php | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/tests/TestExtractFeaturePhp.php b/tests/tests/TestExtractFeaturePhp.php index 2e37a31b2..908d01ed1 100644 --- a/tests/tests/TestExtractFeaturePhp.php +++ b/tests/tests/TestExtractFeaturePhp.php @@ -4,6 +4,9 @@ class TestExtractFeaturePhp extends FeatureFilesTestCase { + /** + * @return string Name of the script. + */ protected function get_script_name(): string { return 'extract-feature-php.php'; } @@ -11,6 +14,8 @@ protected function get_script_name(): string { /** * The script needs nothing beyond the PHP core, so `php.ini` is left out of * the run to keep the environment it is exercised in predictable. + * + * @return string[] Flags to pass to the PHP binary. */ protected function get_php_flags(): array { return array( '-n' ); diff --git a/tests/tests/TestPhpStanFeatureFiles.php b/tests/tests/TestPhpStanFeatureFiles.php index 90ddc9b17..b49255eed 100644 --- a/tests/tests/TestPhpStanFeatureFiles.php +++ b/tests/tests/TestPhpStanFeatureFiles.php @@ -4,12 +4,17 @@ class TestPhpStanFeatureFiles extends FeatureFilesTestCase { + /** + * @return string Name of the script. + */ protected function get_script_name(): string { return 'phpstan-feature-files.php'; } /** * `php.ini` is loaded as usual here, as the script needs ext-tokenizer. + * + * @return string[] Flags to pass to the PHP binary. */ protected function get_php_flags(): array { return array();