Repository: Roave/BetterReflection (php)

Issue:
Parameter with a default value followed by a variadic parameter should be optional
Example code:

```php
function doBar(?callable $callback = null, ...$args): void
{

}
```

BetterReflection considers the `$callback` parameter to be non-optional.

PoC fix: https://github.com/ondrejmirtes/BetterReflection/commit/a155b3187b218fcbbc58865e76154281e9cb0847

Original issue: https://github.com/phpstan/phpstan/issues/3409

Pull request:
Parameter with default value before variadic parameter should be optional
Fixes #646 

Patch:
diff --git a/docs/usage.md b/docs/usage.md
index 54a968e9..b8b238a0 100644
--- a/docs/usage.md
+++ b/docs/usage.md
@@ -266,7 +266,7 @@ $functions = $reflector->getAllFunctions();
 
 ### Reflecting a Closure
 
-The `ReflectionFunction` class has a static constructor which you can reflect directly on a closure:
+The `ReflectionFunction` class has a static constructor which you can reflectdirectly on a closure:
 
 ```php
 <?php
diff --git a/src/Reflection/ReflectionFunctionAbstract.php b/src/Reflection/ReflectionFunctionAbstract.php
index 79ae5ee5..b96c5f48 100644
--- a/src/Reflection/ReflectionFunctionAbstract.php
+++ b/src/Reflection/ReflectionFunctionAbstract.php
@@ -100,16 +100,16 @@ abstract class ReflectionFunctionAbstract
         $overallOptionalFlag = true;
         $lastParamIndex      = count($this->getNode()->params) - 1;
         for ($i = $lastParamIndex; $i >= 0; $i--) {
-            $hasDefault = ($this->getNode()->params[$i]->default !== null);
+            $param = $this->getNode()->params[$i];
 
-            // When we find the first parameter that does not have a default,
+            // When we find the first parameter that does not have a default or is not variadic,
             // flip the flag as all params for this are no longer optional
             // EVEN if they have a default value
-            if (! $hasDefault) {
+            if ($param->default === null && ! $param->variadic) {
                 $overallOptionalFlag = false;
             }
 
-            $this->getNode()->params[$i]->isOptional = $overallOptionalFlag;
+            $param->isOptional = $overallOptionalFlag;
         }
     }
 
diff --git a/src/SourceLocator/Type/AutoloadSourceLocator.php b/src/SourceLocator/Type/AutoloadSourceLocator.php
index 0afd76dd..64a6c284 100644
--- a/src/SourceLocator/Type/AutoloadSourceLocator.php
+++ b/src/SourceLocator/Type/AutoloadSourceLocator.php
@@ -203,7 +203,7 @@ class AutoloadSourceLocator extends AbstractSourceLocator
     /**
      * We can only load constants if they already exist, because PHP does not
      * have constant autoloading. Therefore if it exists, we simply use brute force
-     * to search throughout all included files to find the right filename.
+     * to search throught all included files to find the right filename.
      */
     private function locateConstantByName(string $constantName) : ?string
     {


Test patch:
diff --git a/test/unit/Fixture/ClassesWithPublicOrNonPublicConstructor.php b/test/unit/Fixture/ClassesWithPublicOrNonPublicContructor.php
similarity index 96%
rename from test/unit/Fixture/ClassesWithPublicOrNonPublicConstructor.php
rename to test/unit/Fixture/ClassesWithPublicOrNonPublicContructor.php
index 1de3ccc1..2960f5ac 100644
--- a/test/unit/Fixture/ClassesWithPublicOrNonPublicConstructor.php
+++ b/test/unit/Fixture/ClassesWithPublicOrNonPublicContructor.php
@@ -1,6 +1,6 @@
 <?php
 
-namespace Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicConstructor;
+namespace Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicContructor;
 
 
 class ClassWithPublicConstructor
diff --git a/test/unit/Fixture/Methods.php b/test/unit/Fixture/Methods.php
index cb9004ba..e17b3eb5 100644
--- a/test/unit/Fixture/Methods.php
+++ b/test/unit/Fixture/Methods.php
@@ -67,6 +67,10 @@ abstract class Methods
     {
     }
 
+    public function methodWithFirstParameterWithDefaultValueAndSecondParameterIsVariadic($parameterWithDefaultValue = null, ...$variadicParameter)
+    {
+    }
+
     public function methodWithReference($nonRefParameter, &$refParameter)
     {
     }
diff --git a/test/unit/Reflection/Adapter/ReflectionPropertyTest.php b/test/unit/Reflection/Adapter/ReflectionPropertyTest.php
index 16d0d454..dfc30a07 100644
--- a/test/unit/Reflection/Adapter/ReflectionPropertyTest.php
+++ b/test/unit/Reflection/Adapter/ReflectionPropertyTest.php
@@ -39,7 +39,7 @@ class ReflectionPropertyTest extends TestCase
     /**
      * @dataProvider coreReflectionPropertyNamesProvider
      */
-    public function testCoreReflectionProperties(string $methodName) : void
+    public function testCoreReflectionPropertys(string $methodName) : void
     {
         $reflectionPropertyAdapterReflection = new CoreReflectionClass(ReflectionPropertyAdapter::class);
         self::assertTrue($reflectionPropertyAdapterReflection->hasMethod($methodName));
diff --git a/test/unit/Reflection/ReflectionClassConstantTest.php b/test/unit/Reflection/ReflectionClassConstantTest.php
index 9334758f..0e71fb65 100644
--- a/test/unit/Reflection/ReflectionClassConstantTest.php
+++ b/test/unit/Reflection/ReflectionClassConstantTest.php
@@ -121,7 +121,7 @@ class ReflectionClassConstantTest extends TestCase
         ];
     }
 
-    public function columnsProvider() : array
+    public function columsProvider() : array
     {
         return [
             ["<?php\n\nclass T {\nconst TEST = 1;}", 1, 15],
@@ -131,7 +131,7 @@ class ReflectionClassConstantTest extends TestCase
     }
 
     /**
-     * @dataProvider columnsProvider
+     * @dataProvider columsProvider
      */
     public function testGetStartColumnAndEndColumn(string $php, int $startColumn, int $endColumn) : void
     {
diff --git a/test/unit/Reflection/ReflectionClassTest.php b/test/unit/Reflection/ReflectionClassTest.php
index 0cb888e0..9ed5e79d 100644
--- a/test/unit/Reflection/ReflectionClassTest.php
+++ b/test/unit/Reflection/ReflectionClassTest.php
@@ -35,11 +35,11 @@ use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;
 use Roave\BetterReflectionTest\BetterReflectionSingleton;
 use Roave\BetterReflectionTest\ClassesImplementingIterators;
 use Roave\BetterReflectionTest\ClassesWithCloneMethod;
-use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicConstructor\ClassWithExtendedConstructor;
-use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicConstructor\ClassWithoutConstructor;
-use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicConstructor\ClassWithPrivateConstructor;
-use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicConstructor\ClassWithProtectedConstructor;
-use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicConstructor\ClassWithPublicConstructor;
+use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicContructor\ClassWithExtendedConstructor;
+use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicContructor\ClassWithoutConstructor;
+use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicContructor\ClassWithPrivateConstructor;
+use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicContructor\ClassWithProtectedConstructor;
+use Roave\BetterReflectionTest\ClassesWithPublicOrNonPublicContructor\ClassWithPublicConstructor;
 use Roave\BetterReflectionTest\ClassWithInterfaces;
 use Roave\BetterReflectionTest\ClassWithInterfacesExtendingInterfaces;
 use Roave\BetterReflectionTest\ClassWithInterfacesOther;
@@ -173,7 +173,7 @@ class ReflectionClassTest extends TestCase
             [CoreReflectionMethod::IS_STATIC, 1],
             [CoreReflectionMethod::IS_ABSTRACT, 1],
             [CoreReflectionMethod::IS_FINAL, 1],
-            [CoreReflectionMethod::IS_PUBLIC, 16],
+            [CoreReflectionMethod::IS_PUBLIC, 17],
             [CoreReflectionMethod::IS_PROTECTED, 1],
             [CoreReflectionMethod::IS_PRIVATE, 1],
             [
@@ -183,7 +183,7 @@ class ReflectionClassTest extends TestCase
                 CoreReflectionMethod::IS_PUBLIC |
                 CoreReflectionMethod::IS_PROTECTED |
                 CoreReflectionMethod::IS_PRIVATE,
-                18,
+                19,
             ],
         ];
     }
@@ -626,7 +626,7 @@ PHP;
         self::assertStringContainsString('Some comments here', $classInfo->getDocComment());
     }
 
-    public function testGetDocCommentBetweenComments() : void
+    public function testGetDocCommentBetweeenComments() : void
     {
         $php       = '<?php
             /* A comment */
@@ -1335,7 +1335,7 @@ PHP;
         self::assertFalse($reflector->reflect(ExampleInterface::class)->isInstantiable());
 
         $reflector = new ClassReflector(new SingleFileSourceLocator(
-            __DIR__ . '/../Fixture/ClassesWithPublicOrNonPublicConstructor.php',
+            __DIR__ . '/../Fixture/ClassesWithPublicOrNonPublicContructor.php',
             $this->astLocator,
         ));
 
diff --git a/test/unit/Reflection/ReflectionParameterTest.php b/test/unit/Reflection/ReflectionParameterTest.php
index fb9c7b99..33024897 100644
--- a/test/unit/Reflection/ReflectionParameterTest.php
+++ b/test/unit/Reflection/ReflectionParameterTest.php
@@ -443,6 +443,20 @@ class ReflectionParameterTest extends TestCase
         self::assertFalse($secondParam->isDefaultValueAvailable());
     }
 
+    public function testParameterWithDefaultValueBeforeVariadicParameterShouldBeOptional() : void
+    {
+        $classInfo = $this->reflector->reflect(Methods::class);
+        $method    = $classInfo->getMethod('methodWithFirstParameterWithDefaultValueAndSecondParameterIsVariadic');
+
+        $firstParam = $method->getParameter('parameterWithDefaultValue');
+        self::assertTrue($firstParam->isOptional());
+        self::assertTrue($firstParam->isDefaultValueAvailable());
+
+        $secondParam = $method->getParameter('variadicParameter');
+        self::assertTrue($secondParam->isOptional());
+        self::assertTrue($secondParam->isVariadic());
+    }
+
     /**
      * @group 109
      */
diff --git a/test/unit/Reflection/ReflectionPropertyTest.php b/test/unit/Reflection/ReflectionPropertyTest.php
index a0ed227d..f603389e 100644
--- a/test/unit/Reflection/ReflectionPropertyTest.php
+++ b/test/unit/Reflection/ReflectionPropertyTest.php
@@ -161,7 +161,7 @@ class ReflectionPropertyTest extends TestCase
         self::assertSame($expectedDoc, $property->getDocComment());
     }
 
-    public function testGetDocCommentBetweenComments() : void
+    public function testGetDocCommentBetweeenComments() : void
     {
         $php       = '<?php
             class Bar implements Foo {
@@ -325,7 +325,7 @@ class ReflectionPropertyTest extends TestCase
         ];
     }
 
-    public function columnsProvider() : array
+    public function columsProvider() : array
     {
         return [
             ["<?php\n\nclass T {\npublic \$test = 1;\n}", 1, 17],
@@ -335,7 +335,7 @@ class ReflectionPropertyTest extends TestCase
     }
 
     /**
-     * @dataProvider columnsProvider
+     * @dataProvider columsProvider
      */
     public function testGetStartColumnAndEndColumn(string $php, int $startColumn, int $endColumn) : void
     {
diff --git a/test/unit/SourceLocator/Ast/LocatorTest.php b/test/unit/SourceLocator/Ast/LocatorTest.php
index 7f48ac4b..b4324e95 100644
--- a/test/unit/SourceLocator/Ast/LocatorTest.php
+++ b/test/unit/SourceLocator/Ast/LocatorTest.php
@@ -135,7 +135,7 @@ class LocatorTest extends TestCase
         self::assertInstanceOf(ReflectionConstant::class, $constantInfo);
     }
 
-    public function testReflectThrowsExceptionWhenClassNotFoundAndNoNodesExist() : void
+    public function testReflectThrowsExeptionWhenClassNotFoundAndNoNodesExist() : void
     {
         $php = '<?php';
 
@@ -147,7 +147,7 @@ class LocatorTest extends TestCase
         );
     }
 
-    public function testReflectThrowsExceptionWhenClassNotFoundButNodesExist() : void
+    public function testReflectThrowsExeptionWhenClassNotFoundButNodesExist() : void
     {
         $php = "<?php
         namespace Foo;
diff --git a/test/unit/SourceLocator/SourceStubber/ReflectionSourceStubberTest.php b/test/unit/SourceLocator/SourceStubber/ReflectionSourceStubberTest.php
index 534c98c5..10bb560e 100644
--- a/test/unit/SourceLocator/SourceStubber/ReflectionSourceStubberTest.php
+++ b/test/unit/SourceLocator/SourceStubber/ReflectionSourceStubberTest.php
@@ -303,7 +303,7 @@ class ReflectionSourceStubberTest extends TestCase
         if (! in_array($parameterName, ['mysqli_stmt#bind_param.vars', 'mysqli_stmt#bind_result.vars'], true)
             && ! preg_match('~^RedisCluster#\w+.arg$~', $parameterName)
         ) {
-            // Parameters are variadic but not optional
+            // Parameters are variadic but not optinal
             self::assertSame($original->isOptional(), $stubbed->isOptional(), $parameterName);
         }
 
diff --git a/test/unit/SourceLocator/Type/MemoizingSourceLocatorTest.php b/test/unit/SourceLocator/Type/MemoizingSourceLocatorTest.php
index 1ae5d31a..fe74985e 100644
--- a/test/unit/SourceLocator/Type/MemoizingSourceLocatorTest.php
+++ b/test/unit/SourceLocator/Type/MemoizingSourceLocatorTest.php
@@ -54,7 +54,7 @@ class MemoizingSourceLocatorTest extends TestCase
         $this->memoizingLocator = new MemoizingSourceLocator($this->wrappedLocator);
         $this->identifierNames  = array_unique(array_map(
             static function () : string {
-                return uniqid('identifier', true);
+                return uniqid('identifer', true);
             },
             range(1, 20),
         ));