[RFC] [Discussion] Add openStream() to XML{Reader,Writer}

Le 23 avr. 2024 à 21:23, Niels Dossche dossche.niels@gmail.com a écrit :

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