-
-
Notifications
You must be signed in to change notification settings - Fork 13
Fixed Psalm errors #38
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
WalkthroughThe changes introduce the PHP Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (4)
src/Exception/UnimplementedException.php (1)
9-11: Prefer nativefinalkeyword over PHPDoc@finalPsalm will honor the
@finalannotation, but using the language-levelfinalkeyword enforces immutability at runtime and improves readability. You could replace the docblock with:-/** - * @final - */ -class UnimplementedException extends GRPCException +final class UnimplementedException extends GRPCExceptionAfterward, you may remove the redundant PHPDoc block. Also, please verify that all other exception classes in
src/Exception/are consistently declared final.src/Exception/UnauthenticatedException.php (1)
9-11: Enhance immutability with thefinalkeyword (or correct Psalm tag).
Currently a generic@finaldocblock is used to signal to static analyzers that this class shouldn’t be extended. For stronger, language-level enforcement, declare the class asfinal:-/** - * @final - */ -class UnauthenticatedException extends InvokeException +final class UnauthenticatedException extends InvokeExceptionIf the intent is purely to satisfy Psalm, consider using the
@psalm-finalannotation instead of the generic@final.src/Exception/NotFoundException.php (1)
9-13: Optional: Enforce Finality at Runtime
The@finalannotation is only enforced by static analysis tools. To prevent subclassing at runtime as well, consider adding thefinalkeyword to the class declaration.-class NotFoundException extends InvokeException +final class NotFoundException extends InvokeExceptionsrc/Exception/ServiceException.php (1)
9-11: Marking ServiceException as final for static analysis
The PHPDoc@finalannotation prevents extending this exception in static analysis tools (e.g. Psalm). If you want to enforce finality at runtime, consider adding thefinalkeyword to the class declaration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (11)
composer.json(1 hunks)src/Context.php(3 hunks)src/Exception/GRPCException.php(2 hunks)src/Exception/GRPCExceptionInterface.php(1 hunks)src/Exception/NotFoundException.php(1 hunks)src/Exception/ServiceException.php(1 hunks)src/Exception/UnauthenticatedException.php(1 hunks)src/Exception/UnimplementedException.php(1 hunks)src/Invoker.php(1 hunks)src/ResponseHeaders.php(1 hunks)src/ResponseTrailers.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/Invoker.php (4)
src/Exception/GRPCException.php (3)
Override(56-60)Override(65-69)Override(71-75)src/Context.php (9)
Override(21-28)Override(30-34)Override(36-40)Override(42-54)Override(56-62)Override(64-70)Override(72-78)Override(80-84)Override(86-90)src/ResponseHeaders.php (2)
Override(49-53)Override(55-59)src/ResponseTrailers.php (2)
Override(49-53)Override(55-59)
src/Exception/GRPCExceptionInterface.php (5)
src/Exception/GRPCException.php (3)
Override(56-60)Override(65-69)Override(71-75)src/Context.php (9)
Override(21-28)Override(30-34)Override(36-40)Override(42-54)Override(56-62)Override(64-70)Override(72-78)Override(80-84)Override(86-90)src/ResponseHeaders.php (2)
Override(49-53)Override(55-59)src/ResponseTrailers.php (1)
Override(49-53)src/Invoker.php (1)
Override(19-41)
src/ResponseHeaders.php (2)
src/Context.php (9)
Override(21-28)Override(30-34)Override(36-40)Override(42-54)Override(56-62)Override(64-70)Override(72-78)Override(80-84)Override(86-90)src/ResponseTrailers.php (2)
Override(49-53)Override(55-59)
src/ResponseTrailers.php (2)
src/Context.php (9)
Override(21-28)Override(30-34)Override(36-40)Override(42-54)Override(56-62)Override(64-70)Override(72-78)Override(80-84)Override(86-90)src/ResponseHeaders.php (2)
Override(49-53)Override(55-59)
src/Exception/GRPCException.php (5)
src/Context.php (9)
Override(21-28)Override(30-34)Override(36-40)Override(42-54)Override(56-62)Override(64-70)Override(72-78)Override(80-84)Override(86-90)src/ResponseHeaders.php (2)
Override(49-53)Override(55-59)src/ResponseTrailers.php (2)
Override(49-53)Override(55-59)src/Invoker.php (1)
Override(19-41)src/Exception/MutableGRPCExceptionInterface.php (1)
setDetails(16-16)
src/Context.php (4)
src/Exception/GRPCException.php (3)
Override(56-60)Override(65-69)Override(71-75)src/ResponseHeaders.php (2)
Override(49-53)Override(55-59)src/ResponseTrailers.php (2)
Override(49-53)Override(55-59)src/ContextInterface.php (2)
getValue(27-27)getValues(34-34)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: psalm / PHP 8.3-ubuntu-latest
🔇 Additional comments (11)
src/Exception/NotFoundException.php (1)
9-11: Docblock@finalAnnotation Approved
The PHPDoc@finaltag correctly marks this exception as final for static analysis, matching the other exception classes in this directory.composer.json (1)
48-48: Expanded RoadRunner version constraint
The requirement for"spiral/roadrunner"was updated from^2024.3to^2024.3 || ^2025.1. Ensure that yourcomposer.lockis updated and validate compatibility with RoadRunner 2025.1 by running the full test suite.src/Invoker.php (1)
19-23: Explicit override attribute on invoke()
The#[\Override]attribute clearly denotes thatinvoke()is overriding a method fromInvokerInterface, mirroring similar annotations in other classes for consistency.src/Exception/GRPCExceptionInterface.php (1)
21-22: Combining ReturnTypeWillChange and Override
Applying#[\ReturnTypeWillChange, \Override]correctly suppresses deprecation warnings and documents thatgetCode()implements the interface contract. This follows the established pattern across the codebase.src/ResponseHeaders.php (2)
49-53: Explicit override attribute on getIterator()
Adding#[\Override]makes it clear this method implementsIteratorAggregate::getIterator(), improving readability and consistency.
55-59: Explicit override attribute on count()
Adding#[\Override]makes it clear this method implementsCountable::count(), improving readability and consistency.src/ResponseTrailers.php (1)
49-53: Override attribute on IteratorAggregate & Countable methodsThe
#[\Override]annotations ongetIterator()andcount()explicitly mark these methods as implementations of the\IteratorAggregateand\Countableinterfaces, in line with the project-wide convention.Also applies to: 55-59
src/Exception/GRPCException.php (1)
56-60: Override attribute on exception detail methodsAnnotating
getDetails(),setDetails(), andaddDetails()with#[\Override]correctly indicates they implement the corresponding interface methods, ensuring consistency and clarity across the exception hierarchy.Also applies to: 65-69, 71-75
src/Context.php (3)
21-28: Override attribute on ContextInterface methodsThe addition of
#[\Override]towithValue(),getValue(), andgetValues()accurately marks these methods as implementations ofContextInterface, matching the override patterns applied elsewhere.Also applies to: 30-34, 36-40
42-54: Override attribute on ArrayAccess methodsMarking
offsetExists(),offsetGet(),offsetSet(), andoffsetUnset()with#[\Override]clearly denotes they fulfill theArrayAccesscontract, maintaining consistency across the codebase.Also applies to: 56-62, 64-70, 72-78
80-84: Override attribute on IteratorAggregate & Countable methodsAnnotating
getIterator()andcount()with#[\Override]follows the same convention used in other classes for traversable and countable implementations.Also applies to: 86-90
Summary by CodeRabbit