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

SimpleXmlElement weirdness #2784

Closed
orklah opened this issue Dec 29, 2019 · 9 comments
Closed

SimpleXmlElement weirdness #2784

orklah opened this issue Dec 29, 2019 · 9 comments
Labels
Milestone

Comments

@orklah
Copy link
Contributor

orklah commented Dec 29, 2019

Bug report

PHPStan thinks every direct call to an attribute on a SimpleXmlElement will return a SimpleXmlElement. This is not always true:
https://phpstan.org/r/f9336943-20db-4b3e-a9ba-671b6f58bd78
https://3v4l.org/kWSfE

While the first attribute does indeed return a SimpleXmlElement(even when the xml doesn't contain this element), this is not true for the second one.

PHPStan should not complain about always true checks in this context.

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Dec 30, 2019
@orklah
Copy link
Contributor Author

orklah commented Dec 30, 2019

@ondrejmirtes you added Easy Fixes on this. Any indication how to proceed? I will try to propose the PR

@ondrejmirtes
Copy link
Member

Relevant code: https://github.com/phpstan/phpstan-src/blob/master/src/Type/Php/SimpleXMLElementClassPropertyReflectionExtension.php, https://github.com/phpstan/phpstan-src/blob/master/src/Reflection/Php/SimpleXMLElementProperty.php. You could do new BenevolentUnionType([new ObjectType('SimpleXMLElement'), new NullType()]) and test how it works.

It's possible that BenevolentUnionType should override hasMethod, getMethod etc. methods so that level 8 does not complain about accessing something on possibly null type.

It's completely valid use-case that people are sure about their XML structure and know some attribute isn't going to be null.

@orklah
Copy link
Contributor Author

orklah commented Dec 30, 2019

Thanks! I will try to look at it with a clear head (so, not tomorrow night! Nor the day after!) soon.

@phpstan-bot
Copy link
Contributor

@orklah PHPStan now reports different result with your code snippet:

@@ @@
+No errors
+
+PHP 7.3 – 7.4 (2 errors)
+==========
+
+6: Call to function assert() with true will always evaluate to true.
+6: Instanceof between SimpleXMLElement and SimpleXMLElement will always evaluate to true.
+
+No errors
+
+PHP 7.1 (2 errors)
+==========
+
 6: Call to function assert() with true will always evaluate to true.
 6: Instanceof between SimpleXMLElement and SimpleXMLElement will always evaluate to true.
Full report

PHP 8.0

No errors

PHP 7.3 – 7.4 (2 errors)

Line Error
6 Instanceof between SimpleXMLElement and SimpleXMLElement will always evaluate to true.
6 Instanceof between SimpleXMLElement and SimpleXMLElement will always evaluate to true.

PHP 7.2

No errors

PHP 7.1 (2 errors)

Line Error
6 Instanceof between SimpleXMLElement and SimpleXMLElement will always evaluate to true.
6 Instanceof between SimpleXMLElement and SimpleXMLElement will always evaluate to true.

@orklah
Copy link
Contributor Author

orklah commented Mar 3, 2021

Oh this is weird, PHP 7.2 should not be different than 7.1, 7.3 and 7.4 here.

Not sure about 8.0 though, something may have change internally.

This issue could be fixed by merging phpstan/phpstan-src#76 however

@ondrejmirtes
Copy link
Member

@orklah Sorry, there's some weird bug with the bot and the playground, I need to look into it.

@phpstan-bot
Copy link
Contributor

@orklah After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
+4: Class SimpleXMLElement referenced with incorrect case: SimpleXmlElement.
 6: Call to function assert() with true will always evaluate to true.
+6: Class SimpleXMLElement referenced with incorrect case: SimpleXmlElement.
 6: Instanceof between SimpleXMLElement and SimpleXMLElement will always evaluate to true.
Full report
Line Error
4 Class SimpleXMLElement referenced with incorrect case: SimpleXmlElement.
6 Call to function assert() with true will always evaluate to true.
6 Class SimpleXMLElement referenced with incorrect case: SimpleXmlElement.
6 Instanceof between SimpleXMLElement and SimpleXMLElement will always evaluate to true.

@ondrejmirtes
Copy link
Member

Fixed by: phpstan/phpstan-src#76

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants