Skip to content

Allow to call XMLReader::open() and ::XML() statically#4816

Closed
cmb69 wants to merge 1 commit into
php:masterfrom
cmb69:xmlreader-static-2
Closed

Allow to call XMLReader::open() and ::XML() statically#4816
cmb69 wants to merge 1 commit into
php:masterfrom
cmb69:xmlreader-static-2

Conversation

@cmb69

@cmb69 cmb69 commented Oct 11, 2019

Copy link
Copy Markdown
Member

The implementation of XMLReader::open() and XMLReader::XML() still
supports calling the methods statically and non-statically. However,
as of PHP 8.0.0, calling these methods statically is not allowed,
because they are not declared as static methods. Since we consider it
to be cleaner to call these methods statically, but had deprecated to
call them statically, we properly support both variants.


This PR supersedes PR #4680.

After further consideration, I still think it would be good to rid the option to call these methods non-statically, and to also remove XMLReader::close() in the long run, prepared by a deprecation phase. However, as of PHP 7.0.0 calling these methods triggered a deprecation notice, so likely quite some code has already been adjusted to that, so deprecating the non-static calls right away (PHP 8.0.0) would be very annoying for users.

Anyhow, if we stick with the dual calling mechanism for a while (or even forever), we probably should refactor to use zend_parse_method_parameters().

PS: it might be useful to do this change already for PHP 7.4.

The implementation of `XMLReader::open()` and `XMLReader::XML()` still
supports calling the methods statically and non-statically.  However,
as of PHP 8.0.0, calling these methods statically is not allowed,
because they are not declared as static methods.  Since we consider it
to be cleaner to call these methods statically, but had deprecated to
call them statically, we properly support both variants.
@Girgias

Girgias commented Oct 11, 2019

Copy link
Copy Markdown
Member

I would say this change makes more sense in 7.4 but how late we are in the initial QA period it feels a bit odd. Probably need to ask @derickr what he thinks about it.

@derickr

derickr commented Oct 11, 2019

Copy link
Copy Markdown
Member

No more changes in 7.4! :-)

@cmb69

cmb69 commented Oct 21, 2019

Copy link
Copy Markdown
Member Author

Okay, so we should do this for master only. If there are no objections, I'll merge this in a week.

@nikic nikic self-requested a review October 21, 2019 08:30
@cmb69

cmb69 commented Oct 29, 2019

Copy link
Copy Markdown
Member Author

Thanks! Applied as 6468578.

@cmb69 cmb69 closed this Oct 29, 2019
@cmb69 cmb69 deleted the xmlreader-static-2 branch October 29, 2019 14:59
@carusogabriel carusogabriel added this to the PHP 8.0 milestone May 29, 2020
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