Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated signatures for "XML parser" #3691

Closed
wants to merge 20 commits into from
Closed

Conversation

sreichel
Copy link
Contributor

Updated all signatures according to https://www.php.net/manual/en/function.xml-error-string.php

(hope i did it right with deltas)

@ondrejmirtes
Copy link
Member

Hi, can you show on phpstan.org/try which bugs this fixes? I feel like we don't need most of the changes as PHPStan will already report them correctly, mostly thanks to https://github.com/phpstan/php-8-stubs.

@sreichel
Copy link
Contributor Author

Hi,

e.g. https://phpstan.org/r/84e5838e-96ed-4111-b332-1424bd6ad513

Stubs look wrong to me ... (?)

All changes are ...

  • resource: PHP8+ - "resource" -> "XMLParser"
  • handler: all PHP - string is allowed as handler (next to callable)
  • handler: PHP8.4 - null is allowed

@ondrejmirtes
Copy link
Member

As I said, most of these are taken care of thanks to php-8-stubs.

Please submit only the changes from callable to callable|string, those make sense.

@ondrejmirtes
Copy link
Member

But the "passing string" is being deprecated on 8.4+, so I'm not sure about it:

Passing a non-callable string to handler is now deprecated, use a proper callable for methods, or null to reset the handler.

@ondrejmirtes
Copy link
Member

So it's better to not allow that either.

@sreichel
Copy link
Contributor Author

sreichel commented Nov 29, 2024

Mhhh, we are running php7.4 ... not 8.4.

I have read that it will be deprecated, but phpstan also supports older versions. Not?

So i have to baseline that?

@ondrejmirtes
Copy link
Member

It supports older versions, but you can also fix the deprecated code even before you upgrade to newer version. You eventually want to upgrade from an unsupported version, right :) Having PHPStan report the error forces you to get ready for a newer PHP version sooner.

@ondrejmirtes
Copy link
Member

Normally I'm fixing these errors, but after all these years no one complained about string not being accepted there so I don't think it makes sense to fix that now.

@sreichel
Copy link
Contributor Author

Thanks.

Tried to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants