Immutable unions#8627
Conversation
| new TIntRange(0, 8), | ||
| new TNonEmptyList(Type::getInt()), | ||
| ]), | ||
| 'name' => $str, |
There was a problem hiding this comment.
should this change be included in this PR? Feels like it should be added for 4.x?
There was a problem hiding this comment.
I suppose this could be backported as well, just wanted to include it in this PR since I also introduced memoization for global types and wanted to avoid merge conflicts in this section of the VariableFetchAnalyzer.
There was a problem hiding this comment.
Though it would have to be changed a bit, here I re-use the string type for example, that can cause nasty issues in 4.x where unions aren't immutable
|
Seems like phpunit plugin broke :( |
|
Yep, this PR requires the other PR I sent to the phpunit plugin |
|
Can you check there? We'll need to make a dedicated release for the phpunit plugin on version 5 |
|
+, done! |
|
Also fixed the parent_nodes issues. |
|
Not sure why, but composer install 0.16.1 of psalm's phpunit plugin, not 0.18.0... |
|
That's because in semver |
|
Nevermind, already updated ;) |
|
0.18.1 doesn't seem to resolve the issue here |
|
Yeah, maybe it'll work after merging on master? |
|
Oh! New error :) |
|
Yeah, sorry about this, psalm/psalm-plugin-phpunit#126 works for sure, just tested :) |
|
I think this could be merged now. Can you cleanup some commits (or should I squish everything?). Please try to make sure every baseline addition is legit and it should be ok |
|
You can squash-merge, afaik all commit messages are retained when squash-merging, and yes I made sure all the baselined issues are pre-existing psalm bugs :P |
|
Thanks a lot! |
|
YAY :D |
This PR introduces fully immutable union types and some work towards immutable storages (I realized that while that's a good thing to have, the only thing we really need to avoid mutability bugs are immutable unions).
Marking this as draft due to the ~30 psalm issues I have to fix, plus a very nasty (set of) side-effects related to parent_nodes, which I'm currently trying to fix.
Basically, the trick here is to fix https://psalm.dev/r/eb9860a6c5, which is caused by
src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php:428dropping all already-asserted info about$function_call_arg->value->items[0]and children, becausesrc/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php:429re-assigns some types afterAtomicPropertyFetchAnalyzer::processTaints.Technically, the behavior of
IfElseAnalyzeris correct: when changing parent nodes ie due to a variable re-assignment (first part of the psalm.dev example), all known assertions about the variable and children should be dropped.However, in this specific example in InstancePropertyFetchAnalyzer, originally the re-assignment is propagated to parent contexts, and I haven't figured out yet the proper way of replicating this.