sebastianbergmann / recursion-context

Provides functionality to recursively process PHP variables
BSD 3-Clause "New" or "Revised" License
6.52k stars 18 forks source link

Variables processed by this component are modified #8

Closed sebastianbergmann closed 7 years ago

sebastianbergmann commented 7 years ago

@bwoebi It looks like the SplObjectStorage instance that is used by the SebastianBergmann\RecursionContext\Context class "leaks" into the variable that is passed to add(). This probably happens in https://github.com/sebastianbergmann/recursion-context/blob/master/src/Context.php#L112.

Variables processed using this component must not be modified by it.

sebastianbergmann commented 7 years ago

Moved here from https://github.com/sebastianbergmann/phpunit/issues/2361

bwoebi commented 7 years ago

This is no bug, you just need to not access the value between add() and __destruct().

Make a copy (like in https://github.com/sebastianbergmann/exporter/pull/17) before passing to add() and it's all fine.

The mistake was to tag this as a micro version instead of a major version (as semantics changed).

sebastianbergmann commented 7 years ago

The tags have been deleted and the releases have been removed from Packagist. I'll try to sort this out on Sunday.

bwoebi commented 7 years ago

I'm also sorry for the confusion I caused this week and missing to update all deps in time too :-/

sebastianbergmann commented 7 years ago

No worries. New releases of the affected packages have been made. As of https://github.com/sebastianbergmann/phpunit/commit/a7ffe99c8d24b4f9a2bf08f8df04f0b316ad6110 PHPUnit 5.6 will pull in those new versions.

bwoebi commented 7 years ago

It shouldn't be needed to release a new major for the consumers of recursion-context (like exporter) - there a micro version bump (which accepts ^1||^2) would've been fine after my patch, as the semantics of their API didn't change - just the recursion-context.

sebastianbergmann commented 7 years ago

Maybe, but it's done now. For PHPUnit it will be 5.6.5, though.