Skip to content

Conversation

@ChiragAgg5k
Copy link
Member

@ChiragAgg5k ChiragAgg5k commented Feb 9, 2026

Summary

  • make nullable parameters explicitly nullable to avoid PHP 8.4+ deprecations
  • align Adapter/Stripe/Pay signatures with nullable defaults
  • update Exception and Address constructors for explicit nullables

Testing

  • php -l on all php files

Summary by CodeRabbit

  • Refactor
    • Enhanced type safety across payment processing methods to improve code reliability and consistency.
    • Updated parameter handling for payment operations, refunds, customer management, and billing details to ensure more robust validation and error handling.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 9, 2026

Walkthrough

This pull request updates type hints across the payment module to explicitly declare parameters as nullable. Five abstract and concrete method signatures in the Adapter, Stripe adapter, Address, Exception, and Pay classes are modified to add the nullable operator (?) to parameter types. The changes affect optional parameters like currency, amount, reason, name, email, phone, address, and payment method identifiers. No logic or behavioral changes are introduced; only type declarations are modified for consistency and clarity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: updating nullable types to be explicitly nullable for PHP 8.4+ compatibility. It directly relates to all modified files and the core objective.
Docstring Coverage ✅ Passed Docstring coverage is 88.24% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/php84-nullables

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/Pay/Adapter.php`:
- Line 146: Update the Pay::refund method signature to match Adapter::refund by
changing it to public function refund(string $paymentId, ?int $amount = null,
?string $reason = null): array and forward the new $reason argument when calling
the adapter (ensure the adapter call in Pay::refund passes $paymentId, $amount,
$reason to the underlying Adapter::refund implementation).

In `@src/Pay/Pay.php`:
- Around line 221-224: Pay::updatePaymentMethodBillingDetails currently accepts
a $type parameter but doesn't use it; update it to pass $type through to the
adapter by adding the $type argument to the call to
Adapter::updatePaymentMethodBillingDetails and then update
Adapter::updatePaymentMethodBillingDetails signature (and all adapter
implementations) to accept the new $type parameter so the value is forwarded and
handled consistently.
🧹 Nitpick comments (1)
src/Pay/Adapter/Stripe.php (1)

129-142: LGTM on the nullable type fix. One pre-existing concern worth noting: $amount != null (loose comparison) on line 133 treats 0 as null, so a full-amount refund explicitly passed as 0 would be silently skipped. Consider using !== null for strictness.

Optional: use strict null checks
-        if ($amount != null) {
+        if ($amount !== null) {
             $requestBody['amount'] = $amount;
         }
 
-        if ($reason != null) {
+        if ($reason !== null) {
             $requestBody['reason'] = $reason;
         }

@ChiragAgg5k ChiragAgg5k merged commit 4af7cdf into main Feb 9, 2026
7 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix/php84-nullables branch February 9, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants