-
Notifications
You must be signed in to change notification settings - Fork 214
Fix PHP 8.2 deprecated warnings in Dokan Lite #3078
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
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe pull request updates two files by replacing the deprecated PHP 8.2 magic method Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@includes/ReverseWithdrawal/ReverseWithdrawal.php`:
- Around line 38-43: The __unserialize method in ReverseWithdrawal (and the
identical method in the ChainableContainer trait/class) accepts the required
array $data parameter but never uses it, triggering linter warnings; to fix, add
unset( $data ); at the start (or before returning) of the __unserialize( array
$data ): void method in ReverseWithdrawal.php and the same __unserialize
implementation in includes/Traits/ChainableContainer.php so the parameter is
explicitly marked as unused and the linter warning is suppressed.
| * | ||
| * @param array $data Unserialized data. | ||
| */ | ||
| public function __wakeup() { | ||
| public function __unserialize( array $data ): void { | ||
| $message = ' Backtrace: ' . wp_debug_backtrace_summary(); | ||
| _doing_it_wrong( __METHOD__, $message . esc_html__( 'Unserializing instances of this class is forbidden.', 'dokan-lite' ), DOKAN_PLUGIN_VERSION ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped |
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.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the ReverseWithdrawal.php file
find . -name "ReverseWithdrawal.php" -type fRepository: getdokan/dokan
Length of output: 216
🏁 Script executed:
# Read the file to see the full method context
if [ -f "includes/ReverseWithdrawal/ReverseWithdrawal.php" ]; then
cat -n "includes/ReverseWithdrawal/ReverseWithdrawal.php"
fiRepository: getdokan/dokan
Length of output: 3700
🏁 Script executed:
# Check if there's a __serialize method (companion to __unserialize)
rg "__serialize|__unserialize" --context 5 "includes/ReverseWithdrawal/ReverseWithdrawal.php"Repository: getdokan/dokan
Length of output: 502
🏁 Script executed:
# Check for phpcs directives or linter configuration in the repository
find . -name "phpcs.xml*" -o -name ".phpcs.xml*" -o -name "phpstan.neon*" -o -name "psalm.xml*" | head -5Repository: getdokan/dokan
Length of output: 74
🏁 Script executed:
# Check phpcs configuration
cat phpcs.xml.distRepository: getdokan/dokan
Length of output: 9931
🏁 Script executed:
# Look for patterns of unused parameter handling in the codebase
rg "unset\(" --context 2 | head -30Repository: getdokan/dokan
Length of output: 1827
🏁 Script executed:
# Check if there are phpcs inline suppressions for unused parameters in similar contexts
rg "phpcs:ignore.*unused|@phpstan-ignore-next-line" --context 2 | head -30Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Search for other __unserialize or magic methods in the codebase to see patterns
rg "__unserialize|__serialize|__clone" --context 3Repository: getdokan/dokan
Length of output: 2775
🏁 Script executed:
# Check if ChainableContainer.php has the same issue and if it's addressed
cat -n includes/Traits/ChainableContainer.php | grep -A 5 "__unserialize"Repository: getdokan/dokan
Length of output: 457
🏁 Script executed:
# Verify PHP magic method signature requirements for __unserialize
web_search "PHP __unserialize magic method signature requirements"Repository: getdokan/dokan
Length of output: 108
🌐 Web query:
PHP __unserialize magic method signature parameter required
💡 Result:
Short answer: The required signature is
public function __unserialize(array $data): void
Details:
- __unserialize was introduced as part of the new serialization API in PHP 7.4. [1][2]
- The method must be public and accept a single parameter typed as array; using a different signature (including different parameter type or missing the parameter) is a fatal error. [1]
- It should not return a value (declare return type void). [1][3]
Sources:
[1] PHP Manual — Magic Methods.
[2] PHP RFC: Phasing out Serializable (introducing __serialize/__unserialize).
[3] PHP Manual — examples for builtins showing signature (e.g. SplFixedArray/DateTime __unserialize).
Add unset( $data ); to suppress unused parameter warning—apply same fix to ChainableContainer.php for consistency.
The $data parameter is required by the PHP magic method signature but unused in the implementation. While functionally correct, linters flag it. The same unused parameter issue exists identically in includes/Traits/ChainableContainer.php at line 31, so both should be updated consistently.
Suggested fix
public function __unserialize( array $data ): void {
+ unset( $data ); // Intentionally unused.
$message = ' Backtrace: ' . wp_debug_backtrace_summary();
_doing_it_wrong( __METHOD__, $message . esc_html__( 'Unserializing instances of this class is forbidden.', 'dokan-lite' ), DOKAN_PLUGIN_VERSION ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * | |
| * @param array $data Unserialized data. | |
| */ | |
| public function __wakeup() { | |
| public function __unserialize( array $data ): void { | |
| $message = ' Backtrace: ' . wp_debug_backtrace_summary(); | |
| _doing_it_wrong( __METHOD__, $message . esc_html__( 'Unserializing instances of this class is forbidden.', 'dokan-lite' ), DOKAN_PLUGIN_VERSION ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped | |
| * | |
| * `@param` array $data Unserialized data. | |
| */ | |
| public function __unserialize( array $data ): void { | |
| unset( $data ); // Intentionally unused. | |
| $message = ' Backtrace: ' . wp_debug_backtrace_summary(); | |
| _doing_it_wrong( __METHOD__, $message . esc_html__( 'Unserializing instances of this class is forbidden.', 'dokan-lite' ), DOKAN_PLUGIN_VERSION ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped |
🧰 Tools
🪛 PHPMD (2.15.0)
41-41: Avoid unused parameters such as '$data'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In `@includes/ReverseWithdrawal/ReverseWithdrawal.php` around lines 38 - 43, The
__unserialize method in ReverseWithdrawal (and the identical method in the
ChainableContainer trait/class) accepts the required array $data parameter but
never uses it, triggering linter warnings; to fix, add unset( $data ); at the
start (or before returning) of the __unserialize( array $data ): void method in
ReverseWithdrawal.php and the same __unserialize implementation in
includes/Traits/ChainableContainer.php so the parameter is explicitly marked as
unused and the linter warning is suppressed.
All Submissions:
Changes proposed in this Pull Request:
This pull request fixes PHP 8.2 deprecated warnings in Dokan Lite caused by legacy serialization handling.
__wakeup()usage with__unserialize()as required by PHP 8.2.These changes remove deprecated notices from PHP error logs and improve forward compatibility.
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
WP_DEBUGandWP_DEBUG_LOG.Changelog entry
Fix PHP 8.2 deprecated warnings in Dokan Lite
Previously, Dokan Lite triggered PHP 8.2 deprecated warnings due to outdated serialization handling. This update aligns the code with modern PHP standards while preserving existing behavior.
Before Changes
After Changes
Feature Video (optional)
N/A
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.