On 24/04/2024 10:22, Claude Pache wrote:
>
>
>> Le 23 avr. 2024 à 21:23, Niels Dossche <dossche.ni...@gmail.com> a écrit :
>>
>> On 22/04/2024 21:53, Larry Garfield wrote:
>>> On Mon, Apr 22, 2024, at 6:41 PM, Niels Dossche wrote:
>>>> Hi internals
>>>>
>>>> I'm opening the discussion for my RFC "Add openStream() to
>>>> XML{Reader,Writer}".
>>>> RFC link: https://wiki.php.net/rfc/xmlreader_writer_streams
>>>>
>>>> Kind regards
>>>> Niels
>>>
>>> This seems quite reasonable to me overall. My one question is regarding
>>> the writer version. Why is that not a static method, too? I would have
>>> expected that to be a "named constructor" just like the reader.
>>>
>>> --Larry Garfield
>>
>> Hi Larry
>>
>> XMLReader already had these static methods that act as named constructors,
>> but XMLWriter has no named constructors at the moment.
>> The XMLWriter::openMemory() and XMLWriter::openUri() functions are instance
>> methods that must be called after doing "new XMLWriter".
>> If these two existing functions were static methods instead, I would've made
>> XMLWriter::openStream() static too.
>> So IOW, for consistency I followed the model of the existing XMLWriter
>> methods.
>>
>> While it is possible to do the magic trick that XMLReader uses to have the
>> open methods on XMLWriter both static and non-static, this is quite hacky
>> and was only done to XMLReader for BC reasons.
>
> That’s odd. The inconsistency was introduced (or at least sanctioned) in PHP
> 8.0. In PHP 7, XMLReader::open() and XMLReader::XML() already worked when
> used both as static and non-static methods, but triggered a deprecation
> warning when called statically. The deprecation warning was removed in 8.0,
> regardless of the differing semantics when called statically and
> non-statically, and regardless of the the inconsistency with corresponding
> XMLWriter methods.
>
> Another point: when called statically on a subclass, both `XMLReader::open()`
> and `XMLReader::XML()` return an object of type `XMLReader`, not of the
> subclass: https://3v4l.org/lCOAvJ <https://3v4l.org/lCOAvJ>
> For that reason, they are unusable as static methods on a subclass. The new
> `openStream()` method should work on instances, so that it will be usable on
> subclasses. (And for the same reason, I think it was a mistake to undeprecate
> `XMLReader::{open,XML}()` as static methods in 8.0.)
This is a good point indeed, good catch!
I agree it should be an instance method then to avoid this problem.
It also makes me wonder why the static form is the canonical form shown in the
manual (https://www.php.net/manual/en/xmlreader.open)... We should probably
show the instance method as the recommended one.
I also checked whether it's possible to return "static" instead of "XMLReader"
for the existing static functions, but that's not possible in the general case
because we won't know with which arguments to call the constructor...
Anyway, I'm going to update the RFC for this.
>
> —Claude
>
Kind regards
Niels
