-
-
Notifications
You must be signed in to change notification settings - Fork 78
Bugfix/base gateway return type compat #781
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
Changes from all commits
a4a1907
0109e58
74224b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,11 @@ | |
| * For more info on actual implementations, | ||
| * check the Gateway_Manual class and the Gateway_Stripe class. | ||
| * | ||
| * @extensible External addons (e.g. WooCommerce, custom gateways) extend this class. | ||
| * Do NOT add PHP return type declarations to public methods — it will | ||
| * cause a fatal Compile Error in any addon that overrides the method | ||
| * without the matching return type. Use @return PHPDoc tags instead. | ||
| * | ||
| * @since 2.0.0 | ||
| */ | ||
| abstract class Base_Gateway { | ||
|
|
@@ -165,7 +170,7 @@ public function __construct($order = null) { | |
| * @param \WP_Ultimo\Checkout\Cart $order The order. | ||
| * @return void | ||
| */ | ||
| public function set_order($order): void { | ||
| public function set_order($order) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: No, in PHP 7.4+, a child class cannot override a parent class method that has no parameter type with a narrower typed parameter like int $payment_id, string $gateway_payment_id, or Membership $membership. This causes a fatal error: "Declaration of Child::method(int $param) must be compatible with Parent::method($param)". Untyped parameters in PHP are treated as accepting any type (effectively mixed). Adding a specific type in the child makes the parameter more specific (narrower), which violates contravariance rules for parameters. Child methods must have parameter types that are less specific or equal to the parent's (or untyped). This enforces the Liskov Substitution Principle: code expecting to pass any value to the parent method must still work when using the child class instance. Prior to PHP 7.4, it raised a warning; since PHP 7.4 with full contravariance support, incompatible signatures are fatal errors (strictly enforced in PHP 8+). Workarounds: runtime type checks inside the child method, or redesign with interfaces/abstract methods using the desired types. Citations:
Restore parameter types on public gateway base methods. Removing parameter types from parent methods breaks addon gateway overrides. In PHP 7.4+, if a parent method parameter is untyped, child overrides cannot narrow it with specific types (e.g., Affects lines: 173, 217, 597, 616, 631, 646, 824, 861, 873, 885, 899 🤖 Prompt for AI Agents |
||
|
|
||
| if (null === $order) { | ||
| return; | ||
|
|
@@ -209,7 +214,7 @@ final public function get_id() { | |
| * @param \WP_Ultimo\Models\Membership $membership The membership. | ||
| * @return array{brand: string, last4: string}|null Payment method info, or null. | ||
| */ | ||
| public function get_payment_method_display($membership): ?array { | ||
| public function get_payment_method_display($membership) { | ||
| unset($membership); | ||
| return null; | ||
| } | ||
|
|
@@ -568,7 +573,7 @@ public function process_confirmation() {} | |
| * @since 2.0.0 | ||
| * @return bool | ||
| */ | ||
| public function supports_payment_polling(): bool { | ||
| public function supports_payment_polling() { | ||
|
|
||
| return false; | ||
| } | ||
|
|
@@ -589,7 +594,7 @@ public function supports_payment_polling(): bool { | |
| * @param int $payment_id The local payment ID to verify. | ||
| * @return array{success: bool, status: string, message: string} | ||
| */ | ||
| public function verify_and_complete_payment(int $payment_id): array { | ||
| public function verify_and_complete_payment($payment_id) { | ||
|
|
||
| return [ | ||
| 'success' => false, | ||
|
|
@@ -608,7 +613,7 @@ public function verify_and_complete_payment(int $payment_id): array { | |
| * @param string $gateway_payment_id The gateway payment id. | ||
| * @return string | ||
| */ | ||
| public function get_payment_url_on_gateway($gateway_payment_id): string { | ||
| public function get_payment_url_on_gateway($gateway_payment_id) { | ||
| unset($gateway_payment_id); | ||
| return ''; | ||
| } | ||
|
|
@@ -623,7 +628,7 @@ public function get_payment_url_on_gateway($gateway_payment_id): string { | |
| * @param string $gateway_subscription_id The gateway subscription id. | ||
| * @return string | ||
| */ | ||
| public function get_subscription_url_on_gateway($gateway_subscription_id): string { | ||
| public function get_subscription_url_on_gateway($gateway_subscription_id) { | ||
| unset($gateway_subscription_id); | ||
| return ''; | ||
| } | ||
|
|
@@ -638,7 +643,7 @@ public function get_subscription_url_on_gateway($gateway_subscription_id): strin | |
| * @param string $gateway_customer_id The gateway customer id. | ||
| * @return string | ||
| */ | ||
| public function get_customer_url_on_gateway($gateway_customer_id): string { | ||
| public function get_customer_url_on_gateway($gateway_customer_id) { | ||
| unset($gateway_customer_id); | ||
| return ''; | ||
| } | ||
|
|
@@ -816,7 +821,7 @@ public function get_confirm_url() { | |
| * @param string $message The error message. | ||
| * @return void | ||
| */ | ||
| public function redirect_with_error(string $message): void { | ||
| public function redirect_with_error($message) { | ||
|
|
||
| $url = remove_query_arg(['wu-confirm', 'payment', 'token', 'PayerID', 'ba_token', 'subscription_id', 'status'], $this->return_url ?: wu_get_current_url()); | ||
|
|
||
|
|
@@ -853,7 +858,7 @@ public function get_webhook_listener_url() { | |
| * @param \WP_Ultimo\Models\Payment $payment The payment. | ||
| * @return void | ||
| */ | ||
| public function set_payment($payment): void { | ||
| public function set_payment($payment) { | ||
|
|
||
| $this->payment = $payment; | ||
| } | ||
|
|
@@ -865,7 +870,7 @@ public function set_payment($payment): void { | |
| * @param \WP_Ultimo\Models\Membership $membership The membership. | ||
| * @return void | ||
| */ | ||
| public function set_membership($membership): void { | ||
| public function set_membership($membership) { | ||
|
|
||
| $this->membership = $membership; | ||
| } | ||
|
|
@@ -877,7 +882,7 @@ public function set_membership($membership): void { | |
| * @param \WP_Ultimo\Models\Customer $customer The customer. | ||
| * @return void | ||
| */ | ||
| public function set_customer($customer): void { | ||
| public function set_customer($customer) { | ||
|
|
||
| $this->customer = $customer; | ||
| } | ||
|
|
@@ -891,7 +896,7 @@ public function set_customer($customer): void { | |
| * @param \WP_Ultimo\Models\Membership $membership The membership object. | ||
| * @return void | ||
| */ | ||
| public function trigger_payment_processed($payment, $membership = null): void { | ||
| public function trigger_payment_processed($payment, $membership = null) { | ||
|
|
||
| if (null === $membership) { | ||
| $membership = $payment->get_membership(); | ||
|
|
||
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.
Document the checkout scope consistently.
This narrows the rule to
inc/checkout/signup-fields/, but the repository guideline still applies it to all ofinc/checkout/**/*.php. LeavingAGENTS.mdnarrower than the actual rule will mislead future checkout changes.As per coding guidelines
{inc/gateways/**/*.php,inc/ui/class-base-element.php,inc/models/class-base-model.php,inc/integrations/**/*.php,inc/checkout/**/*.php}: NEVER add PHP return type declarations (...) to public methods on base/abstract classes or interfaces.🤖 Prompt for AI Agents