Support pass-by-reference

[malkusch]

I really would love to integrate my php-mock with prophecy so that you could write prophecies about built-in PHP functions. This works already great, unless you want to use pass-by-reference (e.g. exec()).

You do obviously care about references in ClassCodeGenerator::generateArguments(). But they get lost on the further track. ProphecySubjectPatch::apply() uses func_get_args() which unfortunately doesn't preserve references:

$method->setCode(
    'return $this->getProphecy()->makeProphecyMethodCall(__FUNCTION__, func_get_args());'
);

If you don't mind I would prepare a PR which would not eat references in ProphecySubjectPatch::apply().

[stof]

the issue is that this would require using references everywhere in Prophecy (otherwise they would just be eaten one call later), which would then break things in all cases which should not use them (and might create lots of weird issues). And passing arguments by reference when they were not references initially causes a reference mismatch, which forces to duplicate the value, making things use twice the memory and being slower.

So IMO, this won't be supported.

Thus, I consider that reference-passing signatures are very confusing to use for many people. And they can often lead to performance issues due to reference mismatches (references are most of the time used when thinking that they would make things faster by avoiding to copy data, while a reference mismatch actually makes things worse, and this happens even to the Symfony core team). So I would even consider that not being able to prophesize such API is a Prophecy feature rather than a bug (and @everzet might agree with me on this). This goes in line with the PhpSpec way: making it hard to work with badly designed APIs (according to the PhpSpec team definition)

[malkusch]

this would require using references everywhere in Prophecy

It's quiet a time ago when I touched that code, but the fragments which I have lying around here aren't that invasive. It's very much focused around the offending func_get_args(). ~Anyhow, I would do the dirty work and you can just enjoy a further feature ;)~ (Sorry, I'm out. That idea died a few years ago)

which would then break things in all cases which should not use them

I very much hope you trust your test suite to discover that. At least I do.

So IMO, this won't be supported.

Well, that's actually quiet appealing for me, cause it will safe myself some time and I would simply pass that statement further in my library's documentation. Could @everzet confirm that?

I consider that reference-passing signatures are very confusing

Maybe, and so is PHP's API. Unfortunately that is a hard fact which cannot be changed. If I want to enable prophecies for PHP built-ins, I have to cope with pass-by-reference no matter how confusing you may find them.

I would even consider that not being able to prophesize such API is a Prophecy feature

Then why does ClassCodeGenerator::generateArguments() care about them?

[Seldaek]

I also hit this just now, had to revert back to using PHPUnit's MockBuilder for the one use case with references needed in a callback.. Not the end of the world, but it'd be nice if there was a way to use Prophecy for this, even if it's a separate API to the ->will(callback) shorthand.

[stof]

well, the issue is that multiple layers of Prophecy are involved, to go from the mock class to the executed callback. For references to work, all these layers need to pass by reference.