From 882504b45f4d788a7eb5a73c1f2ac4cfea8ed415 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Tue, 8 Jun 2021 16:17:55 -0400 Subject: [PATCH 1/3] [9.x] Universal HigherOrderWhenProxy --- src/Illuminate/Collections/Enumerable.php | 4 +- .../Collections/HigherOrderWhenProxy.php | 27 ++- .../Collections/Traits/EnumeratesValues.php | 39 +---- .../Support/Traits/Conditionable.php | 18 +- tests/Support/SupportCollectionTest.php | 26 ++- tests/Support/SupportConditionableTest.php | 156 ++++++++++++++++++ 6 files changed, 211 insertions(+), 59 deletions(-) create mode 100644 tests/Support/SupportConditionableTest.php diff --git a/src/Illuminate/Collections/Enumerable.php b/src/Illuminate/Collections/Enumerable.php index 8a0b69455f92..ef99c3143a92 100644 --- a/src/Illuminate/Collections/Enumerable.php +++ b/src/Illuminate/Collections/Enumerable.php @@ -271,11 +271,11 @@ public function filter(callable $callback = null); * Apply the callback if the value is truthy. * * @param bool $value - * @param callable $callback + * @param callable|null $callback * @param callable|null $default * @return static|mixed */ - public function when($value, callable $callback, callable $default = null); + public function when($value, callable $callback = null, callable $default = null); /** * Apply the callback if the collection is empty. diff --git a/src/Illuminate/Collections/HigherOrderWhenProxy.php b/src/Illuminate/Collections/HigherOrderWhenProxy.php index 6653c03a656f..173a78396790 100644 --- a/src/Illuminate/Collections/HigherOrderWhenProxy.php +++ b/src/Illuminate/Collections/HigherOrderWhenProxy.php @@ -2,17 +2,14 @@ namespace Illuminate\Support; -/** - * @mixin \Illuminate\Support\Enumerable - */ class HigherOrderWhenProxy { /** - * The collection being operated on. + * The target being conditionally operated on. * - * @var \Illuminate\Support\Enumerable + * @var mixed */ - protected $collection; + protected $target; /** * The condition for proxying. @@ -24,18 +21,18 @@ class HigherOrderWhenProxy /** * Create a new proxy instance. * - * @param \Illuminate\Support\Enumerable $collection + * @param mixed $target * @param bool $condition * @return void */ - public function __construct(Enumerable $collection, $condition) + public function __construct($target, $condition) { + $this->target = $target; $this->condition = $condition; - $this->collection = $collection; } /** - * Proxy accessing an attribute onto the collection. + * Proxy accessing an attribute onto the target. * * @param string $key * @return mixed @@ -43,12 +40,12 @@ public function __construct(Enumerable $collection, $condition) public function __get($key) { return $this->condition - ? $this->collection->{$key} - : $this->collection; + ? $this->target->{$key} + : $this->target; } /** - * Proxy a method call onto the collection. + * Proxy a method call on the target. * * @param string $method * @param array $parameters @@ -57,7 +54,7 @@ public function __get($key) public function __call($method, $parameters) { return $this->condition - ? $this->collection->{$method}(...$parameters) - : $this->collection; + ? $this->target->{$method}(...$parameters) + : $this->target; } } diff --git a/src/Illuminate/Collections/Traits/EnumeratesValues.php b/src/Illuminate/Collections/Traits/EnumeratesValues.php index c707c36dfbf6..21b0dc6c29e7 100644 --- a/src/Illuminate/Collections/Traits/EnumeratesValues.php +++ b/src/Illuminate/Collections/Traits/EnumeratesValues.php @@ -11,7 +11,6 @@ use Illuminate\Support\Collection; use Illuminate\Support\Enumerable; use Illuminate\Support\HigherOrderCollectionProxy; -use Illuminate\Support\HigherOrderWhenProxy; use JsonSerializable; use Symfony\Component\VarDumper\VarDumper; use Traversable; @@ -45,6 +44,8 @@ */ trait EnumeratesValues { + use Conditionable; + /** * The methods that can be proxied. * @@ -453,29 +454,6 @@ public function sum($callback = null) }, 0); } - /** - * Apply the callback if the value is truthy. - * - * @param bool|mixed $value - * @param callable|null $callback - * @param callable|null $default - * @return static|mixed - */ - public function when($value, callable $callback = null, callable $default = null) - { - if (! $callback) { - return new HigherOrderWhenProxy($this, $value); - } - - if ($value) { - return $callback($this, $value); - } elseif ($default) { - return $default($this, $value); - } - - return $this; - } - /** * Apply the callback if the collection is empty. * @@ -500,19 +478,6 @@ public function whenNotEmpty(callable $callback, callable $default = null) return $this->when($this->isNotEmpty(), $callback, $default); } - /** - * Apply the callback if the value is falsy. - * - * @param bool $value - * @param callable $callback - * @param callable|null $default - * @return static|mixed - */ - public function unless($value, callable $callback, callable $default = null) - { - return $this->when(! $value, $callback, $default); - } - /** * Apply the callback unless the collection is empty. * diff --git a/src/Illuminate/Support/Traits/Conditionable.php b/src/Illuminate/Support/Traits/Conditionable.php index 183d63c66f02..f70fe8412ee6 100644 --- a/src/Illuminate/Support/Traits/Conditionable.php +++ b/src/Illuminate/Support/Traits/Conditionable.php @@ -2,19 +2,25 @@ namespace Illuminate\Support\Traits; +use Illuminate\Support\HigherOrderWhenProxy; + trait Conditionable { /** * Apply the callback if the given "value" is truthy. * * @param mixed $value - * @param callable $callback + * @param callable|null $callback * @param callable|null $default * * @return mixed */ - public function when($value, $callback, $default = null) + public function when($value, callable $callback = null, callable $default = null) { + if (! $callback) { + return new HigherOrderWhenProxy($this, $value); + } + if ($value) { return $callback($this, $value) ?: $this; } elseif ($default) { @@ -28,13 +34,17 @@ public function when($value, $callback, $default = null) * Apply the callback if the given "value" is falsy. * * @param mixed $value - * @param callable $callback + * @param callable|null $callback * @param callable|null $default * * @return mixed */ - public function unless($value, $callback, $default = null) + public function unless($value, callable $callback = null, callable $default = null) { + if (! $callback) { + return new HigherOrderWhenProxy($this, ! $value); + } + if (! $value) { return $callback($this, $value) ?: $this; } elseif ($default) { diff --git a/tests/Support/SupportCollectionTest.php b/tests/Support/SupportCollectionTest.php index 2b335b64f986..55adb0e9e752 100755 --- a/tests/Support/SupportCollectionTest.php +++ b/tests/Support/SupportCollectionTest.php @@ -4298,7 +4298,7 @@ public function testWhenEmpty($collection) { $data = new $collection(['michael', 'tom']); - $data = $data->whenEmpty(function ($collection) { + $data = $data->whenEmpty(function ($data) { return $data->concat(['adam']); }); @@ -4367,6 +4367,30 @@ public function testWhenNotEmptyDefault($collection) $this->assertSame(['michael', 'tom', 'adam'], $data->toArray()); } + /** + * @dataProvider collectionClassProvider + */ + public function testHigherOrderWhenAndUnless($collection) + { + $data = new $collection(['michael', 'tom']); + + $data = $data->when(true)->concat(['chris']); + + $this->assertSame(['michael', 'tom', 'chris'], $data->toArray()); + + $data = $data->when(false)->concat(['adam']); + + $this->assertSame(['michael', 'tom', 'chris'], $data->toArray()); + + $data = $data->unless(false)->concat(['adam']); + + $this->assertSame(['michael', 'tom', 'chris', 'adam'], $data->toArray()); + + $data = $data->unless(true)->concat(['bogdan']); + + $this->assertSame(['michael', 'tom', 'chris', 'adam'], $data->toArray()); + } + /** * @dataProvider collectionClassProvider */ diff --git a/tests/Support/SupportConditionableTest.php b/tests/Support/SupportConditionableTest.php new file mode 100644 index 000000000000..b4c18d1f6ebd --- /dev/null +++ b/tests/Support/SupportConditionableTest.php @@ -0,0 +1,156 @@ +when(2, function($object, $condition) use (&$conditionTriggered) { + $conditionTriggered = true; + $object->on(); + $this->assertEquals(2, $condition); + }, function($object) use (&$defaultTriggered) { + $defaultTriggered = true; + $object->off(); + }); + + $this->assertTrue($object->enabled); + $this->assertTrue($conditionTriggered); + $this->assertFalse($defaultTriggered); + } + + public function testWhenDefaultCallback() + { + $conditionTriggered = false; + $defaultTriggered = false; + + $object = (new CustomConditionableObject()) + ->when(null, function ($object) use (&$conditionTriggered) { + $conditionTriggered = true; + $object->on(); + }, function ($object, $condition) use (&$defaultTriggered) { + $defaultTriggered = true; + $object->up(); + $this->assertNull($condition); + }); + + $this->assertFalse($object->enabled); + $this->assertEquals('up', $object->direction); + $this->assertFalse($conditionTriggered); + $this->assertTrue($defaultTriggered); + } + + public function testUnlessConditionCallback() + { + $conditionTriggered = false; + $defaultTriggered = false; + + $object = (new CustomConditionableObject()) + ->unless(null, function ($object, $condition) use (&$conditionTriggered) { + $conditionTriggered = true; + $object->on(); + $this->assertNull($condition); + }, function ($object) use (&$defaultTriggered) { + $defaultTriggered = true; + $object->up(); + }); + + $this->assertTrue($object->enabled); + $this->assertEquals('down', $object->direction); + $this->assertTrue($conditionTriggered); + $this->assertFalse($defaultTriggered); + } + + public function testUnlessDefaultCallback() + { + $conditionTriggered = false; + $defaultTriggered = false; + + $object = (new CustomConditionableObject()) + ->unless(2, function ($object) use (&$conditionTriggered) { + $conditionTriggered = true; + $object->on(); + }, function ($object, $condition) use (&$defaultTriggered) { + $defaultTriggered = true; + $object->off(); + $this->assertEquals(2, $condition); + }); + + $this->assertFalse($object->enabled); + $this->assertFalse($conditionTriggered); + $this->assertTrue($defaultTriggered); + } + + public function testWhenProxy() + { + $object = (new CustomConditionableObject())->when(true)->on(); + + $this->assertInstanceOf(CustomConditionableObject::class, $object); + $this->assertTrue($object->enabled); + + $object = (new CustomConditionableObject())->when(false)->on(); + + $this->assertInstanceOf(CustomConditionableObject::class, $object); + $this->assertFalse($object->enabled); + } + + public function testUnlessProxy() + { + $object = (new CustomConditionableObject())->unless(false)->on(); + + $this->assertInstanceOf(CustomConditionableObject::class, $object); + $this->assertTrue($object->enabled); + + $object = (new CustomConditionableObject())->unless(true)->on(); + + $this->assertInstanceOf(CustomConditionableObject::class, $object); + $this->assertFalse($object->enabled); + } +} + +class CustomConditionableObject +{ + use Conditionable; + + public $enabled = false; + + public $direction = 'down'; + + public function on() + { + $this->enabled = true; + + return $this; + } + + public function off() + { + $this->enabled = false; + + return $this; + } + + public function down() + { + $this->direction = 'down'; + + return $this; + } + + public function up() + { + $this->direction = 'up'; + + return $this; + } +} From 1802eb8260964129abbaf280c3a366eb299aff61 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Tue, 8 Jun 2021 16:39:12 -0400 Subject: [PATCH 2/3] StyleCI --- tests/Support/SupportConditionableTest.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/Support/SupportConditionableTest.php b/tests/Support/SupportConditionableTest.php index b4c18d1f6ebd..e8435df879ae 100644 --- a/tests/Support/SupportConditionableTest.php +++ b/tests/Support/SupportConditionableTest.php @@ -2,10 +2,8 @@ namespace Illuminate\Tests\Support; -use Illuminate\Support\Optional; use Illuminate\Support\Traits\Conditionable; use PHPUnit\Framework\TestCase; -use stdClass; class SupportConditionableTest extends TestCase { @@ -15,11 +13,11 @@ public function testWhenConditionCallback() $defaultTriggered = false; $object = (new CustomConditionableObject()) - ->when(2, function($object, $condition) use (&$conditionTriggered) { + ->when(2, function ($object, $condition) use (&$conditionTriggered) { $conditionTriggered = true; $object->on(); $this->assertEquals(2, $condition); - }, function($object) use (&$defaultTriggered) { + }, function ($object) use (&$defaultTriggered) { $defaultTriggered = true; $object->off(); }); From 4c376415fcd6f3c54e6903556782c5ea03bff036 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 9 Jun 2021 16:53:26 -0400 Subject: [PATCH 3/3] Replace by-reference variables with exceptions --- tests/Support/SupportCollectionTest.php | 4 +- tests/Support/SupportConditionableTest.php | 75 +++++----------------- 2 files changed, 17 insertions(+), 62 deletions(-) diff --git a/tests/Support/SupportCollectionTest.php b/tests/Support/SupportCollectionTest.php index 55adb0e9e752..0f0a8af49b6d 100755 --- a/tests/Support/SupportCollectionTest.php +++ b/tests/Support/SupportCollectionTest.php @@ -4298,8 +4298,8 @@ public function testWhenEmpty($collection) { $data = new $collection(['michael', 'tom']); - $data = $data->whenEmpty(function ($data) { - return $data->concat(['adam']); + $data = $data->whenEmpty(function () { + throw new Exception('whenEmpty() should not trigger on a collection with items'); }); $this->assertSame(['michael', 'tom'], $data->toArray()); diff --git a/tests/Support/SupportConditionableTest.php b/tests/Support/SupportConditionableTest.php index e8435df879ae..c61fdd0bbee2 100644 --- a/tests/Support/SupportConditionableTest.php +++ b/tests/Support/SupportConditionableTest.php @@ -2,6 +2,7 @@ namespace Illuminate\Tests\Support; +use Exception; use Illuminate\Support\Traits\Conditionable; use PHPUnit\Framework\TestCase; @@ -9,84 +10,54 @@ class SupportConditionableTest extends TestCase { public function testWhenConditionCallback() { - $conditionTriggered = false; - $defaultTriggered = false; - $object = (new CustomConditionableObject()) - ->when(2, function ($object, $condition) use (&$conditionTriggered) { - $conditionTriggered = true; + ->when(2, function ($object, $condition) { $object->on(); $this->assertEquals(2, $condition); - }, function ($object) use (&$defaultTriggered) { - $defaultTriggered = true; - $object->off(); + }, function () { + throw new Exception('when() should not trigger default callback on a truthy value'); }); $this->assertTrue($object->enabled); - $this->assertTrue($conditionTriggered); - $this->assertFalse($defaultTriggered); } public function testWhenDefaultCallback() { - $conditionTriggered = false; - $defaultTriggered = false; - $object = (new CustomConditionableObject()) - ->when(null, function ($object) use (&$conditionTriggered) { - $conditionTriggered = true; + ->when(null, function () { + throw new Exception('when() should not trigger on a falsy value'); + }, function ($object, $condition) { $object->on(); - }, function ($object, $condition) use (&$defaultTriggered) { - $defaultTriggered = true; - $object->up(); $this->assertNull($condition); }); - $this->assertFalse($object->enabled); - $this->assertEquals('up', $object->direction); - $this->assertFalse($conditionTriggered); - $this->assertTrue($defaultTriggered); + $this->assertTrue($object->enabled); } public function testUnlessConditionCallback() { - $conditionTriggered = false; - $defaultTriggered = false; - $object = (new CustomConditionableObject()) - ->unless(null, function ($object, $condition) use (&$conditionTriggered) { - $conditionTriggered = true; + ->unless(null, function ($object, $condition) { $object->on(); $this->assertNull($condition); - }, function ($object) use (&$defaultTriggered) { - $defaultTriggered = true; - $object->up(); + }, function () { + throw new Exception('unless() should not trigger default callback on a falsy value'); }); $this->assertTrue($object->enabled); - $this->assertEquals('down', $object->direction); - $this->assertTrue($conditionTriggered); - $this->assertFalse($defaultTriggered); } public function testUnlessDefaultCallback() { - $conditionTriggered = false; - $defaultTriggered = false; - $object = (new CustomConditionableObject()) - ->unless(2, function ($object) use (&$conditionTriggered) { - $conditionTriggered = true; + ->unless(2, function () { + throw new Exception('unless() should not trigger on a truthy value'); + }, function ($object, $condition) { $object->on(); - }, function ($object, $condition) use (&$defaultTriggered) { - $defaultTriggered = true; - $object->off(); $this->assertEquals(2, $condition); }); - $this->assertFalse($object->enabled); - $this->assertFalse($conditionTriggered); - $this->assertTrue($defaultTriggered); + $this->assertTrue($object->enabled); } public function testWhenProxy() @@ -122,8 +93,6 @@ class CustomConditionableObject public $enabled = false; - public $direction = 'down'; - public function on() { $this->enabled = true; @@ -137,18 +106,4 @@ public function off() return $this; } - - public function down() - { - $this->direction = 'down'; - - return $this; - } - - public function up() - { - $this->direction = 'up'; - - return $this; - } }