Fix Safe namespace deprecation w 8000 arnings#11227
Conversation
📝 WalkthroughWalkthroughThis PR makes widespread signature updates to add explicit nullable type hints (e.g., Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
commit: |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
a2a5d22 to
742dcd8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Appwrite/Utopia/Database/Documents/User.php`:
- Line 134: The method signature for tokenVerify currently uses an optional
parameter with a default (?int $type = null) followed by required parameters,
causing a PHP 8.x deprecation; update the tokenVerify declaration to remove the
default value so the parameter reads ?int $type (i.e., change from ?int $type =
null to ?int $type) in the User::tokenVerify method, leaving the rest of the
signature and logic unchanged since all call sites already pass the $type
explicitly.
🧹 Nitpick comments (1)
src/Appwrite/Platform/Modules/Compute/Base.php (1)
147-147: Remove unused$requestparameter from both methods - requires coordinated updates across multiple callers.Both
redeployVcsSite(line 147) andredeployVcsFunction(line 56) have unused$requestparameters. However, removing this parameter is a breaking change that requires updating callers:
- Positional argument calls in
Update.phpfiles (2 instances in Sites and Functions modules) will break if the parameter is removed- Named parameter calls in
Create.phpfiles use named arguments and would remain unaffectedIf proceeding with this refactor, ensure all positional calls are updated to remove the
$requestargument, or convert them to named parameter style.
742dcd8 to
6c43949
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Appwrite/Utopia/Database/Documents/User.php (1)
177-179:⚠️ Potential issue | 🟡 MinorDead code: duplicate
return false;statement.Line 179 is unreachable due to the
return false;on line 177. This appears to be an accidental duplication.🧹 Proposed fix
return false; - - return false; } }
🧹 Nitpick comments (2)
src/Appwrite/Platform/Action.php (1)
43-47: LGTM! Consider updating the docblock for consistency.The nullable type hint
?callable $callback = nullis correct and aligns with PHP 8.3 guidance. The existingis_callable($callback)guards ensure safe invocation.The docblock on line 43 still documents
@param callable $callback— consider updating to@param callable|null $callbackfor consistency.,
📝 Suggested docblock update
/** * Foreach Document * Call provided callback for each document in the collection * * `@param` string $projectId * `@param` string $collection * `@param` array $queries - * `@param` callable $callback + * `@param` callable|null $callback * * `@return` void */src/Appwrite/Platform/Modules/Compute/Base.php (1)
147-147: Remove the unused$requestparameter or suppress the PHPMD warning.The
$requestparameter at line 147 is never referenced in the method body and has no interface or parent class constraints. Either remove it if there are no backward compatibility concerns, or add a@SuppressWarnings(PHPMD.UnusedFormalParameter)annotation with a comment explaining why it must remain.
✨ Benchmark results
⚡ Benchmark Comparison
|
Summary
docker compose exec appwrite sdksoutputTesting