Show Expr::countDistinct() and Expr::concat() use variable-length argument lists

[craigfrancis]

The functions Expr::countDistinct() and Expr::concat() both use func_get_args() to accept a variable number of arguments.

They should explicitly show this via the "..." operator, which has been supported since PHP 5.6.

[greg0ire]

Not sure why this wasn't addressed in #8319

[greg0ire]

There is a (small) BC-break for extending classes: https://3v4l.org/qQUlA

[craigfrancis]

Thanks Grégoire.

I wonder if a few other instances of the func_get_args() functions could be removed as well, but I should probably keep this PR simple.

[craigfrancis]

As to the BC-break for extending Expr... that's a good point... is this something that happens with Expr?

[greg0ire]

is this something that happens with Expr?

I'm really not sure how to find out 😅 … all I know is that it is not final nor internal.

[craigfrancis]

is this something that happens with Expr?

I'm really not sure how to find out 😅 … all I know is that it is not final nor internal.

Good point, I suppose anyone could want a better version of these methods... where they could prefer a lower-case count() :-)

[greg0ire]

Maybe retarget to 3.0.x then, and make sure that there are no other occurrences of func_get_args() in the codebase?

[craigfrancis]

I've changed the target for this PR.

And a quick search of 3.0.x suggests the only other instance of func_get_args() is in ReflectionReadonlyProperty, and I'm not sure I should be touching that (I'm not too familiar with what it's doing).

[greg0ire]

I think even if you tried it wouldn't work, as this class extends another one defined by PHP itself. You would probably end up with an incompatible signature.

[derrabus]

Good catch, just some nit-picking.

[craigfrancis]

Thanks Alexander

[greg0ire]

Please rebase interactively, get rid of 58df2e1 and squash all other commits into one.

[craigfrancis]

Sorry, made even more of a mess of the commit history (I'm putting it down to the heat)... I've opened a new one, which uses 3.0.x as the base, and has a single commit.

[greg0ire]

Sorry, made even more of a mess of the commit history (I'm putting it down to the heat)... I've opened a new one, which uses 3.0.x as the base, and has a single commit.

Why don't you reopen and force push instead?

[craigfrancis]

Why don't you reopen and force push instead?

While I created a branch for this change, I wasn’t paying attention, and committed to the numbered branch… and when it came to the rebase, I was rushing, as I’m cooking food atm and nearly burnt the rice :-)

[greg0ire]

Slow down lol