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

Merging #814 (ccdcc64) into master (87a0822) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #814   +/-   ##
=========================================
  Coverage     88.65%   88.65%           
- Complexity     1055     1056    +1     
=========================================
  Files            95       95           
  Lines          2661     2663    +2     
=========================================
+ Hits           2359     2361    +2     
  Misses          302      302           

Impacted Files	Coverage Δ
src/main/php/PHPMD/Rule/AbstractLocalVariable.php	91.20% <100.00%> (+0.19%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[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.