fix: remove return type declarations from base classes to restore addon compatibility#780
Merged
superdav42 merged 2 commits intomainfrom Apr 10, 2026
Merged
Conversation
External gateway plugins that extend Base_Gateway without matching return type declarations cause a PHP fatal error. Removing return types from the abstract base class restores compatibility with third-party gateways while keeping the concrete implementations (Stripe, PayPal) typed.
…store addon compatibility PHP enforces that child class method signatures must be compatible with parent declarations. When a parent adds a return type (e.g. ': string'), any existing child class that overrides that method without the return type causes a fatal Compile Error on PHP 8.x. This broke the WooCommerce addon v2.0.22 (and potentially other addons) when upgrading to UM v2.5.1, because Base_Gateway added ': string' to get_payment_url_on_gateway() and other methods that addons override. Removes return type declarations from all overridable public methods in: - Base_Gateway (12 methods) + all bundled gateway subclasses - Base_Element (17 methods) + all bundled element subclasses - Base_Model (6 methods) - Base_Host_Provider (11 methods) + DNS_Provider_Interface - Base_Capability_Module (7 methods) + Capability_Module interface - Base_Signup_Field (1 method) Root cause: two commits added return types to existing public APIs: - b41dc2b 'Use PHP 7.4 features' added void returns to setters - b8df0b8 'Add PayPal REST gateway' added string returns to URL methods PHPDoc @return tags are preserved for IDE/static analysis support. Bundled subclasses that override these methods also have their return types removed for consistency. External addon subclasses that already added matching return types will continue to work (child can be more specific than parent). Reported-by: Kenedy (WC addon v2.0.22 fatal on UM v2.5.1)
Contributor
📝 WalkthroughWalkthroughThis PR systematically removes explicit PHP return type declarations and parameter type hints from method signatures across gateway, integration, model, and UI element classes. Internal logic and behavior remain unchanged; only type enforcement at the signature level is relaxed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
inc/integrations/host-providers/class-cloudflare-host-provider.php (1)
312-369:⚠️ Potential issue | 🟠 MajorDon't abort the whole subdomain cleanup when one Cloudflare record is missing.
At Line 353,
returnexitson_remove_subdomain()on the first empty lookup, so the later candidate (notably thewww.record) is never removed. This branch also masksWP_Errorresponses fromcloudflare_api_call(). Use explicit error handling andcontinuefor a missing record instead.♻️ Proposed fix
foreach ($domains_to_remove as $original_subdomain) { $dns_entries = $this->cloudflare_api_call( "client/v4/zones/$zone_id/dns_records/", 'GET', [ 'name' => $original_subdomain, 'type' => 'CNAME', ] ); - if ( ! $dns_entries->result) { - return; - } + if (is_wp_error($dns_entries)) { + wu_log_add( + 'integration-cloudflare', + sprintf( + 'Failed to look up subdomain "%s" in Cloudflare. Reason: %s', + $original_subdomain, + $dns_entries->get_error_message() + ), + LogLevel::ERROR + ); + + return; + } + + if (empty($dns_entries->result)) { + continue; + } $dns_entry_to_remove = $dns_entries->result[0];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/integrations/host-providers/class-cloudflare-host-provider.php` around lines 312 - 369, The on_remove_subdomain method currently returns early when a DNS lookup or delete fails, preventing the loop from attempting the second candidate (e.g., the 'www.' record); update the foreach in class Cloudflare_Host_Provider::on_remove_subdomain to handle errors explicitly: after calling $this->cloudflare_api_call(...) for $dns_entries, if it returns a WP_Error or has an empty result, log a warning and continue to the next $original_subdomain instead of returning; similarly, after the DELETE call, if $results is a WP_Error log the failure and continue (do not return), and only log success when the delete actually succeeds. Ensure you reference $dns_entries, $dns_entry_to_remove and $results variables and use continue rather than return so both candidate records are processed.inc/ui/class-base-element.php (1)
220-252:⚠️ Potential issue | 🟠 MajorCompatibility pass is incomplete—concrete UI element lifecycle hooks still typed.
Untyping
Base_Element::init()helps addons extending the abstract base, but 17+ concrete element classes still exposesetup(),setup_preview(), andregister_scripts()with: voidreturn types. Any addon extending one of these concrete classes (Current_Site_Element,Domain_Mapping_Element,Template_Switching_Element,Site_Actions_Element,Site_Maintenance_Element,Thank_You_Element, and others) and overriding one of these hooks without matching return type will still hit the PHP 7.4+ declaration fatal this PR aims to eliminate. Either remove return types from these lifecycle hooks in concrete classes, or confirm addons do not extend these classes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/ui/class-base-element.php` around lines 220 - 252, The concrete element classes still declare lifecycle hooks with a ": void" return type (e.g., methods setup(), setup_preview(), register_scripts() on classes like Current_Site_Element, Domain_Mapping_Element, Template_Switching_Element, Site_Actions_Element, Site_Maintenance_Element, Thank_You_Element, etc.), which will break addons that override them without the type; remove the explicit ": void" return type from these concrete lifecycle method signatures (and from any overrides in those classes) so their declarations match the untyped Base_Element::init() lifecycle usage, or alternatively ensure every addon/subclass is updated to declare the same return type—pick the former (remove return types) for compatibility and update the listed methods across those concrete classes.
🧹 Nitpick comments (4)
inc/checkout/signup-fields/class-base-signup-field.php (1)
247-247: Document this intentional type-hint exception inline.This signature change is compatibility-critical; add a short note in the docblock (or inline) that
: voidis intentionally omitted for addon subclass compatibility, to prevent accidental reintroduction later.Suggested docblock tweak
/** * Sets the config values for the current field. + * + * Note: Return type intentionally omitted to preserve compatibility with + * third-party subclasses that may not declare `: void`. * * `@since` 2.0.0 * * `@param` array $attributes Array containing settings for the field. * `@return` void */ public function set_attributes($attributes) {As per coding guidelines, "Use type hints where present in existing PHP code. Use return type
voidon init methods."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/checkout/signup-fields/class-base-signup-field.php` at line 247, The set_attributes method signature intentionally omits a return type to preserve backward-compatible addon subclass overrides; add a brief inline docblock or single-line comment above the public function set_attributes($attributes) stating that the missing ": void" return type is deliberate for addon compatibility and must not be reintroduced, e.g., reference the compatibility requirement and the function name so future maintainers know this is intentional.inc/gateways/class-base-gateway.php (1)
592-592: Minor cleanup: mark intentionally unused parameter in default verifier.At Line 592,
$payment_idis intentionally unused in the fallback implementation; explicitly unsetting it keeps static analysis clean.✂️ Proposed cleanup
public function verify_and_complete_payment($payment_id) { + unset($payment_id); return [ 'success' => false,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/gateways/class-base-gateway.php` at line 592, The default implementation of verify_and_complete_payment($payment_id) leaves the $payment_id parameter unused; to silence static analyzers, explicitly mark it unused by unsetting it (e.g., call unset($payment_id)) at the start of the verify_and_complete_payment method body so the parameter is intentionally discarded.inc/gateways/class-paypal-gateway.php (1)
1718-1721: Optional: explicitly mark unused parameter in the legacy PayPal payment URL stub.At Line 1718, the argument is intentionally unused; adding
unset()keeps static analysis quiet.🧹 Proposed cleanup
public function get_payment_url_on_gateway($gateway_payment_id) { + unset($gateway_payment_id); return ''; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/gateways/class-paypal-gateway.php` around lines 1718 - 1721, The get_payment_url_on_gateway($gateway_payment_id) stub currently ignores its parameter; explicitly mark it unused by calling unset($gateway_payment_id) at the start of the function (or otherwise reference it in a no-op) and then return an empty string, so static analyzers stop flagging the unused variable; update the function body in class PayPal gateway (get_payment_url_on_gateway) accordingly.inc/ui/class-command-palette-manager.php (1)
40-40: Codify theinit()return-type exception used in this PR.Line 40 removes
: voidfrominit(), which now conflicts with the current PHP guideline. If this compatibility exception is intentional, please update the guideline/AGENTS rule so future changes don’t oscillate between conflicting standards.As per coding guidelines, "Use type hints where present in existing PHP code. Use return type
voidon init methods."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/ui/class-command-palette-manager.php` at line 40, The init() method in class Command_Palette_Manager lost its return type hint; restore the PHP return type by changing the method signature back to public function init(): void to comply with the project's guideline for init methods; ensure this matches any parent/interface signatures (Command_Palette_Manager::init) and update any unit tests or callers if they relied on the previous signature, or if the removal was intentional, update the guideline/AGENTS rule to document this compatibility exception.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/ui/class-template-previewer.php`:
- Line 45: Update the Template_Previewer::init method to match the Singleton
trait signature by adding the void return type; locate the init() method in the
Template_Previewer class and change its declaration to include ": void" so it
aligns with \WP_Ultimo\Traits\Singleton::init(): void and PHP method signature
compatibility.
---
Outside diff comments:
In `@inc/integrations/host-providers/class-cloudflare-host-provider.php`:
- Around line 312-369: The on_remove_subdomain method currently returns early
when a DNS lookup or delete fails, preventing the loop from attempting the
second candidate (e.g., the 'www.' record); update the foreach in class
Cloudflare_Host_Provider::on_remove_subdomain to handle errors explicitly: after
calling $this->cloudflare_api_call(...) for $dns_entries, if it returns a
WP_Error or has an empty result, log a warning and continue to the next
$original_subdomain instead of returning; similarly, after the DELETE call, if
$results is a WP_Error log the failure and continue (do not return), and only
log success when the delete actually succeeds. Ensure you reference
$dns_entries, $dns_entry_to_remove and $results variables and use continue
rather than return so both candidate records are processed.
In `@inc/ui/class-base-element.php`:
- Around line 220-252: The concrete element classes still declare lifecycle
hooks with a ": void" return type (e.g., methods setup(), setup_preview(),
register_scripts() on classes like Current_Site_Element, Domain_Mapping_Element,
Template_Switching_Element, Site_Actions_Element, Site_Maintenance_Element,
Thank_You_Element, etc.), which will break addons that override them without the
type; remove the explicit ": void" return type from these concrete lifecycle
method signatures (and from any overrides in those classes) so their
declarations match the untyped Base_Element::init() lifecycle usage, or
alternatively ensure every addon/subclass is updated to declare the same return
type—pick the former (remove return types) for compatibility and update the
listed methods across those concrete classes.
---
Nitpick comments:
In `@inc/checkout/signup-fields/class-base-signup-field.php`:
- Line 247: The set_attributes method signature intentionally omits a return
type to preserve backward-compatible addon subclass overrides; add a brief
inline docblock or single-line comment above the public function
set_attributes($attributes) stating that the missing ": void" return type is
deliberate for addon compatibility and must not be reintroduced, e.g., reference
the compatibility requirement and the function name so future maintainers know
this is intentional.
In `@inc/gateways/class-base-gateway.php`:
- Line 592: The default implementation of
verify_and_complete_payment($payment_id) leaves the $payment_id parameter
unused; to silence static analyzers, explicitly mark it unused by unsetting it
(e.g., call unset($payment_id)) at the start of the verify_and_complete_payment
method body so the parameter is intentionally discarded.
In `@inc/gateways/class-paypal-gateway.php`:
- Around line 1718-1721: The get_payment_url_on_gateway($gateway_payment_id)
stub currently ignores its parameter; explicitly mark it unused by calling
unset($gateway_payment_id) at the start of the function (or otherwise reference
it in a no-op) and then return an empty string, so static analyzers stop
flagging the unused variable; update the function body in class PayPal gateway
(get_payment_url_on_gateway) accordingly.
In `@inc/ui/class-command-palette-manager.php`:
- Line 40: The init() method in class Command_Palette_Manager lost its return
type hint; restore the PHP return type by changing the method signature back to
public function init(): void to comply with the project's guideline for init
methods; ensure this matches any parent/interface signatures
(Command_Palette_Manager::init) and update any unit tests or callers if they
relied on the previous signature, or if the removal was intentional, update the
guideline/AGENTS rule to document this compatibility exception.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a8daf22-fdb9-4641-92d1-6776cb321d58
📒 Files selected for processing (36)
inc/checkout/signup-fields/class-base-signup-field.phpinc/gateways/class-base-gateway.phpinc/gateways/class-base-paypal-gateway.phpinc/gateways/class-base-stripe-gateway.phpinc/gateways/class-paypal-gateway.phpinc/gateways/class-paypal-rest-gateway.phpinc/integrations/class-base-capability-module.phpinc/integrations/host-providers/class-base-host-provider.phpinc/integrations/host-providers/class-cloudflare-host-provider.phpinc/integrations/host-providers/class-cpanel-host-provider.phpinc/integrations/host-providers/class-hestia-host-provider.phpinc/integrations/host-providers/interface-dns-provider.phpinc/integrations/interface-capability-module.phpinc/models/class-base-model.phpinc/ui/class-account-summary-element.phpinc/ui/class-base-element.phpinc/ui/class-billing-info-element.phpinc/ui/class-checkout-element.phpinc/ui/class-command-palette-manager.phpinc/ui/class-current-membership-element.phpinc/ui/class-current-site-element.phpinc/ui/class-domain-mapping-element.phpinc/ui/class-invoices-element.phpinc/ui/class-limits-element.phpinc/ui/class-login-form-element.phpinc/ui/class-magic-link-url-element.phpinc/ui/class-my-sites-element.phpinc/ui/class-payment-methods-element.phpinc/ui/class-simple-text-element.phpinc/ui/class-site-actions-element.phpinc/ui/class-site-maintenance-element.phpinc/ui/class-template-previewer.phpinc/ui/class-template-switching-element.phpinc/ui/class-thank-you-element.phpinc/ui/class-toolbox.phpinc/ui/class-tours.php
| * @return void | ||
| */ | ||
| public function init(): void { | ||
| public function init() { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate singleton-related definitions.
fd -i 'singleton' inc | sort
# 2) Inspect interface/trait signatures for init/get_instance contracts.
rg -n -C3 --type=php 'interface\s+.*Singleton|trait\s+Singleton|function\s+init\s*\(|function\s+get_instance\s*\(' inc
# 3) Check whether classes using the Singleton trait also implement a Singleton interface.
rg -n -C2 --type=php 'class\s+\w+.*\{|use\s+\\WP_Ultimo\\Traits\\Singleton|implements\s+.*Singleton' incRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 50394
🏁 Script executed:
cat -n inc/ui/class-template-previewer.php | head -60Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1532
Add return type void to init() method.
Template_Previewer uses \WP_Ultimo\Traits\Singleton (line 24), which declares init(): void. The method on line 45 is missing the return type hint and must be updated to match the trait signature and align with coding guidelines.
public function init(): void {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/ui/class-template-previewer.php` at line 45, Update the
Template_Previewer::init method to match the Singleton trait signature by adding
the void return type; locate the init() method in the Template_Previewer class
and change its declaration to include ": void" so it aligns with
\WP_Ultimo\Traits\Singleton::init(): void and PHP method signature
compatibility.
Collaborator
Author
SummaryFixes a fatal Compile Error on PHP 8.x that breaks the WooCommerce addon v2.0.22 (and potentially other addons) when upgrading to UM v2.5.1. Sites go completely down on any page load. Root CauseTwo earlier commits added PHP return type declarations to public methods on base/abstract classes that external addons extend:
This is a fatal error — no graceful degradation, site is completely down. What ChangedRemoved return type declarations from all overridable public methods across 6 base/abstract classes and their bundled subclasses (36 files total):
Why Not Just Fix the Addon?The addon fix is trivial (add
Testing
Runtime Testing
Merged via PR #780 to main. aidevops.sh v3.6.235 spent 6m on this as a headless bash routine. |
|
Performance Test Results Performance test results for 954d77d are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
2 tasks
superdav42
added a commit
that referenced
this pull request
Apr 11, 2026
…, static analysis - Cloudflare on_remove_subdomain: use continue instead of return when DNS record is missing so both subdomain candidates are processed; add explicit WP_Error check for API lookup failures with proper logging - Remove : void return types from setup(), setup_preview(), register_scripts() on all concrete UI element classes to prevent PHP fatal errors in addons that override these methods without matching return type declarations - Restore : void to Command_Palette_Manager::init() and Template_Previewer::init() to match Singleton trait signature (concrete classes, not base/abstract) - Add compatibility note to Base_Signup_Field::set_attributes() docblock - Add unset() for intentionally unused parameters in Base_Gateway and PayPal gateway Fixes #794
This was referenced Apr 11, 2026
superdav42
added a commit
that referenced
this pull request
Apr 11, 2026
…, static analysis (#796) - Cloudflare on_remove_subdomain: use continue instead of return when DNS record is missing so both subdomain candidates are processed; add explicit WP_Error check for API lookup failures with proper logging - Remove : void return types from setup(), setup_preview(), register_scripts() on all concrete UI element classes to prevent PHP fatal errors in addons that override these methods without matching return type declarations - Restore : void to Command_Palette_Manager::init() and Template_Previewer::init() to match Singleton trait signature (concrete classes, not base/abstract) - Add compatibility note to Base_Signup_Field::set_attributes() docblock - Add unset() for intentionally unused parameters in Base_Gateway and PayPal gateway Fixes #794
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a fatal Compile Error on PHP 8.x that breaks the WooCommerce addon v2.0.22 (and potentially other addons) when upgrading to UM v2.5.1. Sites go completely down on any page load.
Root Cause
Two earlier commits added PHP return type declarations to public methods on base/abstract classes that external addons extend:
b41dc2b2"Use PHP 7.4 features": voidto setters (set_order,set_payment,set_membership,set_customer,trigger_payment_processed)b8df0b85"Add PayPal REST gateway": stringto URL methods (get_payment_url_on_gateway,get_subscription_url_on_gateway,get_customer_url_on_gateway)PHP enforces that child class method signatures must be compatible with parent declarations. When a parent adds
: string, any existing child class overriding that method without the return type causes:This is a fatal error — no graceful degradation, site is completely down.
What Changed
Removed return type declarations from all overridable public methods across 6 base/abstract classes and their bundled subclasses (36 files total):
Base_Gateway(12 methods) — the confirmed breaking classBase_Element(17 methods) —output(): voidis abstract, every UI element addon must implement itBase_Model(6 methods) —set_slug(),hydrate(), etc.Base_Host_Provider(11 methods) — extended by hosting integration addonsBase_Capability_Module+Capability_Moduleinterface (7 methods)Base_Signup_Field(1 method)All bundled subclasses (Stripe, PayPal, PayPal REST, all UI elements, host providers) also had their return types removed for consistency.
PHPDoc
@returntags are preserved — IDEs and PHPStan still get type information.Why Not Just Fix the Addon?
The addon fix is trivial (add
: stringto the override). But:Base_Element::output(): voidabstract method would break every custom UI element addonTesting
Runtime Testing
self-assessed— the fix is mechanical (removing type declarations), verified by PHPStan, and the failure mode is a PHP Compile Error that would be immediately visibleSummary by CodeRabbit