Replace @deprecated by #[\Deprecated] for internal functions / class constants
#14750
Conversation
4045cf3 to
4250ee3
Compare
|
In the previous PR there were some unresolved threads from @Girgias remaining (regarding version information / messages). And there was a discussion regarding the formatting of the version within the |
ext/libxml/libxml.stub.php
Outdated
| @@ -172,7 +172,7 @@ function libxml_get_errors(): array {} | |||
|
|
|||
| function libxml_clear_errors(): void {} | |||
|
|
|||
| /** @deprecated */ | |||
| #[\Deprecated(since: '8.0')] | |||
There was a problem hiding this comment.
reason: because entity loading is disabled by default since PHP 8.0.
There was a problem hiding this comment.
@nielsdos Would libxml_disable_entity_loader(false) reenable it or not?
There was a problem hiding this comment.
Yes but only when also passing LIBXML_NOENT to the parsing options of the XML extensions.
There was a problem hiding this comment.
I understand that external entities are dangerous, but the deprecation without any alternative sounds wrong.
Anyway: Do you have a specific wording suggestion here? Best to suggest it using GitHub's suggestion feature.
There was a problem hiding this comment.
but the deprecation without any alternative sounds wrong.
You can still use them though, this was necessary to combat shit defaults in libxml2 < v2.9. Since PHP 8.0 we require at least libxml2 v2.9 and so the problem went away.
There was a problem hiding this comment.
| #[\Deprecated(since: '8.0')] | |
| #[\Deprecated(message: 'External entity loading is disabled by default', since: '8.0')] |
There was a problem hiding this comment.
You can still use them though.
I understand this is not the correct place for this discussion, but does it require the use of this deprecated function to enable them or will this happen automatically when calling libxml_set_external_entity_loader() or so? If the former, then this doesn't really feel like still being able to use them.
Regarding the change: Thanks, applied (with s/External/as external/ to better fit the grammar).
There was a problem hiding this comment.
I understand this is not the correct place for this discussion, but does it require the use of this deprecated function to enable them
No you don't have to use this. You have to pass the LIBXML_NOENT option to an XML parser. libxml_set_external_entity_loader() is orthogonal to this
5fe18aa to
3110205
Compare
Thanks, all applied and then slightly adjusted for formatting:
|
7620042 to
1c1c6ce
Compare
|
@Girgias I just did a rebase of the stuff you already reviewed and pushed some more commits to add the missing data / to correct some data. All deprecations have a |
| @@ -12,12 +12,12 @@ zip_close($zip); | |||
|
|
|||
| ?> | |||
| --EXPECTF-- | |||
| Deprecated: Function zip_open() is deprecated in %s on line %d | |||
| Deprecated: Function zip_open() is deprecated since 8.0, use ZipArchive::open() instead in %s on line %d | |||
There was a problem hiding this comment.
Okay this is a complete aside, but I just realized that in the documentation, our manual style guide requires use to say "deprecated as of PHP 8.0" instead of "since", not sure if this is something we should match or not, but that's orthogonal to this PR.
There was a problem hiding this comment.
I think keeping consistency with the property name is valuable here.
…ass constants Co-authored-by: Gina Peter Banyard <girgias@php.net> Co-authored-by: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
e9f571b to
67bb216
Compare
This is spun out of and depends on #11293.