Add PHPStan analysis for PHP blocks in feature files - #354
Open
swissspidy wants to merge 3 commits into
Open
Conversation
Feature files embed PHP snippets in docstrings that no static analysis tool
currently looks at. This extracts those snippets into standalone PHP files
padded so that their line numbers match the feature file, runs PHPStan over
them, and reports the findings against the feature file they came from.
PHPStan needs a few things that a code style check does not:
* The opening tag has to come first. Padding in front of it counts as inline
HTML, which makes a `declare()` or `namespace` statement in a block a fatal
error, so the tag a block brings along itself is dropped instead.
* Blocks that are not standalone PHP have to be left out, since a single file
that fails to parse makes PHPStan abort the whole run. Feature files hold
deliberately broken snippets as well as Behat placeholders such as
`{POST_ID}` that are substituted before the snippet is written to disk.
* Blocks declaring the same class or function are analysed separately from
each other, so that a name is not resolved to another block's declaration.
The analysis is opt-in through a `phpstan-feature-files.neon[.dist]` file in
the package root, layered on top of the defaults in `phpstan/feature-files.neon`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M4oxq4G2FxhwJfHd3w6VLw
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe change adds PHPStan analysis for PHP blocks embedded in Behat feature files. It adds extraction and reporting utilities, integrates them into the Composer test workflow, adds configuration, and documents and tests the workflow. ChangesFeature-file PHPStan analysis
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Composer
participant run_phpstan_tests
participant phpstan_feature_files
participant PHPStan
participant FeatureFiles
Composer->>run_phpstan_tests: run aggregate test command
run_phpstan_tests->>phpstan_feature_files: extract PHP blocks
phpstan_feature_files->>FeatureFiles: read feature files
phpstan_feature_files-->>run_phpstan_tests: return batches and manifest
run_phpstan_tests->>PHPStan: analyze PHP batches
PHPStan-->>run_phpstan_tests: return JSON diagnostics
run_phpstan_tests->>phpstan_feature_files: report diagnostics
phpstan_feature_files-->>Composer: return mapped findings and status
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M4oxq4G2FxhwJfHd3w6VLw
Member
Author
|
@coderabbitai review |
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Fixes the defects the review surfaced, all of which are reproducible: * Map the errors PHPStan reports through a canonical path instead of a string prefix. The path it reports is not necessarily spelled the way the extraction wrote it -- macOS resolves `/var` to `/private/var` and Windows has a short and a long form of a directory name -- which made every finding come out as `Unexpected file` and failed the unit tests on both platforms. * Resolve the symlink Composer installs in the vendor binary directory before deriving the root of this package from it. Without that the analysis was silently skipped for every package using the framework, since it looked for the extraction script under `vendor/utils` and never found it. * Keep a package whose feature files hold no PHP block at all from failing. No batch means no results, which left the glob unexpanded and made the report treat the pattern itself as a missing file. * Guard the fields of the PHPStan results before reading them. * Only pass PHPStan's own output through when it produced no results, so its notes about interpreting errors do not bury the report. * Skip the dot entries while looking for feature files. The rule that a docstring opening with `<?php` counts as PHP whichever step precedes it is kept: it covers PHP files that are not named `*.php`, such as the `.maintenance` file. The documentation now says so instead of claiming that only a `.php` file step is considered. Also corrects the documented way to ignore an error, which matched nothing: the pattern applies to the extracted file, whose name carries the line range and is relative to the `features` directory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M4oxq4G2FxhwJfHd3w6VLw
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feature files embed PHP snippets in docstrings that no static analysis tool
currently looks at. This extracts those snippets into standalone PHP files
padded so that their line numbers match the feature file, runs PHPStan over
them, and reports the findings against the feature file they came from.
PHPStan needs a few things that a code style check does not:
HTML, which makes a
declare()ornamespacestatement in a block a fatalerror, so the tag a block brings along itself is dropped instead.
that fails to parse makes PHPStan abort the whole run. Feature files hold
deliberately broken snippets as well as Behat placeholders such as
{POST_ID}that are substituted before the snippet is written to disk.each other, so that a name is not resolved to another block's declaration.
The analysis is opt-in through a
phpstan-feature-files.neon[.dist]file inthe package root, layered on top of the defaults in
phpstan/feature-files.neon.Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01M4oxq4G2FxhwJfHd3w6VLw
Summary by CodeRabbit
New Features
Documentation
Chores