Fix arginfos of required arguments for some Reflection methods

[carusogabriel]

As reported in #74035 and #71416, ReflectionClass::newInstance and ReflectionMethod::invoke have wrong number or required arguments in arginfo.

Looks like there was previous PRs fixing these bugs, but there were reject due they also change the arginfo of variadics arguments. This time, I'm only changing the number of required arguments, been a bug fix, not a BC break.

If this PR gets merge, we also need to fix the documentation for ReflectionClass::newInstance.

[nikic]

This is also a BC break, but I'd be fine with giving it a try in 7.3, as inheritors are likely to already use the signature with an optional argument. Definitely can't go into 7.1 though.

[carusogabriel]

This is also a BC break

@nikic Sorry, but why? Arginfo is dedicated to the Reflection API, right? Also, we've already made these changes in the past. Do methods and functions have diferent behaviours?

but I'd be fine with giving it a try in 7.3

I'll change the base branch after review my comment below.

[cmb69]

@carusogabriel Please consider the following script:

<?php

class MyReflectionMethod extends ReflectionMethod
{
    public function invoke($object, $args) {}
}

It compiles fine before this PR, but throws a warning after merging:

Warning: Declaration of MyReflectionMethod::invoke($object, $args) should be compatible with ReflectionMethod::invoke($object, $args = NULL) in %s

[carusogabriel]

@cmb69 Thank you for the explanation, things are now clear for me why changing arginfo of methods, especially those ones that users can extend are considered BC break. I'll close this one and add to our list of small things we should fix in PHP.NEXT.

Thanks.

[Majkl578]

This is unacceptable BC break.

Breaks Doctrine:

There was 1 error:

1) Doctrine\Tests\Common\Reflection\StaticReflectionClassTest::testGetName
Declaration of Doctrine\Common\Reflection\StaticReflectionClass::newInstance($args) should be compatible with ReflectionClass::newInstance($args = NULL)

/www/doctrine/reflection/lib/Doctrine/Common/Reflection/StaticReflectionClass.php:413
/www/doctrine/reflection/tests/Doctrine/Tests/Common/Reflection/StaticReflectionClassTest.php:150

ERRORS!
Tests: 124, Assertions: 230, Errors: 1.

Breaks BetterReflection:

1) Roave\BetterReflectionTest\SourceLocator\Type\PhpInternalSourceLocatorTest::testAllGeneratedStubsAreInSyncWithInternalReflectionClasses with data set "ReflectionMethod" ('ReflectionMethod')
ReflectionMethod#invoke.args
Failed asserting that false is identical to true.

/tmp/BetterReflection/test/unit/SourceLocator/Type/PhpInternalSourceLocatorTest.php:427
/tmp/BetterReflection/test/unit/SourceLocator/Type/PhpInternalSourceLocatorTest.php:369
/tmp/BetterReflection/test/unit/SourceLocator/Type/PhpInternalSourceLocatorTest.php:301
/tmp/BetterReflection/test/unit/SourceLocator/Type/PhpInternalSourceLocatorTest.php:207

FAILURES!
Tests: 2063, Assertions: 27964, Failures: 1, Skipped: 3, Incomplete: 13.

Please don't touch Reflection for once. :X

cc @Ocramius

[Ocramius]

Yeah, sadly it impacts existing libraries in a hard way, and likely not fixable in a minor release