-
Notifications
You must be signed in to change notification settings - Fork 1
Extract RuleTestCase from shipmonk/phpstan-rules #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Extract RuleTestCase class from tests/ to src/ with ShipMonk\PHPStanDev namespace - Create composer package shipmonk/phpstan-dev for PHPStan rule testing utilities - Copy and adapt QA tools configuration (PHPCS, PHPStan, EditorConfig, etc.) - Add comprehensive README with usage examples and development workflow - Set up GitHub Actions CI workflows - Configure git attributes for proper package exports 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
d6c5f35
to
53b81aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Extracts the RuleTestCase
class into its own package, sets up configuration and tooling for PHPStan rule testing, and adds documentation and CI workflows.
- Introduce
RuleTestCase
insrc/RuleTestCase.php
with error parsing and autofix support - Add project configuration files (
phpunit.xml.dist
,phpstan.neon.dist
,.editorconfig
, etc.) and CI workflows - Provide
README.md
with usage examples and development workflow
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/RuleTestCase.php | Core testing framework for PHPStan rules with error handling |
phpunit.xml.dist | PHPUnit configuration |
phpstan.neon.dist | PHPStan configuration |
composer.json | Defines new package metadata, dependencies, and scripts |
composer-dependency-analyser.php | Entry point for dependency analysis |
README.md | Usage instructions, examples, and development workflow |
.github/workflows/lint.yml | XML linter CI job |
.github/workflows/checks.yml | Combined checks CI job |
.gitattributes | Export-ignore settings for package |
.editorconfig | Editor configuration |
cache/.gitkeep | Placeholder to track cache directory |
Comments suppressed due to low confidence (1)
.github/workflows/lint.yml:25
- The workflow references
phpcs.xml.dist
, but no such file was added in this PR. Either include the file or update the workflow to point to the correct CodeSniffer config.
xml-file: phpcs.xml.dist
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Replace comma-separated array values with proper XML element format - Remove deprecated Generic.Functions.CallTimePassByReference sniff - Replace deprecated Squiz.WhiteSpace.LanguageConstructSpacing with Generic.WhiteSpace.LanguageConstructSpacing - Update ForbiddenFunctions, ForbiddenComments, ForbiddenAnnotations, and TypeHint rules 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Make the test case more generic by removing the hardcoded requirement that error identifiers must start with 'shipmonk.'. This allows the utility to be used by any project with their own identifier naming. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Add autoload-dev section to composer.json to properly map test namespace ShipMonkTests\PHPStanDev\ to tests/ directory following PSR-4 conventions. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Create RuleTestCaseTest to validate RuleTestCase functionality - Add DisallowDivisionByLiteralZeroRule as example PHPStan rule - Implement proper PSR-4 namespace structure for tests - Organize test data in Data directory following convention - Include test data file with expected error comments The test suite demonstrates how to use RuleTestCase and validates that the enhanced testing framework works correctly with error detection and validation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Key improvements: - Update regex to properly parse multiple error comments using non-greedy matching - Simplify expected errors parsing by removing complex sorting logic - Add comprehensive test cases for multiple errors per line scenarios - Add dedicated test method demonstrating 2, 3, and 4 errors on single lines - Add test cases for mixed valid/invalid operations on same line The enhanced RuleTestCase now correctly handles the common scenario where multiple PHPStan rule violations occur on the same line of code, making it much more practical for testing complex rules. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Update actions/checkout from v3 to v4 in lint workflow for better performance and security. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Include tests directory in PHPStan analysis paths - Exclude test data files (tests/Rule/Data/*) from analysis as they contain intentionally problematic code - Remove empty ignoreErrors section 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Test against all supported PHP versions (7.4, 8.0, 8.1, 8.2, 8.3, 8.4) to ensure compatibility across the entire supported range. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add detailed testing patterns and examples - Document autofix mode usage and warnings - Include advanced testing scenarios (multiple files, PHP versions, custom config) - Add error comment validation examples - Improve directory structure and workflow guidance 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Libraries should not commit composer.lock to allow consuming applications to resolve dependencies flexibly. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
src/RuleTestCase.php
Outdated
abstract class RuleTestCase extends OriginalRuleTestCase | ||
{ | ||
|
||
protected function analyseFile(string $file, bool $autofix = false): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets support multiple files as in DCD so that we can utilize it even there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why is multiple files better then multiple calls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When single analysis of multiple files is needed. Crucial e.g. for trait tests, where definition filepaths get tricky. See usages in DCD, it was addded there for this exact reason.
], | ||
"require": { | ||
"php": "^7.4 || ^8.0", | ||
"phpstan/phpstan": "^2.1.8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it works just fine even on much lower versions. Maybe even 2.*
- Rename analyseFile() to analyzeFiles() - Change signature to accept array of files only - Process errors per file with proper filtering - Update all references in tests and documentation - Add better error messages showing which file has mismatches 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
54a7fea
to
3276bae
Compare
🤖 Generated with Claude Code