From 6de99d4ec3ce6687e6c486a0b5977332db745186 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Thu, 23 Jul 2026 14:20:39 +0200 Subject: [PATCH 01/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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; + } } }