Skip to content

Add PHPCS linting for PHP blocks in feature files - #340

Open
swissspidy wants to merge 13 commits into
mainfrom
try/phpcs-in-feature-files
Open

Add PHPCS linting for PHP blocks in feature files#340
swissspidy wants to merge 13 commits into
mainfrom
try/phpcs-in-feature-files

Conversation

@swissspidy

@swissspidy swissspidy commented Jul 23, 2026

Copy link
Copy Markdown
Member

Not sure yet if really worth it, will need some testing.

Summary by CodeRabbit

  • New Features

    • Added validation and automatic formatting for PHP code embedded in feature files.
    • Added tooling to extract embedded PHP for checks and synchronize formatting fixes back into feature files.
  • Bug Fixes

    • Improved temporary-file cleanup, including when checks are interrupted.
    • Preserved and reported code-quality check failures while allowing applicable checks to complete.
  • Style

    • Standardized formatting in feature-file test scenarios.
  • Tests

    • Added comprehensive coverage for PHP extraction, validation, formatting, and synchronization.

@swissspidy swissspidy added the scope:testing Related to testing label Jul 23, 2026
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d2b71908-4123-49a3-be0f-0f4523a8ce20

📥 Commits

Reviewing files that changed from the base of the PR and between 2c962f3 and 9c79335.

📒 Files selected for processing (2)
  • tests/tests/TestExtractFeaturePhp.php
  • utils/extract-feature-php.php

📝 Walkthrough

Walkthrough

The change adds PHP extraction and synchronization for .feature files. PHPCS and PHPCBF now validate or format extracted blocks, preserve accumulated failures, and clean temporary files. PHPUnit tests cover extraction, updates, validation, and error handling.

Changes

Feature PHP checks

Layer / File(s) Summary
Feature PHP extraction and synchronization
utils/extract-feature-php.php
Adds a CLI utility that validates targets, extracts PHP blocks into padded temporary files, and synchronizes processed content back into .feature files.
Extraction utility validation
tests/tests/TestExtractFeaturePhp.php
Adds tests for extraction, target validation, generated-file cleanup, indentation, PHP tags, padding, synchronization, and failure cases.
PHPCS and PHPCBF script integration
bin/run-phpcbf-cleanup, bin/run-phpcs-tests, features/*.feature
Accumulates command statuses, conditionally processes PHP blocks in feature files, cleans temporary files, and applies minor syntax-formatting changes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Developer
  participant run_phpcs_tests
  participant extract_feature_php
  participant PHPCS
  participant FeatureFiles
  Developer->>run_phpcs_tests: run checks
  run_phpcs_tests->>extract_feature_php: extract PHP blocks
  extract_feature_php->>FeatureFiles: read .feature files
  extract_feature_php-->>run_phpcs_tests: create temporary PHP files
  run_phpcs_tests->>PHPCS: check temporary PHP files
  PHPCS-->>run_phpcs_tests: return check status
  run_phpcs_tests-->>Developer: exit with accumulated status
Loading

Suggested reviewers: brianhenryie

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding PHPCS linting for PHP blocks in feature files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch try/phpcs-in-feature-files

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PHPStan (2.2.7)

PHPStan was skipped because the config uses disallowed bootstrapFiles, bootstrapFile, or includes directives.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@swissspidy
swissspidy marked this pull request as ready for review August 4, 2026 08:42
@swissspidy
swissspidy requested a review from a team as a code owner August 4, 2026 08:42
Copilot AI lite review requested due to automatic review settings August 4, 2026 08:42

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

Fixes for the PHP block extraction and synchronization:

* Place an added `<?php` tag on the docstring delimiter line, so that it
  is no longer overwritten by the first line of code and blocks without an
  opening tag are actually linted as PHP.
* Only treat a docstring as PHP when it follows a step creating a PHP file
  or starts with `<?php`. Expectations such as `Then the wp-config.php
  file should contain:` are not necessarily valid PHP and must not be
  reformatted.
* Determine the indentation to restore from the first line holding code,
  as a leading empty line would otherwise be used as indentation.
* Remove only the padding added during extraction instead of all leading
  and trailing empty lines, preserving intentional ones.
* Keep empty lines leading up to an opening tag, so that reported line
  numbers keep matching.
* Only remove files from the target directory that a previous extraction
  created, and refuse target directories that hold the feature files
  themselves.
* Only treat the first argument as an action when it is one, so that
  passing two directories does not swap source and target.
* Report failures on STDERR and through the exit code.
* Skip a block instead of writing it back when the extracted file no
  longer lines up with the feature file.

Fixes for the scripts running the checks:

* Propagate failures of the extraction, PHPCS, PHPCBF and synchronization
  steps, and skip synchronization when extraction failed.
* Preserve the PHPCS exit status by writing the report to a file instead
  of piping it through `sed`.
* Keep nested directories when normalizing reported feature file paths.

Adds unit tests covering extraction and synchronization.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KVnFXuhCGs4NT2A7FmDo4e
coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@swissspidy

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@swissspidy swissspidy added this to the 5.2.3 milestone Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope:testing Related to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants