Fix #9997 dynamic properties on SimpleXmlElement #10049

ygottschalk · 2023-07-25T12:59:47Z

Refs #9997
Types dynamic property access on SimpleXmlElement to SimpleXmlElement|null.

BC impact:
SimpleXmlElement is not considered an universal object crate anymore. Therefore things like $xmlElement->myExtraStringData = 'fooBar'; will not work anymore when this get merged.

I do not know why this code was in place and I am not sure if my changes are 'legit': https://github.com/vimeo/psalm/pull/10049/files#diff-a3219413916e4af22971b07d3ef169950d3b17b7cf58f48366c408b1d49ade5bR326
I looked through Atomic.php and some other file and it seemed like null checks (in most places) are done by checking $atomic instanceof TNull and not considering TNamedObject('null') as null. Thats why I removed the code...

Extra info copied from commit message

make SimpleXmlElement and SimpleXmlIterator not a universal object crate
added typed magic __get method to SimpleXmlElement
adjusted tests to reflect new behaviour of SimpleXmlElement
added tests
'fixed' code translating null to TNamedObject('null') instead of TNull

orklah · 2023-07-27T16:57:25Z

src/Psalm/Type/Atomic.php

-                }
-
-                return new TNamedObject($value);
+                return new TNull();


This change will prevent this to work in PHP <7: https://3v4l.org/nbaYr

orklah · 2023-07-27T16:58:25Z

aside from the change in Atomic, this seems good :)

- make SimpleXmlElement and SimpleXmlIterator not a universal object crate - added typed magic __get method to SimpleXmlElement - adjusted tests to reflect new behaviour of SimpleXmlElement - added tests

ygottschalk · 2023-08-05T15:13:21Z

src/Psalm/Type/Atomic.php

@@ -323,7 +323,7 @@ private static function createInner(
                return $analysis_php_version_id !== null ? new TNamedObject($value) : new TScalar();

            case 'null':
-                if ($analysis_php_version_id === null || $analysis_php_version_id >= 8_00_00) {
+                if ($analysis_php_version_id === null || $analysis_php_version_id >= 7_00_00) {


Then lets use this code in PHP<7

ygottschalk · 2023-08-05T15:17:22Z

@orklah ready to merge. psl fail seems unrelated

orklah · 2023-08-05T23:32:16Z

Thanks!

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [google/protobuf](https://developers.google.com/protocol-buffers/) ([source](https://github.com/protocolbuffers/protobuf-php)) | require | minor | `3.23.4` -> `3.24.1` | | [grpc/grpc](https://grpc.io) ([source](https://github.com/grpc/grpc-php)) | require | minor | `1.52.0` -> `1.57.0` | | [phpstan/phpstan](https://github.com/phpstan/phpstan) | require-dev | patch | `1.10.27` -> `1.10.29` | | [phpunit/phpunit](https://phpunit.de/) ([source](https://github.com/sebastianbergmann/phpunit)) | require-dev | patch | `10.3.1` -> `10.3.2` | | [vimeo/psalm](https://github.com/vimeo/psalm) | require-dev | minor | `5.14.1` -> `5.15.0` | --- ### Release Notes <details> <summary>protocolbuffers/protobuf-php (google/protobuf)</summary> ### [`v3.24.1`](https://github.com/protocolbuffers/protobuf-php/compare/v3.24.0...v3.24.1) [Compare Source](https://github.com/protocolbuffers/protobuf-php/compare/v3.24.0...v3.24.1) ### [`v3.24.0`](https://github.com/protocolbuffers/protobuf-php/compare/v3.23.4...v3.24.0) [Compare Source](https://github.com/protocolbuffers/protobuf-php/compare/v3.23.4...v3.24.0) </details> <details> <summary>grpc/grpc-php (grpc/grpc)</summary> ### [`v1.57.0`](https://github.com/grpc/grpc-php/compare/v1.52.0...v1.57.0) [Compare Source](https://github.com/grpc/grpc-php/compare/v1.52.0...v1.57.0) </details> <details> <summary>phpstan/phpstan (phpstan/phpstan)</summary> ### [`v1.10.29`](https://github.com/phpstan/phpstan/releases/tag/1.10.29) [Compare Source](https://github.com/phpstan/phpstan/compare/1.10.28...1.10.29) # Improvements 🔧 - Update nikic/php-parser to v4.17.1 - PHP 8.3 features are no longer parse errors. Full-fledged PHP 8.3 support is coming later this year. - Update BetterReflection to 6.12.0 # Bugfixes 🐛 - PHPStan Pro - when launching, `PHP_BINARY` needs to be escaped (phpstan/phpstan-src@2c7cfd8) # Function signature fixes 🤖 - Fix FTP-related function signatures ([#2551](https://github.com/phpstan/phpstan-src/pull/2551)), thanks [@thg2k](https://github.com/thg2k)! ### [`v1.10.28`](https://github.com/phpstan/phpstan/releases/tag/1.10.28) [Compare Source](https://github.com/phpstan/phpstan/compare/1.10.27...1.10.28) # Improvements 🔧 - Update BetterReflection to 6.12.0 (phpstan/phpstan-src@7c49c94) # Bugfixes 🐛 - Nullsafe operator on `null` results in `null` (phpstan/phpstan-src@5c40c85), [#9721](https://github.com/phpstan/phpstan/issues/9721) - Properties set in the native constructor are initialized in additional constructors (phpstan/phpstan-src@1b0c6a0), [#9619](https://github.com/phpstan/phpstan/issues/9619) - Fix performance problem with nested BooleanOr and BooleanAnd (phpstan/phpstan-src@9adae6c), [#9690](https://github.com/phpstan/phpstan/issues/9690), [#9676](https://github.com/phpstan/phpstan/issues/9676) - CallableTypeHelper - copy variadic parameters if the accepting closure has more parameters (phpstan/phpstan-src@31ed326), [#9699](https://github.com/phpstan/phpstan/issues/9699) # Internals 🔍 - Simplify access to ClassReflection in ClassPropertyNode ([#2565](https://github.com/phpstan/phpstan-src/pull/2565)), thanks [@mad-briller](https://github.com/mad-briller)! </details> <details> <summary>sebastianbergmann/phpunit (phpunit/phpunit)</summary> ### [`v10.3.2`](https://github.com/sebastianbergmann/phpunit/compare/10.3.1...10.3.2) [Compare Source](https://github.com/sebastianbergmann/phpunit/compare/10.3.1...10.3.2) </details> <details> <summary>vimeo/psalm (vimeo/psalm)</summary> ### [`v5.15.0`](https://github.com/vimeo/psalm/releases/tag/5.15.0) [Compare Source](https://github.com/vimeo/psalm/compare/5.14.1...5.15.0)  #### What's Changed ##### Features - Check if argument passed to isset() is not a variable by [@Nitamet](https://github.com/Nitamet) in [https://github.com/vimeo/psalm/pull/10068](https://github.com/vimeo/psalm/pull/10068) - Fix [#9997](https://github.com/vimeo/psalm/issues/9997) dynamic properties on SimpleXmlElement by [@ygottschalk](https://github.com/ygottschalk) in [https://github.com/vimeo/psalm/pull/10049](https://github.com/vimeo/psalm/pull/10049) - Nicer PHP version checking by [@Nitamet](https://github.com/Nitamet) in [https://github.com/vimeo/psalm/pull/10129](https://github.com/vimeo/psalm/pull/10129) ##### Fixes - Fixed `DOMDocument::load*` signatures by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10073](https://github.com/vimeo/psalm/pull/10073) - make (s)printf error reporting more correct/literal by [@kkmuffme](https://github.com/kkmuffme) in [https://github.com/vimeo/psalm/pull/10088](https://github.com/vimeo/psalm/pull/10088) - Fix parameter having `object` type in PHPDoc and only `&` in the method's definition, used with code having an object calling it's method with itself as an argument for mentioned parameter by [@kubawerlos](https://github.com/kubawerlos) in [https://github.com/vimeo/psalm/pull/10104](https://github.com/vimeo/psalm/pull/10104) - mysqli_field_seek returns true by [@kamil-tekiela](https://github.com/kamil-tekiela) in [https://github.com/vimeo/psalm/pull/10107](https://github.com/vimeo/psalm/pull/10107) - Fix bcdiv nullable scale stub by [@TheDevick](https://github.com/TheDevick) in [https://github.com/vimeo/psalm/pull/10106](https://github.com/vimeo/psalm/pull/10106) - Cleanup `test` command and `fixAll` action by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10108](https://github.com/vimeo/psalm/pull/10108) - Correct type for `$enum->name` by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10109](https://github.com/vimeo/psalm/pull/10109) - argv and argc were inconsistent in Context with VariableFetchAnalyzer by [@kkmuffme](https://github.com/kkmuffme) in [https://github.com/vimeo/psalm/pull/10093](https://github.com/vimeo/psalm/pull/10093) - Fix `hash_pbkdf2` `$options` parameter by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10110](https://github.com/vimeo/psalm/pull/10110) - Forbid faulty `nikic/php-parser` version by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10112](https://github.com/vimeo/psalm/pull/10112) - `strip_tags()/$allowed_tags` can accept arrays since 7.4 by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10122](https://github.com/vimeo/psalm/pull/10122) - Fix crash when assertion array is not a list by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10123](https://github.com/vimeo/psalm/pull/10123) - Fix crash on array access to undefined class by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10134](https://github.com/vimeo/psalm/pull/10134) - Correct exit code for invalid version format by [@weirdan](https://github.com/weirdan) in [https://github.com/vimeo/psalm/pull/10135](https://github.com/vimeo/psalm/pull/10135) #### New Contributors - [@Nitamet](https://github.com/Nitamet) made their first contribution in [https://github.com/vimeo/psalm/pull/10068](https://github.com/vimeo/psalm/pull/10068) - [@TheDevick](https://github.com/TheDevick) made their first contribution in [https://github.com/vimeo/psalm/pull/10106](https://github.com/vimeo/psalm/pull/10106) **Full Changelog**: vimeo/psalm@5.14.1...5.15.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 4am on Monday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/cerbos/cerbos-sdk-php).  Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Signed-off-by: Oğuzhan Durgun <oguzhandurgun95@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

ygottschalk force-pushed the fix/9997-simplexamlelment-dynamic-properties branch 3 times, most recently from 3cc8503 to 2257d7e Compare July 25, 2023 13:37

ygottschalk changed the title ~~Fix #9997~~ Fix #9997 dynamic properties on SimpleXmlElement Jul 25, 2023

ygottschalk marked this pull request as ready for review July 25, 2023 13:49

orklah reviewed Jul 27, 2023

View reviewed changes

orklah added the release:feature The PR will be included in 'Features' section of the release notes label Jul 27, 2023

ygottschalk force-pushed the fix/9997-simplexamlelment-dynamic-properties branch from 2257d7e to 33db549 Compare August 5, 2023 13:40

ygottschalk changed the base branch from master to 5.x August 5, 2023 13:41

ygottschalk marked this pull request as draft August 5, 2023 13:43

ygottschalk force-pushed the fix/9997-simplexamlelment-dynamic-properties branch from 33db549 to 9b77760 Compare August 5, 2023 13:59

Fix vimeo#9997

a4de6d9

- make SimpleXmlElement and SimpleXmlIterator not a universal object crate - added typed magic __get method to SimpleXmlElement - adjusted tests to reflect new behaviour of SimpleXmlElement - added tests

ygottschalk force-pushed the fix/9997-simplexamlelment-dynamic-properties branch from 9b77760 to a4de6d9 Compare August 5, 2023 15:07

ygottschalk commented Aug 5, 2023

View reviewed changes

ygottschalk marked this pull request as ready for review August 5, 2023 15:13

orklah merged commit 3098e51 into vimeo:5.x Aug 5, 2023
47 checks passed

edsrzf mentioned this pull request Sep 14, 2023

SimpleXMLElement regressions on Psalm 5.15 #10204

Closed

kthaler mentioned this pull request Jan 9, 2024

Add SimpleXMLElement::__set() method handling #10535

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #9997 dynamic properties on SimpleXmlElement #10049

Fix #9997 dynamic properties on SimpleXmlElement #10049

ygottschalk commented Jul 25, 2023 •

edited

Loading

orklah Jul 27, 2023

orklah commented Jul 27, 2023

ygottschalk Aug 5, 2023

ygottschalk commented Aug 5, 2023

orklah commented Aug 5, 2023

Fix #9997 dynamic properties on SimpleXmlElement #10049

Fix #9997 dynamic properties on SimpleXmlElement #10049

Conversation

ygottschalk commented Jul 25, 2023 • edited Loading

orklah Jul 27, 2023

Choose a reason for hiding this comment

orklah commented Jul 27, 2023

ygottschalk Aug 5, 2023

Choose a reason for hiding this comment

ygottschalk commented Aug 5, 2023

orklah commented Aug 5, 2023

ygottschalk commented Jul 25, 2023 •

edited

Loading