-
Notifications
You must be signed in to change notification settings - Fork 0
feat: improve PHPStan type annotations for Result interface #7
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
- Add @phpstan-assert-if-false annotations to isOk() and isErr() - Use conditional return types for unwrap(), unwrapErr(), and unwrapOr() - These improvements enable better type inference and reduce false positives This change makes the Result type more closely aligned with Rust's type safety
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 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.
Summary of Changes
Hello @valbeat, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've enhanced the "Result" interface's PHPStan type annotations to significantly improve type safety and reduce false positives. This change refines how PHPStan understands the state of "Result" objects, leading to more accurate type inference and a better developer experience without altering any runtime behavior.
Highlights
- Type Assertion Improvements: I've added "@phpstan-assert-if-false" annotations to the "isOk()" and "isErr()" methods. This allows PHPStan to correctly infer the type of the Result object when these methods return "false", specifically asserting "Err" when "isOk()" is false, and "Ok" when "isErr()" is false.
- Conditional Return Types: I've updated the "unwrap()", "unwrapErr()", and "unwrapOr()" methods to use conditional return types. This provides more precise type information based on whether the Result is Ok or Err, aligning more closely with Rust's Result type safety and reducing the need for manual type casting or "@phpstan-ignore" directives.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request significantly improves the PHPStan type annotations for the Result interface. The use of @phpstan-assert-if-false on isOk() and isErr(), and the introduction of conditional return types for unwrap(), unwrapErr(), and unwrapOr() are excellent changes that will enhance type safety and improve the developer experience by providing more accurate type inference to static analysis tools.
The changes are correct and well-implemented. To make the interface even more robust and consistent, I'd suggest applying similar conditional return types to other methods in a follow-up effort. For instance:
unwrapOrElse(callable $fn)could be annotated with@return ($this is Ok<T> ? T : U).match(callable $ok_fn, callable $err_fn)could be annotated with@return ($this is Ok<T> ? U : V).
Additionally, while reviewing, I noticed that the docblock for mapOr() specifies a return type of T|U, but the implementations in both Ok and Err classes always return a value of type U. Correcting this to @return U would improve the accuracy of the type hint.
Overall, this is a valuable contribution that strengthens the library's type safety. Great work!
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
This PR enhances the PHPStan type annotations for the Result interface to provide better type safety and reduce false positives. The changes focus on improving type inference through assertion annotations and conditional return types.
- Adds assertion annotations to
isOk()andisErr()methods for better type narrowing - Implements conditional return types for unwrap methods to improve type inference
- Aligns PHP implementation more closely with Rust's Result type safety patterns
Summary
@phpstan-assert-if-falseannotations toisOk()andisErr()methodsunwrap(),unwrapErr(), andunwrapOr()methodsChanges
This PR enhances the PHPStan type annotations in the Result interface to provide better type safety:
1. Assertion Annotations
isOk(): Now asserts that$thisisErr<E>when falseisErr(): Now asserts that$thisisOk<T>when false2. Conditional Return Types
unwrap():@return ($this is Ok<T> ? T : never)unwrapErr():@return ($this is Err<E> ? E : never)unwrapOr():@return ($this is Ok<T> ? T : U)Benefits
Test Plan
Summary by CodeRabbit