Mark typehinted variables as nullable if they have null defaults #144
Conversation
…ine with PHP 8.4 deprecations
WalkthroughThis change updates multiple method and constructor signatures across several classes to use explicit nullable type declarations ( Changes
Sequence Diagram(s)Not applicable: No new features or control flow changes introduced. Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ollyollyollyltd, 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 updated the codebase to explicitly mark type-hinted parameters as nullable (?Type) in cases where they also have a null default value. This change improves the clarity of method signatures and enhances compatibility with static analysis tools, ensuring that the intended nullability of these parameters is clearly communicated.
Highlights
- Explicit Nullable Type Hints: Updated numerous method and constructor signatures across the Arrayy library to use nullable type hints (e.g., ?string, ?int, ?array, ?callable, ?bool, ?TypeInterface) for parameters that have a null default.
- Improved Type Consistency: Ensured consistency in type declarations, making the code more robust and easier to analyze by tools like PHPStan or Psalm.
- Enhanced Code Clarity: Addressed implicit nullability by making it explicit, which is a best practice in modern PHP development.
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.
Code Review
This pull request correctly adds nullable type hints to parameters that have a null default value. The changes are consistent across multiple files and improve the code's type safety and clarity for static analysis tools. All the modifications are correct and follow best practices. This is a great improvement.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/Collection/Collection.php (1)
84-101: Docblock & phpstan annotation now out of sync with the new nullable signature
$checkPropertiesInConstructoris now declared as?bool, but the docblock still says@param bool, and the phpstan annotation for$iteratorClassis missing the|nullpart.
Keeping docs/types aligned is essential for static analysis and IDE hints.@@ - * @param bool $checkPropertiesInConstructor optional <p> + * @param bool|null $checkPropertiesInConstructor optional <p> @@ - * @phpstan-param class-string<\Arrayy\ArrayyIterator> $iteratorClass + * @phpstan-param class-string<\Arrayy\ArrayyIterator>|null $iteratorClass
🧹 Nitpick comments (1)
src/Arrayy.php (1)
2648-2656: Nullable$comparisonOpaccepted – guard against unknown op?While adding
?stringis correct, an unexpected empty string or unknown op will still lead to$ops[$comparisonOp]throwing an undefined-index notice. Consider validating$comparisonOpearlier and throwing a descriptive exception.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Arrayy.php(16 hunks)src/ArrayyRewindableExtendedGenerator.php(1 hunks)src/ArrayyRewindableGenerator.php(1 hunks)src/Collection/Collection.php(1 hunks)src/Collection/CollectionInterface.php(1 hunks)src/StaticArrayy.php(1 hunks)src/Type/InstanceCollection.php(1 hunks)src/Type/InstancesCollection.php(1 hunks)
⏰ 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: Codacy Static Code Analysis
🔇 Additional comments (20)
src/StaticArrayy.php (1)
78-78: LGTM: Correctly addresses PHP 8.4 deprecation warningThe nullable type declaration
?int $stop = nullproperly aligns with the method's logic and existing documentation, resolving the implicit nullable parameter deprecation warning.src/ArrayyRewindableExtendedGenerator.php (1)
18-18: LGTM: Proper nullable callable parameterThe explicit nullable type
?callable $onRewind = nullcorrectly reflects that this parameter can accept null values, consistent with the property declaration and usage pattern.src/Collection/CollectionInterface.php (1)
499-499: LGTM: Interface method properly updated for nullable parameterThe explicit nullable type
?int $length = nullcorrectly represents that this parameter can be null (indicating no limit), aligning with the documentation and expected behavior.src/ArrayyRewindableGenerator.php (1)
52-52: LGTM: Nullable callable parameter correctly declaredThe explicit nullable type
?callable $onRewind = nullaligns with the property declaration, documentation, and runtime usage check, properly addressing the PHP 8.4 deprecation warning.src/Type/InstanceCollection.php (1)
29-30: LGTM: Constructor parameters properly updated to nullable typesBoth parameters are correctly declared as nullable (
?stringand?bool) to match their documented behavior and the constructor logic that handles null values with appropriate fallbacks.src/Type/InstancesCollection.php (1)
29-31: Explicit nullability correctly declared – nothing else to fixSwitching the three parameters to nullable types (
?string,?bool,?array) fully resolves the PHP-8.4 deprecation.
Docblocks and phpstan annotations already reflect the change, so no action required here.src/Arrayy.php (14)
1944-1958: Explicit nullability added to delimiter/regex parameters – LGTMThe new
?stringtypes align with the null-default behaviour and silence PHP 8.4 deprecation warnings. No behavioural change is introduced.
2837-2852:firstsImmutable()now nullable-int – LGTMThe internal null-check already existed; signature now matches runtime behaviour.
2866-2881:firstsKeys()nullability – LGTM
2898-2912:firstsMutable()nullability – LGTM
4268-4297:lastsImmutable()nullability – LGTM
4313-4323:lastsMutable()nullability – LGTM
4727-4730:mostUsedValues()nullability – LGTM
5233-5263:randomImmutable()nullability – LGTM
5350-5372:randomMutable()nullability – LGTM
5432-5447:randomWeighted()nullability – LGTMNo logic change; weight handling remains intact.
6251-6266:shuffle()now accepts nullable source array – LGTM
6457-6469:slice()nullability – LGTM
6722-6737:splice()nullability – LGTM
6970-6995:toPermutation()nullability – LGTM
Fixes #143
This change is
Summary by CodeRabbit