-
Notifications
You must be signed in to change notification settings - Fork 10
Fix incorrect type hint in docblocks for getOr and getOrSend methods #18
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request updates the method signatures in the Changes
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (1)
src/Nullable.php (1)
35-35
: Type hint improvement is appropriate but could be more comprehensive.The change from
@param string $default
to@param string|array $default
is indeed more accurate since thegetOr
method can accept both a string and an array as the default value. This is verified by the implementation at line 46-50 where it checks if$default
is callable (which can be a string function name or an array with class/method) and otherwise returns it directly.However, the type hint could be even more comprehensive as
$default
can actually be of any type (not just string or array) that would be returned if the predicate returns true.For even better IDE support, consider updating to
@param mixed $default
to reflect that any type can be used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Nullable.php
(2 hunks)
🔇 Additional comments (1)
src/Nullable.php (1)
75-75
: Type hint improvement is appropriate.The change from
@param string $callable
to@param string|array $callable
improves accuracy as PHP callables can be:
- A string containing a function name
- An array containing an object and method name
- An array containing a class name and static method
This modification aligns with the implementation at line 86-88 where
is_callable()
is used to validate the parameter, and with line 98 which references providing "a valid http response or a callable".
This pull request updates the docblocks of the
getOr
andgetOrSend
methods to reflect the correct type hinting.✔ Changes:
I updated the
@param
annotation for $default and $callable to include an array in addition to a string.This ensures that the documentation aligns with the expected parameter types, improving code clarity and maintainability.
This change helps IDEs suggest code better by providing more accurate type hints.
No functional changes were made to the code. this is a documentation fix only.
Summary by CodeRabbit