fixed PHP 8.4 deprecations #39
Conversation
WalkthroughThe changes in this pull request involve updating the method signatures of several functions in the Changes
Poem
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 (
|
![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 and nitpick comments (1)
src/Worker.php (1)
Line range hint
138-143: Update PHPDoc to reflect nullable parameter.The method signature correctly marks the parameter as nullable, but the PHPDoc should be updated to match.
- * @param class-string<Payload>|null $class + * @param ?class-string<Payload> $class
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/Worker.php(2 hunks)src/WorkerInterface.php(1 hunks)
🔇 Additional comments (6)
src/WorkerInterface.php (3)
54-54: LGTM! Correctly addresses PHP 8.4 deprecation.
The change from string $class = null to ?string $class = null properly addresses the PHP 8.4 deprecation by making the nullable parameter explicit while maintaining backward compatibility.
62-62: LGTM! Correctly addresses PHP 8.4 deprecation.
The change from string $class = null to ?string $class = null properly addresses the PHP 8.4 deprecation by making the nullable parameter explicit while maintaining backward compatibility.
54-62: Verify interface implementations are updated.
Since we're modifying an interface, let's ensure all implementing classes have been updated with the new method signatures.
✅ Verification successful
Interface implementations are correctly updated
The verification shows that Worker class in src/Worker.php is the only implementation of WorkerInterface, and it correctly implements both methods with matching signatures:
hasPayload(?string $class = null): boolgetPayload(?string $class = null): ?Payload
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all PHP files that might implement WorkerInterface
echo "Searching for WorkerInterface implementations..."
rg -l "implements.*WorkerInterface" --type php
# Look for hasPayload and getPayload method signatures
echo "Checking method signatures in potential implementations..."
rg "(?:public|protected|private)\s+function\s+(?:hasPayload|getPayload)\s*\([^)]*\)" \
--type php \
--multiline \
--ignore-case
Length of output: 749
src/Worker.php (3)
Line range hint 126-136: LGTM: Consistent nullable parameter handling.
The method signature change is correct and the implementation properly handles the nullable parameter. The return type was already correctly marked as nullable.
121-124: LGTM: Explicit nullable parameter declaration.
The change correctly addresses the PHP 8.4 deprecation by making the nullable parameter explicit in the method signature. This maintains backward compatibility while following PHP 8.4 best practices.
Let's verify the interface alignment:
✅ Verification successful
✓ Method signature correctly aligns with interface
The hasPayload method signature in Worker.php perfectly matches the declaration in WorkerInterface.php, both using ?string $class = null: bool. The change maintains the contract while addressing PHP 8.4 deprecations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that WorkerInterface has the same nullable parameter declaration
ast-grep --pattern 'interface WorkerInterface {
$$$
hasPayload(?string $class = null): bool
$$$
}'
Length of output: 106
Script:
#!/bin/bash
# Let's first find the interface file
fd -e php WorkerInterface
# Then check the content of the interface file
rg -B2 -A2 "hasPayload" --type php
Length of output: 1412
Line range hint 121-143: Verify no other implicit nullable parameters.
The changes correctly address the PHP 8.4 deprecation warnings for the payload-related methods. Let's verify there are no other implicit nullable parameters in the codebase that might trigger similar warnings.
✅ Verification successful
All nullable parameters are properly typed, no action needed
The codebase scan shows all method parameters with default null values already have explicit nullable type declarations (?string, ?int). This includes:
hasPayload(?string $class = null)getPayload(?string $class = null)findPayload(?string $class = null)respond(Payload $payload, ?int $codec = null)send(string $body = '', string $header = '', bool $eos = true, ?int $codec = null)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other method parameters with default null that might need explicit nullable type
ast-grep --pattern 'function $NAME($PARAM = null)'
Length of output: 52
Script:
#!/bin/bash
# Search for method parameters with default null using ripgrep
rg "function.*\(.*=\s*null.*\)" --type php -A 1
Length of output: 840
2 participants
Got rid of deprecations like following:
Deprecated: Spiral\RoadRunner\Worker::hasPayload(): Implicitly marking parameter $class as nullable is deprecated, the explicit nullable type must be used instead in app/vendor/spiral/roadrunner-worker/src/Worker.php on line 121
My gRPC service does not run because of these deprecations.
Summary by CodeRabbit
New Features
Bug Fixes