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.

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

Update serialization mechanism #90

Merged
merged 4 commits into from
Apr 17, 2024

Conversation

gmazzap
Copy link
Contributor

@gmazzap gmazzap commented Apr 16, 2024

Currently we have a rule that discourages the usage of magic __serialize and __unserialize while suggesting usage of the Serializable interface.

That was the "state of the art" up to PHP 7.3, and the rule even predates that.

With PHP 7.4 the Serializable interface is not recommended anymore (the RFC define it as "severely broken").

The new recommended method is to use __serialize and __unserialize methods.

Not being a major release PHP 7.4 did not deprecate Serializable.

PHP 8.1 triggers a deprecation warning if Serializable without also having __serialize and __unserialize.

And PHP 9 will probably remove the interface altogether.

Because v2 is PHP 7.4 we can safely discourage Serializable already and fully recommend __serialize and __unserialize.

This PR does that by:

  • Edit the sniff that discouraged __serialize and __unserialize to actually suggest them where __sleep and __wakeup are used
  • Add a sniff to discourage Serializable interface, suggesting __serialize and __unserialize

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Feature.

What is the current behavior? (You can also link to an open issue here)

  • __serialize and __unserialize are discouraged
  • Serializable interface is suggested

What is the new behavior (if this is a feature change)?

  • __serialize and __unserialize are suggested
  • Serializable interface is discouraged

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

  • New errors might be reported

Other information:

--

- Suggest suggest __serialize and __unserialize (previously discouraged)
- Add sniff to discourage Serializable interface

See
- https://wiki.php.net/rfc/custom_object_serialization
- https://www.php.net/manual/en/class.serializable.php
Copy link
Contributor

@shvlv shvlv left a comment

Choose a reason for hiding this comment

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

We have to update documentation

| `DisableMagicSerialize` | Disable usage of `__serialize`, `__sleep`, `__unserialize`, `__wakeup`. | | |
and add a new item for DisableSerializeInterfaceSniff.

Apart from that looks good 👍

- Added docs for DisableSerializeInterface
- Updated docs for DisableMagicSerialize
@gmazzap
Copy link
Contributor Author

gmazzap commented Apr 17, 2024

We have to update documentation

Thank you @shvlv, done.

@gmazzap gmazzap merged commit 89128a5 into version/2 Apr 17, 2024
32 checks passed
@gmazzap gmazzap deleted the fix/update-serialization-mechanism branch April 17, 2024 06:57
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.

None yet

3 participants