Hello Internals,
I would like to gather feedback on the proposal to deprecate userland
creation and cloning of __PHP_Incomplete_Class objects.
Here are the reasons I believe this deprecation would be beneficial:
- __PHP_Incomplete_Class is intended to be an internal mechanism for
handling incomplete object unserialization, not for direct userland
manipulation; - This change would align with PHP's general direction of preventing
misuse of internal classes; - Classes like Generator already prevent userland instantiation,
making this change consistent with existing patterns; - No added-value on using this class in userland, stdClass does
already the job if one wants to manipulate "incomplete" classes.
Having stdClass and __PHP_Incomplete_Class could be misleading; - Prior to PHP 7.2,
is_object()
on such object returned false, showing
how this is some kind of special case.
The proposed deprecation would affect:
- Direct instantiation:
new __PHP_Incomplete_Class()
- Cloning operations:
clone $incompleteClassInstance
Regarding the impact this would have, a search for “new
__PHP_Incomplete_Class” on GitHub seems to show that it is mostly used
in Rector rules targeting PHP 7.2. It therefore seems to me to be
minimal and worthwhile from the point of view of the robustness of the
language. Adding "language: PHP" shows even less results, apart from
test files from php-src.
If this proposal receives interest, I would be happy to implement the
deprecation warnings in the engine and prepare an RFC if required.
Unless the change is actually uncontroversial? It seems to me, but I'm
looking forward to your thoughts and feedback.
Best,
Alexandre Daubois
Hello Internals,
I would like to gather feedback on the proposal to deprecate userland
creation and cloning of __PHP_Incomplete_Class objects.Here are the reasons I believe this deprecation would be beneficial:
- __PHP_Incomplete_Class is intended to be an internal mechanism for
handling incomplete object unserialization, not for direct userland
manipulation;
This is probably a misunderstanding/inaccuracy: Not __PHP_Incomplete_Class is an (intended or otherwise) mechanism. It is a class (thin by definition) that is used as collaborator in that (globally accessible) mechanism.
The class name is reserved for PHP, because it starts with two underscore characters.
The class is final (PHP 8):
$ php --rc __PHP_Incomplete_Class
Class [ <internal:standard> final class __PHP_Incomplete_Class ] {
- Constants [0] {
}
- Static properties [0] {
}
- Static methods [0] {
}
- Properties [0] {
}
- Methods [0] {
}
}
Additionally I would say, that it is total for manipulation, as the problem is that the original class was not found (a time of deserialization), and the user needs to be able to deal with that, therefore it is in use for manipulation, and then it is in itself a manipulation (in the reviver while deserializing, used as a stand-in; my understanding.)
- This change would align with PHP's general direction of preventing
misuse of internal classes;
Would you please be so kind and give 2-3 examples of misuse of that internal class and 2-3 of that with other internal classes for visibility?
I have to admit, I live under the boring imagination that classes are being used, and it does not fuel my fantasy enough how classes could be misused.
- Classes like Generator already prevent userland instantiation,
making this change consistent with existing patterns;
This is probably a misunderstanding/inaccuracy: The behaviour of the Generator class that you describe is not because it is an internal class or follows a pattern, but its runtime properties require that. Instantiation remains public from user land, by writing a function that has the yield keyword in it's body (before, after or without the return statement) and invoking it. Otherwise a Generator would not work.
For comparison: __PHP_Incomplete_Class is just a class that is occasionally used by PHP deserialization. There is absolutely no problem with language parity in allowing this class, and I don't see why __PHP_Incomplete_Class should require special treatment (e.g., at runtime).
If there had been a problem with it, I believe it would have been fixed in good faith. Unfortunately, I don't see any specific changes in your proposal to verify this.
- No added-value on using this class in userland, stdClass does
already the job if one wants to manipulate "incomplete" classes.
I'm not exactly the most creative person on the boat at this time of day, but writing a PHP library that deserializes and can't find the class name to mimic the behavior expected by PHP is, in my opinion, not “no added value,” but a preliminary assessment of the code, the use of the language, and its interfaces by someone else.
Having stdClass and __PHP_Incomplete_Class could be misleading;
Please explain by example.
- Prior to PHP 7.2,
is_object()
on such object returned false, showing
how this is some kind of special case.
Perhaps this is not relevant at all, but I'd shared the reference to the tracker of that.
This would have allowed me to see what this "some kind of special case" actually is/was. It looks like a bug to me that was fixed in 7.2, maybe some cleanup in Zend.
The proposed deprecation would affect:
- Direct instantiation:
new __PHP_Incomplete_Class()
Why should instantiating with the new keyword not be allowed any longer? What is the alternative suggestion and what is its benefit?
- Cloning operations:
clone $incompleteClassInstance
Why should cloning not be allowed any longer? What should I do instead of cloning, e.g. when I need to deep clone an unserialized object tree that contains it?
Regarding the impact this would have, a search for “new
__PHP_Incomplete_Class” on GitHub seems to show that it is mostly used
in Rector rules targeting PHP 7.2. It therefore seems to me to be
minimal and worthwhile from the point of view of the robustness of the
language.
How would your proposed changes make the language more robust?
Which robustness principles do you see currently being violated and how does your suggestion restore or lift them?
Adding "language: PHP" shows even less results, apart from
test files from php-src.If this proposal receives interest, I would be happy to implement the
deprecation warnings in the engine and prepare an RFC if required.
Unless the change is actually uncontroversial? It seems to me, but I'm
looking forward to your thoughts and feedback.Best,
Alexandre Daubois
Comments/asks
-- hakre