Skip to content
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.

Sign up for GitHub

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

Jump to bottom

BUGFIX: __wakeup must not return #3203

Draft
wants to merge 1 commit into
base: 7.3
Choose a base branch
from
Draft

BUGFIX: __wakeup must not return #3203

wants to merge 1 commit into from

Conversation

kdambekalns
Copy link
Member

Generated proxy classes return something in __wakeup, but that is not allowed since PHP 8.0. Root cause for this is the fact, that we check for void but take an "undefined return type" as "something".

This fixes the issue by checking for 'void' || null…

Review instructions

The __wakeup() method in the generated proxy class must not contain return $result anymore.

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked wit !!! and have upgrade-instructions

@kdambekalns
BUGFIX: __wakeup must not return
1477014 
Generated proxy classes return something in `__wakeup`, but that is
not allowed since PHP 8.0. Root cause for this is the fact, that we
check for `void` but take an "undefined return type" as "something".

This fixes the issue by checking for `'void' || null`…
@kdambekalns kdambekalns self-assigned this Oct 31, 2023
robertlemke
robertlemke approved these changes Oct 31, 2023
View reviewed changes
Copy link
Member

@robertlemke robertlemke left a comment

Choose a reason for hiding this comment

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

Good catch!

@kdambekalns kdambekalns marked this pull request as draft October 31, 2023 11:53
@kdambekalns
Copy link
Member Author

Ok, as I feared this is not as simple as it seemed. To be continued…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robertlemke robertlemke robertlemke approved these changes

@fcool fcool Awaiting requested review from fcool

@kitsunet kitsunet Awaiting requested review from kitsunet

@albe albe Awaiting requested review from albe

Assignees

@kdambekalns kdambekalns

Labels
7.3 Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kdambekalns @robertlemke