Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when mocking built-in function with pass-by-reference argument and argument with default value (PHP 8) #201

Open
jimbonator opened this issue Jun 14, 2022 · 1 comment

Comments

@jimbonator
Copy link

This is for AspectMock 4.1.1.

During our migration from PHP 7.4 to PHP 8.1, we've encountered this pattern a few times:

When our PHP-Unit tests mock a PHP built-in function that has at least one argument passed by reference and one argument with a default value, the mock fails with the error ParseError: syntax error, unexpected token "=", expecting ")".

The stack track looks like this:

/tmp/flockoipmSG:4
/petabox/www/common/vendor/codeception/aspect-mock/src/AspectMock/Core/Mocker.php:275
/petabox/www/common/vendor/codeception/aspect-mock/src/AspectMock/Core/Registry.php:38
/petabox/www/common/vendor/codeception/aspect-mock/src/AspectMock/Test.php:248

Examining the temporary file reveals the problem:

$ cat /tmp/flockoipmSG
<?php
namespace Atomic;
if (!function_exists('Atomic\flock')) {
    function flock($p0, int $p1, &$p2 = null=NULL) {
         $args = [];
         switch(count(func_get_args())) {
             case 3: $args = [$p0, $p1, &$p2]; break;
             case 2: $args = [$p0, $p1]; break;
             case 1: $args = [$p0]; break;
         }
         if (($__am_res = __amock_before_func('Atomic','flock', $args)) !== __AM_CONTINUE__) {
             return $__am_res;
         }
         return call_user_func_array('flock', $args);
    }
}

The function declaration has two default values for an argument (function flock($p0, int $p1, &$p2 = null=NULL).

Other functions which have given us this problem are:

  • system()
  • stream_socket_client()
  • stream_select()

The last one is interesting because, unlike the others, the passed-by-reference arguments don't have default values. Only the final argument does:

stream_select(
    ?array &$read,
    ?array &$write,
    ?array &$except,
    ?int $seconds,
    ?int $microseconds = null
): int|false

That's what leads me to believe the problem is the existence of both in the function signature, but not necessarily that a single argument needs to be both pass-by-reference and have a default value for this to fail.

We don't see this under PHP 7.4, so I assume this related to the PHP version.

@BotRoy
Copy link

BotRoy commented Jul 22, 2022

Hi @jimbonator , I'm testing out your goaop/parser-reflection pull request to make AspectMock php 8 compatible (running php 8.0.17, I made your PR into a patch for goaop/parser-reflection, and I'm using --ignore-platform-reqs).

I see this issue too, and I found a temporary fix. I don't really understand all the code surrounding this, so there's a decent chance I broke something else with this.

For example, the exec function, when mocked, contains the line function exec(string $p0, &$p1 = null=NULL, &$p2 = null=NULL) {

Based on the man page for exec, it seems that PHP is appending the = null. In src/AspectMock/Intercept/FunctionInjector.php, aspect-mock appends =NULL because the function is internal and the parameter is optional:

if ($internal && $parameter->isOptional()) {
    $text .= "=NULL";
}

If I remove the above code, the errors go away and no additional errors/failures are introduced. I am still trying to understand this code, the commit/PR it was introduced in, what other things this could possibly break, and what a better fix is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants