Skip to content

Include class name in Serializable deprecation message #7346

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

Closed
wants to merge 1 commit into from

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Aug 6, 2021

The deprecation message was originally introduced in 3e6b447 (#6494).

I first encountered this notice when testing the MongoDB extension with PHP 8.1, which produced many duplicate messages that provided no detail about the particular class that needed to be fixed:

Deprecated: The Serializable interface is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in Unknown on line 0

@@ -20,4 +20,4 @@ class D extends A implements I {

?>
--EXPECTF--
Deprecated: The Serializable interface is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in %s on line %d
Deprecated: %s implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in %s on line %d
Copy link
Member Author

Choose a reason for hiding this comment

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

The PHPT files were all changed by a single search/replace. I assume the class name is as irrelevant as the filename and line number, which also use patterns.

That said, if it makes sense to have at least one assertion for the message including the offending class name perhaps Zend/tests/serializable_deprecation.phpt is the test in which to do so. Let me know if so.

@kocsismate
Copy link
Member

Duplicate of #7343 :( (I'm not sure though which message format is the best)

@nikic
Copy link
Member

nikic commented Aug 11, 2021

I slightly preferred the phrasing here (with the class name prominently at the front), so went with this implementation. Merged with a tweak to use ZSTR_VAL.

@wouterj
Copy link

wouterj commented Aug 11, 2021

Thanks @jmikola! I also like this message more.

@jmikola jmikola deleted the serializable-deprecation-msg branch August 11, 2021 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants