Fix #9997 dynamic properties on SimpleXmlElement by ygottschalk · Pull Request #10049 · vimeo/psalm

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


            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://togithub.com/protocolbuffers/protobuf-php)) | require | minor | `3.23.4` -> `3.24.1` | | [grpc/grpc](https://grpc.io) ([source](https://togithub.com/grpc/grpc-php)) | require | minor | `1.52.0` -> `1.57.0` | | [phpstan/phpstan](https://togithub.com/phpstan/phpstan) | require-dev | patch | `1.10.27` -> `1.10.29` | | [phpunit/phpunit](https://phpunit.de/) ([source](https://togithub.com/sebastianbergmann/phpunit)) | require-dev | patch | `10.3.1` -> `10.3.2` | | [vimeo/psalm](https://togithub.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://togithub.com/protocolbuffers/protobuf-php/compare/v3.24.0...v3.24.1) [Compare Source](https://togithub.com/protocolbuffers/protobuf-php/compare/v3.24.0...v3.24.1) ### [`v3.24.0`](https://togithub.com/protocolbuffers/protobuf-php/compare/v3.23.4...v3.24.0) [Compare Source](https://togithub.com/protocolbuffers/protobuf-php/compare/v3.23.4...v3.24.0) </details> <details> <summary>grpc/grpc-php (grpc/grpc)</summary> ### [`v1.57.0`](https://togithub.com/grpc/grpc-php/compare/v1.52.0...v1.57.0) [Compare Source](https://togithub.com/grpc/grpc-php/compare/v1.52.0...v1.57.0) </details> <details> <summary>phpstan/phpstan (phpstan/phpstan)</summary> ### [`v1.10.29`](https://togithub.com/phpstan/phpstan/releases/tag/1.10.29) [Compare Source](https://togithub.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://togithub.com/phpstan/phpstan-src/pull/2551)), thanks [@thg2k](https://togithub.com/thg2k)! ### [`v1.10.28`](https://togithub.com/phpstan/phpstan/releases/tag/1.10.28) [Compare Source](https://togithub.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://togithub.com/phpstan/phpstan/issues/9721) - Properties set in the native constructor are initialized in additional constructors (phpstan/phpstan-src@1b0c6a0), [#9619](https://togithub.com/phpstan/phpstan/issues/9619) - Fix performance problem with nested BooleanOr and BooleanAnd (phpstan/phpstan-src@9adae6c), [#9690](https://togithub.com/phpstan/phpstan/issues/9690), [#9676](https://togithub.com/phpstan/phpstan/issues/9676) - CallableTypeHelper - copy variadic parameters if the accepting closure has more parameters (phpstan/phpstan-src@31ed326), [#9699](https://togithub.com/phpstan/phpstan/issues/9699) # Internals 🔍 - Simplify access to ClassReflection in ClassPropertyNode ([#2565](https://togithub.com/phpstan/phpstan-src/pull/2565)), thanks [@mad-briller](https://togithub.com/mad-briller)! </details> <details> <summary>sebastianbergmann/phpunit (phpunit/phpunit)</summary> ### [`v10.3.2`](https://togithub.com/sebastianbergmann/phpunit/compare/10.3.1...10.3.2) [Compare Source](https://togithub.com/sebastianbergmann/phpunit/compare/10.3.1...10.3.2) </details> <details> <summary>vimeo/psalm (vimeo/psalm)</summary> ### [`v5.15.0`](https://togithub.com/vimeo/psalm/releases/tag/5.15.0) [Compare Source](https://togithub.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://togithub.com/Nitamet) in [https://github.com/vimeo/psalm/pull/10068](https://togithub.com/vimeo/psalm/pull/10068) - Fix [#9997](https://togithub.com/vimeo/psalm/issues/9997) dynamic properties on SimpleXmlElement by [@ygottschalk](https://togithub.com/ygottschalk) in [https://github.com/vimeo/psalm/pull/10049](https://togithub.com/vimeo/psalm/pull/10049) - Nicer PHP version checking by [@Nitamet](https://togithub.com/Nitamet) in [https://github.com/vimeo/psalm/pull/10129](https://togithub.com/vimeo/psalm/pull/10129) ##### Fixes - Fixed `DOMDocument::load*` signatures by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10073](https://togithub.com/vimeo/psalm/pull/10073) - make (s)printf error reporting more correct/literal by [@kkmuffme](https://togithub.com/kkmuffme) in [https://github.com/vimeo/psalm/pull/10088](https://togithub.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://togithub.com/kubawerlos) in [https://github.com/vimeo/psalm/pull/10104](https://togithub.com/vimeo/psalm/pull/10104) - mysqli_field_seek returns true by [@kamil-tekiela](https://togithub.com/kamil-tekiela) in [https://github.com/vimeo/psalm/pull/10107](https://togithub.com/vimeo/psalm/pull/10107) - Fix bcdiv nullable scale stub by [@TheDevick](https://togithub.com/TheDevick) in [https://github.com/vimeo/psalm/pull/10106](https://togithub.com/vimeo/psalm/pull/10106) - Cleanup `test` command and `fixAll` action by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10108](https://togithub.com/vimeo/psalm/pull/10108) - Correct type for `$enum->name` by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10109](https://togithub.com/vimeo/psalm/pull/10109) - argv and argc were inconsistent in Context with VariableFetchAnalyzer by [@kkmuffme](https://togithub.com/kkmuffme) in [https://github.com/vimeo/psalm/pull/10093](https://togithub.com/vimeo/psalm/pull/10093) - Fix `hash_pbkdf2` `$options` parameter by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10110](https://togithub.com/vimeo/psalm/pull/10110) - Forbid faulty `nikic/php-parser` version by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10112](https://togithub.com/vimeo/psalm/pull/10112) - `strip_tags()/$allowed_tags` can accept arrays since 7.4 by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10122](https://togithub.com/vimeo/psalm/pull/10122) - Fix crash when assertion array is not a list by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10123](https://togithub.com/vimeo/psalm/pull/10123) - Fix crash on array access to undefined class by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10134](https://togithub.com/vimeo/psalm/pull/10134) - Correct exit code for invalid version format by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10135](https://togithub.com/vimeo/psalm/pull/10135) #### New Contributors - [@Nitamet](https://togithub.com/Nitamet) made their first contribution in [https://github.com/vimeo/psalm/pull/10068](https://togithub.com/vimeo/psalm/pull/10068) - [@TheDevick](https://togithub.com/TheDevick) made their first contribution in [https://github.com/vimeo/psalm/pull/10106](https://togithub.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://togithub.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>

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [vimeo/psalm](https://togithub.com/vimeo/psalm) | require-dev | minor | `~5.14.0` -> `~5.15.0` | --- ### Release Notes <details> <summary>vimeo/psalm (vimeo/psalm)</summary> ### [`v5.15.0`](https://togithub.com/vimeo/psalm/releases/tag/5.15.0) [Compare Source](https://togithub.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://togithub.com/Nitamet) in [https://github.com/vimeo/psalm/pull/10068](https://togithub.com/vimeo/psalm/pull/10068) - Fix [#9997](https://togithub.com/vimeo/psalm/issues/9997) dynamic properties on SimpleXmlElement by [@ygottschalk](https://togithub.com/ygottschalk) in [https://github.com/vimeo/psalm/pull/10049](https://togithub.com/vimeo/psalm/pull/10049) - Nicer PHP version checking by [@Nitamet](https://togithub.com/Nitamet) in [https://github.com/vimeo/psalm/pull/10129](https://togithub.com/vimeo/psalm/pull/10129) ##### Fixes - Fixed `DOMDocument::load*` signatures by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10073](https://togithub.com/vimeo/psalm/pull/10073) - make (s)printf error reporting more correct/literal by [@kkmuffme](https://togithub.com/kkmuffme) in [https://github.com/vimeo/psalm/pull/10088](https://togithub.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://togithub.com/kubawerlos) in [https://github.com/vimeo/psalm/pull/10104](https://togithub.com/vimeo/psalm/pull/10104) - mysqli_field_seek returns true by [@kamil-tekiela](https://togithub.com/kamil-tekiela) in [https://github.com/vimeo/psalm/pull/10107](https://togithub.com/vimeo/psalm/pull/10107) - Fix bcdiv nullable scale stub by [@TheDevick](https://togithub.com/TheDevick) in [https://github.com/vimeo/psalm/pull/10106](https://togithub.com/vimeo/psalm/pull/10106) - Cleanup `test` command and `fixAll` action by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10108](https://togithub.com/vimeo/psalm/pull/10108) - Correct type for `$enum->name` by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10109](https://togithub.com/vimeo/psalm/pull/10109) - argv and argc were inconsistent in Context with VariableFetchAnalyzer by [@kkmuffme](https://togithub.com/kkmuffme) in [https://github.com/vimeo/psalm/pull/10093](https://togithub.com/vimeo/psalm/pull/10093) - Fix `hash_pbkdf2` `$options` parameter by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10110](https://togithub.com/vimeo/psalm/pull/10110) - Forbid faulty `nikic/php-parser` version by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10112](https://togithub.com/vimeo/psalm/pull/10112) - `strip_tags()/$allowed_tags` can accept arrays since 7.4 by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10122](https://togithub.com/vimeo/psalm/pull/10122) - Fix crash when assertion array is not a list by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10123](https://togithub.com/vimeo/psalm/pull/10123) - Fix crash on array access to undefined class by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10134](https://togithub.com/vimeo/psalm/pull/10134) - Correct exit code for invalid version format by [@weirdan](https://togithub.com/weirdan) in [https://github.com/vimeo/psalm/pull/10135](https://togithub.com/vimeo/psalm/pull/10135) #### New Contributors - [@Nitamet](https://togithub.com/Nitamet) made their first contribution in [https://github.com/vimeo/psalm/pull/10068](https://togithub.com/vimeo/psalm/pull/10068) - [@TheDevick](https://togithub.com/TheDevick) made their first contribution in [https://github.com/vimeo/psalm/pull/10106](https://togithub.com/vimeo/psalm/pull/10106) **Full Changelog**: vimeo/psalm@5.14.1...5.15.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), 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. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] 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/open-feature/php-sdk).  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

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
orklah merged 1 commit intovimeo:5.xfrom
ygottschalk:fix/9997-simplexamlelment-dynamic-properties

ygottschalk commented Jul 25, 2023 •

edited

Loading

Uh oh!

orklah Jul 27, 2023

Uh oh!

orklah commented Jul 27, 2023

Uh oh!

ygottschalk Aug 5, 2023

Uh oh!

ygottschalk commented Aug 5, 2023

Uh oh!

orklah commented Aug 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ygottschalk commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orklah Jul 27, 2023

Choose a reason for hiding this comment

Uh oh!

orklah commented Jul 27, 2023

Uh oh!

ygottschalk Aug 5, 2023

Choose a reason for hiding this comment

Uh oh!

ygottschalk commented Aug 5, 2023

Uh oh!

orklah commented Aug 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ygottschalk commented Jul 25, 2023 •

edited

Loading