From eed3ed55138cb3d2e6de9bf81c8e1784315ef653 Mon Sep 17 00:00:00 2001
From: Luke Downing <lukeraymonddowning@gmail.com>
Date: Tue, 19 Oct 2021 21:40:40 +0100
Subject: [PATCH] Vastly improves the logic around bound datasets to make them
 more user friendly.

---
 src/Concerns/Testable.php    | 29 +++++++++++++++++++++----
 src/Support/Reflection.php   |  8 +++++++
 tests/.snapshots/success.txt | 13 ++++++++++--
 tests/Features/Datasets.php  | 41 ++++++++++++++++++++++++++++++++++++
 4 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/src/Concerns/Testable.php b/src/Concerns/Testable.php
index 72bf02bb..a6633612 100644
--- a/src/Concerns/Testable.php
+++ b/src/Concerns/Testable.php
@@ -7,8 +7,10 @@ namespace Pest\Concerns;
 use Closure;
 use Pest\Support\ChainableClosure;
 use Pest\Support\ExceptionTrace;
+use Pest\Support\Reflection;
 use Pest\TestSuite;
 use PHPUnit\Framework\ExecutionOrderDependency;
+use function sprintf;
 use Throwable;
 
 /**
@@ -257,7 +259,7 @@ trait Testable
      */
     public function toString(): string
     {
-        return \sprintf(
+        return sprintf(
             '%s::%s',
             self::$__filename,
             $this->__description
@@ -283,9 +285,28 @@ trait Testable
      */
     private function resolveTestArguments(array $arguments): array
     {
-        return array_map(function ($data) {
-            return $data instanceof Closure ? $this->__callClosure($data, []) : $data;
-        }, $arguments);
+        if (count($arguments) !== 1) {
+            return $arguments;
+        }
+
+        if (!$arguments[0] instanceof Closure) {
+            return $arguments;
+        }
+
+        $underlyingTest     = Reflection::getFunctionVariable($this->__test, 'factoryTest');
+        $testParameterTypes = array_values(Reflection::getFunctionArguments($underlyingTest));
+
+        if (in_array($testParameterTypes[0], ['Closure', 'callable'])) {
+            return $arguments;
+        }
+
+        $boundDatasetResult = $this->__callClosure($arguments[0], []);
+
+        if (count($testParameterTypes) === 1 || !is_array($boundDatasetResult)) {
+            return [$boundDatasetResult];
+        }
+
+        return array_values($boundDatasetResult);
     }
 
     /**
diff --git a/src/Support/Reflection.php b/src/Support/Reflection.php
index 44cd754c..afdc1b94 100644
--- a/src/Support/Reflection.php
+++ b/src/Support/Reflection.php
@@ -204,4 +204,12 @@ final class Reflection
 
         return $arguments;
     }
+
+    /**
+     * @return mixed
+     */
+    public static function getFunctionVariable(Closure $function, string $key)
+    {
+        return (new ReflectionFunction($function))->getStaticVariables()[$key] ?? null;
+    }
 }
diff --git a/tests/.snapshots/success.txt b/tests/.snapshots/success.txt
index 0c6a57ab..7bac8198 100644
--- a/tests/.snapshots/success.txt
+++ b/tests/.snapshots/success.txt
@@ -96,11 +96,20 @@
   ✓ more than two datasets with (2) / (4) / (5)
   ✓ more than two datasets with (2) / (4) / (6)
   ✓ more than two datasets did the job right
-  ✓ it can resolve a dataset after the test case is available with (Closure Object (...))
+  ✓ it can resolve a dataset after the test case is available with (Closure Object (...)) #1
+  ✓ it can resolve a dataset after the test case is available with (Closure Object (...)) #2
   ✓ it can resolve a dataset after the test case is available with shared yield sets with (Closure Object (...)) #1
   ✓ it can resolve a dataset after the test case is available with shared yield sets with (Closure Object (...)) #2
   ✓ it can resolve a dataset after the test case is available with shared array sets with (Closure Object (...)) #1
   ✓ it can resolve a dataset after the test case is available with shared array sets with (Closure Object (...)) #2
+  ✓ it resolves a potential bound dataset logically with ('foo', Closure Object (...))
+  ✓ it resolves a potential bound dataset logically even when the closure comes first with (Closure Object (...), 'bar')
+  ✓ it will not resolve a closure if it is type hinted as a closure with (Closure Object (...)) #1
+  ✓ it will not resolve a closure if it is type hinted as a closure with (Closure Object (...)) #2
+  ✓ it will not resolve a closure if it is type hinted as a callable with (Closure Object (...)) #1
+  ✓ it will not resolve a closure if it is type hinted as a callable with (Closure Object (...)) #2
+  ✓ it can correctly resolve a bound dataset that returns an array with (Closure Object (...))
+  ✓ it can correctly resolve a bound dataset that returns an array but wants to be spread with (Closure Object (...))
 
    PASS  Tests\Features\Exceptions
   ✓ it gives access the the underlying expectException
@@ -720,5 +729,5 @@
   ✓ it is a test
   ✓ it uses correct parent class
 
-  Tests:  4 incompleted, 9 skipped, 478 passed
+  Tests:  4 incompleted, 9 skipped, 487 passed
   
\ No newline at end of file
diff --git a/tests/Features/Datasets.php b/tests/Features/Datasets.php
index be94e596..b65f3680 100644
--- a/tests/Features/Datasets.php
+++ b/tests/Features/Datasets.php
@@ -232,6 +232,7 @@ it('can resolve a dataset after the test case is available', function ($result)
     expect($result)->toBe('bar');
 })->with([
     function () { return $this->foo; },
+    [function () { return $this->foo; }],
 ]);
 
 it('can resolve a dataset after the test case is available with shared yield sets', function ($result) {
@@ -241,3 +242,43 @@ it('can resolve a dataset after the test case is available with shared yield set
 it('can resolve a dataset after the test case is available with shared array sets', function ($result) {
     expect($result)->toBeInt()->toBeLessThan(3);
 })->with('bound.array');
+
+it('resolves a potential bound dataset logically', function ($foo, $bar) {
+    expect($foo)->toBe('foo');
+    expect($bar())->toBe('bar');
+})->with([
+    ['foo', function () { return 'bar'; }], // This should be passed as a closure because we've passed multiple arguments
+]);
+
+it('resolves a potential bound dataset logically even when the closure comes first', function ($foo, $bar) {
+    expect($foo())->toBe('foo');
+    expect($bar)->toBe('bar');
+})->with([
+    [function () { return 'foo'; }, 'bar'], // This should be passed as a closure because we've passed multiple arguments
+]);
+
+it('will not resolve a closure if it is type hinted as a closure', function (Closure $data) {
+    expect($data())->toBeString();
+})->with([
+    function () { return 'foo'; },
+    function () { return 'bar'; },
+]);
+
+it('will not resolve a closure if it is type hinted as a callable', function (callable $data) {
+    expect($data())->toBeString();
+})->with([
+    function () { return 'foo'; },
+    function () { return 'bar'; },
+]);
+
+it('can correctly resolve a bound dataset that returns an array', function (array $data) {
+    expect($data)->toBe(['foo', 'bar', 'baz']);
+})->with([
+    function () { return ['foo', 'bar', 'baz']; },
+]);
+
+it('can correctly resolve a bound dataset that returns an array but wants to be spread', function (string $foo, string $bar, string $baz) {
+    expect([$foo, $bar, $baz])->toBe(['foo', 'bar', 'baz']);
+})->with([
+    function () { return ['foo', 'bar', 'baz']; },
+]);