Add comprehensive test coverage across all phpstan rules#55
Conversation
Agent-Logs-Url: https://github.com/voku/phpstan-rules/sessions/13317758-8972-46dc-9ea8-ee2aaa0855eb Co-authored-by: voku <264695+voku@users.noreply.github.com>
Coverage Report for CI Build 24198084383Coverage increased (+3.5%) to 94.118%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request expands the test suite for several PHPStan rules, including DisallowedCallMethodOnNullRule, ElseIfConditionBasicRule, ExtendedBinaryOpRule, and various boolean condition rules. It also introduces a new WrongCastRuleTest along with its fixtures. The review feedback points out that new error scenarios added to the match and switch fixtures lack corresponding assertions in their respective test files, which will cause test failures. Additionally, a comment in the ExtendedBinaryOpRule fixture incorrectly identifies a null-coalesce operation as an error when the rule actually ignores it.
| $obj = new \stdClass(); | ||
| $flag = rand(0, 1) ? true : false; | ||
| $result = match (true) { | ||
| $obj && $flag => 'Has object and flag', |
There was a problem hiding this comment.
| $obj = new \stdClass(); | ||
| $flag = rand(0, 1) ? true : false; | ||
| switch (true) { | ||
| case $obj && $flag: |
There was a problem hiding this comment.
Similar to the match fixtures, this file adds several new error scenarios (lines 319, 352, 360), but the test file tests/IfConditionSwitchCaseRuleTest.php is missing from the pull request. These new errors must be asserted in the test case to ensure the build passes and the rule is correctly verified.
| } | ||
| } | ||
|
|
||
| // Error: null-coalesce that is always non-null because the left side is a non-empty string |
There was a problem hiding this comment.
The comment indicates an error is expected for the null-coalesce operator, but ExtendedBinaryOpRule explicitly ignores Coalesce nodes (see ExtendedBinaryOpRule.php line 74). Additionally, the corresponding test tests/ExtendedBinaryOpRuleTest.php does not assert an error for this line. This comment should be corrected to avoid confusion.
// OK: null-coalesce is not handled by this rule|
@copilot Review the following cleaned github pull request excerpt. Add comprehensive test coverage across all phpstan rules#55 WrongCastRule had zero test coverage, and all other rule tests had minimal fixture coverage with few edge cases or OK-path assertions. Three test methods covering the full behavior matrix: Four PSR-4-named fixture files required because $scope->getClassReflection() resolves via the autoloader at rule runtime, not PHPStan's AST — classes must be in correctly named files to be found. Uncovered Changes No uncovered changes found. No coverage regressions found. /gemini review This pull request expands the test suite for several PHPStan rules, including DisallowedCallMethodOnNullRule, ElseIfConditionBasicRule, ExtendedBinaryOpRule, and various boolean condition rules. It also introduces a new WrongCastRuleTest along with its fixtures. The review feedback points out that new error scenarios added to the match and switch fixtures lack corresponding assertions in their respective test files, which will cause test failures. Additionally, a comment in the ExtendedBinaryOpRule fixture incorrectly identifies a null-coalesce operation as an error when the rule actually ignores it. |
|
Thank you for requesting a review. This pull request significantly improves the test coverage for several PHPStan rules, which is a valuable contribution. The new However, I've identified a few areas where the test assertions do not fully align with the error conditions described in the fixture files. These discrepancies could lead to confusion or mask potential issues in the rules themselves. Please consider the following:
Addressing these points will ensure the tests accurately reflect the intended behavior of the rules and prevent future regressions. |
…t errors) Agent-Logs-Url: https://github.com/voku/phpstan-rules/sessions/84b979fa-0855-4da3-afb8-b66a1c109536 Co-authored-by: voku <264695+voku@users.noreply.github.com>
... All three issues from the review were already addressed in commit 53026b9:
All 18 tests continue to pass. |
ExtendedBinaryOpRuleFixtures.php: change "Error: null-coalesce..." → "OK: null-coalesce is not handled by this rule (ExtendedBinaryOpRule skips Coalesce nodes)"IfConditionMatchRuleFixtures.php: change "Error: use a method..." → accurate OK comment explaining the rule checks match subject type vs arm condition type, not inner expressionsIfConditionSwitchCaseRuleFixtures.php: change three "Error:" comments → accurate OK comments explaining why float/null don't trigger and why stdClass inside&&is not checked