Support calls to functions with variadic parameters

[AJenbo]

Type: bugfix
Breaking change: no

Previously this would often fail on usage of sprintf() and other function that takes variadic parameters.

[tvbeek]

@AJenbo thanks for your PR, do you have the possibility to add a test that failed before this PR and is fixed with this PR?

I assign this PR to you, but feel free to say if you need help creating the tests.

[AJenbo]

Sure, will try and get around to it some time this week.

[AJenbo]

Oh, I forgot all about this one. I'll try come up with a same that show the issue.

[AJenbo]

No fair you guys merged another fix for this just a month later, and the didn't even trace down what the cause was 😂

#818

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.78%. Comparing base (0a3060e) to head (090d480).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #814      +/-   ##
============================================
- Coverage     92.84%   92.78%   -0.06%     
- Complexity     1277     1278       +1     
============================================
  Files           112      112              
  Lines          3478     3257     -221     
============================================
- Hits           3229     3022     -207     
+ Misses          249      235      -14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AJenbo]

Ok, I added a test, removed the "be quiet" fix, and made this one more explicit as to what it is doing. Hope that makes it good enough to merge :)

[kylekatarnls]

Hi, thanks to this I discovered that a parameter can be both variadic and reference 🤯

The test is good as it's what also missed in #818 but I did not succeed to find a case that would explain why we need specific stuff for PHP < 5.6 or why min() solution is better than isset()

I tried:

#973 (min)
PHP 5.3 result

#974 (isset)
PHP 5.3 result

By the way I added there a test for variadic+reference but current fixes still don't support it, so we need an other fix for that.

class Test
{
    public function foo(string $format, mixed &...$values): string
    {
        $values[1] = 9;

        return sprintf($format, ...$values);
    }

    public function bar()
    {
        $variadic1 = 1;
        $this->print("Text %d", $variadic1, $variadic2);

        return $variadic2;
    }
}

$t = new Test();

echo $t->bar();

[AJenbo]

Without PHP < 5.6 it will fail because isVeriadic() is only available from PHP 5.6. I chose to use || in case someone is trying to analyze PHP 5.6 code in 5.5. But we could also set it to fail in that case using && instead.

min() allows you to know that it was the variadic paramter, and will correctly let you know if it is by reference. isset() simply ignores it and says it's not by reference even if it is.

I think it also better allow you to realize why the count might be higher, rather then just ignoring a case that appears like it should never happen.

[kylekatarnls]

min() allows you to know that it was the variadic paramter, and will correctly let you know if it is by reference. isset() simply ignores it and says it's not by reference even if it is.

That's also what I believed reading the code but as you can see here it's still producing a violation: #973 (in both PHP <= 5.6 and > 5.6)

[ravage84]

@AJenbo can this be updated to be mergable into 3.0.0?

[AJenbo]

Sure, though there isn't anything BC in it.

[kylekatarnls]

Suggested change

	if (version_compare(PHP_VERSION, '5.6', '<') || $reflectionFunction->isVariadic()) {
	if (PHP_VERSION_ID < 50600 || $reflectionFunction->isVariadic()) {

[AJenbo]

Pulling this in to the merge in to 3.x