-
Notifications
You must be signed in to change notification settings - Fork 35
Add PHP 8.5 support #300
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
Add PHP 8.5 support #300
Conversation
Update continuous integration workflow to test against PHP 8.5 by: - Adding PHP 8.4 to the old_stable versions list - Setting current_stable to 8.5 This ensures the package is tested and compatible with PHP 8.5. Note that composer.json already supports PHP 8.5 via the ^8.0 constraint. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR enhances the GitHub Actions CI pipeline to test against PHP 8.5 by moving 8.4 into the legacy versions list and promoting 8.5 to the current stable target, with no changes required to package constraints. Flow diagram for CI PHP version update processflowchart TD
Start([Start CI Workflow]) --> CheckOldStable["Check old_stable versions"]
CheckOldStable --> UpdateOldStable["Add 8.4 to old_stable"]
UpdateOldStable --> SetCurrentStable["Set current_stable to 8.5"]
SetCurrentStable --> RunTests["Run tests with updated PHP versions"]
RunTests --> End([End])
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughUpdated CI matrix (added "8.4" to old_stable, bumped current_stable to "8.5"), added two unit tests in Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test
participant S as SetterMethod
participant V as VisitorImplementation
rect rgb(240,248,255)
Note over T,S: Setup SetterMethod and Visitor (visitor may throw Unbound)
end
T->>S: accept(visitor)
S->>V: visitSetterMethod(setter)
alt Visitor throws Unbound
V-->>S: throws Unbound
alt Setter is optional
S-->>T: return null
else
S-->>T: propagate Unbound (exception)
end
else Visitor returns value
V-->>S: value
S-->>T: apply value / return result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 2.x #300 +/- ##
=============================================
+ Coverage 99.49% 100.00% +0.50%
Complexity 365 365
=============================================
Files 50 50
Lines 996 995 -1
=============================================
+ Hits 991 995 +4
+ Misses 5 0 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5001614 to
0aa8dce
Compare
This commit adds two new test cases to improve code coverage for the SetterMethod::accept() method's exception handling logic: 1. testAcceptWithUnboundException: Tests that Unbound exceptions are properly re-thrown when isOptional is false 2. testAcceptWithUnboundExceptionOptional: Tests that Unbound exceptions are silently handled (not re-thrown) when isOptional is true These tests cover the previously untested catch block in SetterMethod::accept(), improving the method's line coverage from ~75% to 93.75%. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit updates the PHPCS configuration to use the recommended Generic.WhiteSpace.LanguageConstructSpacing sniff instead of the deprecated Squiz.WhiteSpace.LanguageConstructSpacing. Changes: - Exclude the deprecated Squiz.WhiteSpace.LanguageConstructSpacing from Doctrine rules - Add Generic.WhiteSpace.LanguageConstructSpacing to Additional Rules This eliminates the deprecation warning: "The bearcs standard uses 1 deprecated sniff - Squiz.WhiteSpace.LanguageConstructSpacing This sniff has been deprecated since v3.3.0 and will be removed in v4.0.0" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/di/SetterMethodTest.php (1)
101-155: LGTM with suggestion: Consider extracting the duplicated visitor stub.The test correctly validates that
SetterMethod::accept()returnsnullwhen the setter is optional and anUnboundexception is thrown. However, the anonymous visitor implementation (lines 107-151) is nearly identical to the one in the previous test (lines 52-96).Consider extracting the visitor stub to reduce duplication:
private function createUnboundThrowingVisitor(): VisitorInterface { return new class implements VisitorInterface { public function visitSetterMethod(string $method, Arguments $arguments): void { throw new Unbound(FakeTyreInterface::class); } // ... other stub methods }; }Then both tests can reuse:
$visitor = $this->createUnboundThrowingVisitor();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/di/SetterMethodTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/di/SetterMethodTest.php (5)
src/di/Exception/Unbound.php (1)
Unbound(16-62)tests/di/Fake/FakeCar.php (2)
FakeCar(11-97)postConstruct(89-96)src/di/SetterMethod.php (2)
SetterMethod(15-82)setOptional(62-65)src/di/Arguments.php (1)
Arguments(14-78)src/di/Argument.php (1)
isDefaultAvailable(81-84)
🪛 PHPMD (2.15.0)
tests/di/SetterMethodTest.php
59-59: Avoid unused parameters such as '$newInstance'. (undefined)
(UnusedFormalParameter)
59-59: Avoid unused parameters such as '$postConstruct'. (undefined)
(UnusedFormalParameter)
63-63: Avoid unused parameters such as '$isSingleton'. (undefined)
(UnusedFormalParameter)
63-63: Avoid unused parameters such as '$dependency'. (undefined)
(UnusedFormalParameter)
63-63: Avoid unused parameters such as '$context'. (undefined)
(UnusedFormalParameter)
69-69: Avoid unused parameters such as '$value'. (undefined)
(UnusedFormalParameter)
74-74: Avoid unused parameters such as '$aopBind'. (undefined)
(UnusedFormalParameter)
78-78: Avoid unused parameters such as '$class'. (undefined)
(UnusedFormalParameter)
78-78: Avoid unused parameters such as '$bind'. (undefined)
(UnusedFormalParameter)
83-83: Avoid unused parameters such as '$setterMethods'. (undefined)
(UnusedFormalParameter)
88-88: Avoid unused parameters such as '$arguments'. (undefined)
(UnusedFormalParameter)
93-93: Avoid unused parameters such as '$index'. (undefined)
(UnusedFormalParameter)
93-93: Avoid unused parameters such as '$isDefaultAvailable'. (undefined)
(UnusedFormalParameter)
93-93: Avoid unused parameters such as '$defaultValue'. (undefined)
(UnusedFormalParameter)
93-93: Avoid unused parameters such as '$parameter'. (undefined)
(UnusedFormalParameter)
114-114: Avoid unused parameters such as '$newInstance'. (undefined)
(UnusedFormalParameter)
114-114: Avoid unused parameters such as '$postConstruct'. (undefined)
(UnusedFormalParameter)
118-118: Avoid unused parameters such as '$isSingleton'. (undefined)
(UnusedFormalParameter)
118-118: Avoid unused parameters such as '$dependency'. (undefined)
(UnusedFormalParameter)
118-118: Avoid unused parameters such as '$context'. (undefined)
(UnusedFormalParameter)
124-124: Avoid unused parameters such as '$value'. (undefined)
(UnusedFormalParameter)
129-129: Avoid unused parameters such as '$aopBind'. (undefined)
(UnusedFormalParameter)
133-133: Avoid unused parameters such as '$class'. (undefined)
(UnusedFormalParameter)
133-133: Avoid unused parameters such as '$bind'. (undefined)
(UnusedFormalParameter)
138-138: Avoid unused parameters such as '$setterMethods'. (undefined)
(UnusedFormalParameter)
143-143: Avoid unused parameters such as '$arguments'. (undefined)
(UnusedFormalParameter)
148-148: Avoid unused parameters such as '$index'. (undefined)
(UnusedFormalParameter)
148-148: Avoid unused parameters such as '$isDefaultAvailable'. (undefined)
(UnusedFormalParameter)
148-148: Avoid unused parameters such as '$defaultValue'. (undefined)
(UnusedFormalParameter)
148-148: Avoid unused parameters such as '$parameter'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (2)
tests/di/SetterMethodTest.php (2)
10-10: LGTM: Import added for visitor stub type hints.The
ReflectionParameterimport is correctly added to support the type hints in the anonymous visitor implementations below.
46-99: LGTM: Test correctly verifies non-optional setter behavior.The test properly validates that
SetterMethod::accept()re-throws anUnboundexception when the setter is not marked as optional, matching the implementation insrc/di/SetterMethod.phplines 54-68.
Changed @CodeCoverageIgnore to @codeCoverageIgnore in Injector.php to properly exclude the DirectoryNotWritable exception line from coverage reports. PHPUnit requires the annotation to be lowercase 'code' not 'Code'. This fix ensures the line is correctly ignored by code coverage tools. Coverage improvement: - Injector class: now 100% (21/21 lines) - Overall: 99.24% (787/793 lines) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add .sonarcloud.properties to exclude .github/workflows/** from SonarCloud security hotspot analysis. Rationale: - GitHub Actions workflow files using version tags (@v1, @v2, etc.) trigger S7637 security hotspot warnings - Using version tags is GitHub Actions best practice and industry standard - These are references to our own trusted repositories - Excluding workflows from analysis prevents false positive warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
797739d to
be7c607
Compare
|



Summary
Details
This PR updates the continuous integration workflow to include PHP 8.5 support. The existing
composer.jsonrequirement of"php": "^7.2 || ^8.0"already supports PHP 8.5, so no changes to package requirements were necessary. This update ensures that the package is tested against PHP 8.5 to maintain compatibility.Test plan
🤖 Generated with Claude Code
Summary by Sourcery
Update CI workflow to add PHP 8.5 support by including it in the PHP versions matrix, demoting 8.4 to old_stable and marking 8.5 as current_stable.
CI:
Summary by CodeRabbit
Chores
Style
Tests