Remove shared StorageInterface in favor of package-level contracts#434
Conversation
📝 WalkthroughWalkthroughThe shared top-level Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 Tip You can disable poems in the walkthrough.Disable the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #434 +/- ##
=========================================
Coverage 82.64% 82.64%
Complexity 2721 2721
=========================================
Files 241 241
Lines 7305 7305
=========================================
Hits 6037 6037
Misses 1268 1268 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Cookie/Contracts/CookieStorageInterface.php`:
- Around line 54-59: Add an explicit void return type to the interface method
flush() in CookieStorageInterface so it matches set(): void and delete(): void
and the implementing class Cookie::flush(): void; update the method signature
from "public function flush();" to include ": void" to ensure consistency
between the interface and its implementation.
In `@src/Session/Contracts/SessionStorageInterface.php`:
- Around line 42-59: Update the SessionStorageInterface by adding explicit ":
void" return types to its method signatures for set(), delete(), and flush() so
they match the implemented signatures (and other interfaces like ConfigInterface
and DebuggerStoreInterface); modify the declarations in SessionStorageInterface
(methods set(string $key, $value), delete(string $key), flush()) to include the
void return type to enforce the contract consistency with the implementations in
SessionTrait.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 626db702-8bf1-4673-8d77-c1b2c59a6b29
📒 Files selected for processing (6)
src/Config/Config.phpsrc/Config/Contracts/ConfigInterface.phpsrc/Cookie/Contracts/CookieStorageInterface.phpsrc/Debugger/Contracts/DebuggerStoreInterface.phpsrc/Debugger/DebuggerStore.phpsrc/Session/Contracts/SessionStorageInterface.php
…eStorageInterface for consistency across all package contracts.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Cookie/Contracts/CookieStorageInterface.php (1)
25-29: Tightenall()contract toarrayinstead ofmixed.
all()represents full cookie storage and the concrete implementation returns an array; declaring that in the interface improves static guarantees and contract clarity.Proposed refactor
/** * Gets whole storage data - * `@return` mixed + * `@return` array<string, mixed> */ - public function all(); + public function all(): array;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Cookie/Contracts/CookieStorageInterface.php` around lines 25 - 29, The interface method all() currently returns mixed; tighten its return type to array by changing the CookieStorageInterface::all() signature to return array, update its docblock accordingly, and then update every implementing class (implementations of CookieStorageInterface) to match the new return type on their all() methods, adjusting any callers or tests that relied on mixed to accept/expect an array; ensure method signatures remain compatible with PHP return type rules and run static checks/tests to catch any mismatches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Cookie/Contracts/CookieStorageInterface.php`:
- Around line 25-29: The interface method all() currently returns mixed; tighten
its return type to array by changing the CookieStorageInterface::all() signature
to return array, update its docblock accordingly, and then update every
implementing class (implementations of CookieStorageInterface) to match the new
return type on their all() methods, adjusting any callers or tests that relied
on mixed to accept/expect an array; ensure method signatures remain compatible
with PHP return type rules and run static checks/tests to catch any mismatches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b341578-ca00-43f9-8e93-99edea259726
📒 Files selected for processing (2)
src/Cookie/Contracts/CookieStorageInterface.phpsrc/Session/Contracts/SessionStorageInterface.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Session/Contracts/SessionStorageInterface.php
Closes #433
Summary by CodeRabbit