Opening this issue to allow discussion of possible breaking changes in psalm v5 to allow the immutability of types stored inside storages.
I've been encountering a good number of parallelism bugs, that are actually execution order bugs caused by corruption of some part of function, method, class or argument storages.
To debug this, in my fork I have "frozen" the contents of all storages after scanning, by recursively rendering immutable all stored Unions, using a flag that triggers exceptions when using mutator methods.
While this has helped me a lot in finding bugs (which led to bugfix PR #8131), another bug I manually found and fixed in #8098 isn't detected by rendering Unions immutable, and is probably caused by a reassignment of the as property in a TTemplate* Atomic type, which is obviously not detected even if the Union in the as itself is also marked immutable.
This is more problematic, and highlights a problem with the struct-like nature of Atomic types: lacking encapsulation, one can't easily detect the re-assignment of properties in Atomic types.
The breaking change I'm proposing to solve this issue (and to help detect misuse through static analysis) is to render all Atomic classes @immutable, and create a separate AtomicMutable class hierarchy which is mutable, with methods to convert between the two, sharing common methods between the two using traits; the same could be done for Unions, using MutableUnions instead.
This would allow statically detecting misuse of types (including in plugins, etc), and even at runtime if we were to require PHP 8.1 for psalm v5 to use readonly properties.
Opening this issue to allow discussion of possible breaking changes in psalm v5 to allow the immutability of types stored inside storages.
I've been encountering a good number of parallelism bugs, that are actually execution order bugs caused by corruption of some part of function, method, class or argument storages.
To debug this, in my fork I have "frozen" the contents of all storages after scanning, by recursively rendering immutable all stored Unions, using a flag that triggers exceptions when using mutator methods.
While this has helped me a lot in finding bugs (which led to bugfix PR #8131), another bug I manually found and fixed in #8098 isn't detected by rendering Unions immutable, and is probably caused by a reassignment of the
asproperty in aTTemplate*Atomic type, which is obviously not detected even if the Union in theasitself is also marked immutable.This is more problematic, and highlights a problem with the struct-like nature of
Atomictypes: lacking encapsulation, one can't easily detect the re-assignment of properties inAtomictypes.The breaking change I'm proposing to solve this issue (and to help detect misuse through static analysis) is to render all
Atomicclasses@immutable, and create a separateAtomicMutableclass hierarchy which is mutable, with methods to convert between the two, sharing common methods between the two using traits; the same could be done for Unions, using MutableUnions instead.This would allow statically detecting misuse of types (including in plugins, etc), and even at runtime if we were to require PHP 8.1 for psalm v5 to use
readonlyproperties.