chore: remove ExtenderInterface[] as a conditional option, only support callable or ::class invoke (#3904)

* chore: remove ExtenderInterface[] as a conditional option, only support callable or ::class invoke

* Apply fixes from StyleCI

* stan

* review

---------

Co-authored-by: StyleCI Bot <bot@styleci.io>

extensions/mentions/extend.php

@@ -27,7 +27,6 @@ use Flarum\Post\Event\Revised;
 use Flarum\Post\Filter\PostFilterer;
 use Flarum\Post\Post;
 use Flarum\Tags\Api\Serializer\TagSerializer;
-use Flarum\Tags\Tag;
 use Flarum\User\User;
 
 return [
@@ -126,7 +125,7 @@ return [
 
     // Tag mentions
     (new Extend\Conditional())
-        ->whenExtensionEnabled('flarum-tags', [
+        ->whenExtensionEnabled('flarum-tags', fn () => [
             (new Extend\Formatter)
                 ->render(Formatter\FormatTagMentions::class)
                 ->unparse(Formatter\UnparseTagMentions::class),

framework/core/src/Extend/Conditional.php

@@ -13,17 +13,34 @@ use Flarum\Extension\Extension;
 use Flarum\Extension\ExtensionManager;
 use Illuminate\Contracts\Container\Container;
 
+/**
+ * The Conditional extender allows developers to conditionally apply other extenders
+ * based on either boolean values or results from callable functions.
+ *
+ * This is useful for applying extenders only if certain conditions are met,
+ * such as the presence of an enabled extension or a specific configuration setting.
+ */
 class Conditional implements ExtenderInterface
 {
     /**
-     * @var array<array{condition: bool|callable, extenders: ExtenderInterface[]}>
+     * An array of conditions and their associated extenders.
+     *
+     * Each entry should have:
+     * - 'condition': a boolean or callable that should return a boolean.
+     * - 'extenders': a callable returning an array of extenders, or an invokable class string.
+     *
+     * @var array<array{condition: bool|callable, extenders: callable|string}>
      */
     protected array $conditions = [];
 
     /**
-     * @param ExtenderInterface[] $extenders
+     * Apply extenders only if a specific extension is enabled.
+     *
+     * @param string $extensionId The ID of the extension.
+     * @param callable|string $extenders A callable returning an array of extenders, or an invokable class string.
+     * @return self
      */
-    public function whenExtensionEnabled(string $extensionId, array $extenders): self
+    public function whenExtensionEnabled(string $extensionId, callable|string $extenders): self
     {
         return $this->when(function (ExtensionManager $extensions) use ($extensionId) {
             return $extensions->isEnabled($extensionId);
@@ -31,16 +48,28 @@ class Conditional implements ExtenderInterface
     }
 
     /**
-     * @param ExtenderInterface[] $extenders
+     * Apply extenders only if a specific extension is disabled.
+     *
+     * @param string $extensionId The ID of the extension.
+     * @param callable|string $extenders A callable returning an array of extenders, or an invokable class string.
+     * @return self
      */
-    public function whenExtensionDisabled(string $extensionId, array $extenders): self
+    public function whenExtensionDisabled(string $extensionId, callable|string $extenders): self
     {
         return $this->when(function (ExtensionManager $extensions) use ($extensionId) {
             return ! $extensions->isEnabled($extensionId);
         }, $extenders);
     }
 
-    public function when(callable|bool $condition, array $extenders): self
+    /**
+     * Apply extenders based on a condition.
+     *
+     * @param bool|callable $condition A boolean or callable that should return a boolean.
+     *                                 If this evaluates to true, the extenders will be applied.
+     * @param callable|string $extenders A callable returning an array of extenders, or an invokable class string.
+     * @return self
+     */
+    public function when(callable|bool $condition, callable|string $extenders): self
     {
         $this->conditions[] = [
             'condition' => $condition,
@@ -50,6 +79,13 @@ class Conditional implements ExtenderInterface
         return $this;
     }
 
+    /**
+     * Iterates over the conditions and applies the associated extenders if the conditions are met.
+     *
+     * @param Container $container
+     * @param Extension|null $extension
+     * @return void
+     */
     public function extend(Container $container, Extension $extension = null): void
     {
         foreach ($this->conditions as $condition) {
@@ -58,7 +94,11 @@ class Conditional implements ExtenderInterface
             }
 
             if ($condition['condition']) {
-                foreach ($condition['extenders'] as $extender) {
+                $extenders = $condition['extenders'];
+
+                $extenders = $container->call($extenders);
+
+                foreach ($extenders as $extender) {
                     $extender->extend($container, $extension);
                 }
             }

framework/core/tests/integration/extenders/ConditionalTest.php

@@ -25,14 +25,14 @@ class ConditionalTest extends TestCase
     {
         $this->extend(
             (new Extend\Conditional())
-                ->when(true, [
+                ->when(true, function () {
+                    return [
                         (new Extend\ApiSerializer(ForumSerializer::class))
                             ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
+                                return ['customConditionalAttribute' => true];
+                            })
                     ];
                 })
-                ])
         );
 
         $this->app();
@@ -53,14 +53,14 @@ class ConditionalTest extends TestCase
     {
         $this->extend(
             (new Extend\Conditional())
-                ->when(false, [
+                ->when(false, function () {
+                    return [
                         (new Extend\ApiSerializer(ForumSerializer::class))
                             ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
+                                return ['customConditionalAttribute' => true];
+                            })
                     ];
                 })
-                ])
         );
 
         $this->app();
@@ -83,14 +83,14 @@ class ConditionalTest extends TestCase
             (new Extend\Conditional())
                 ->when(function () {
                     return true;
-                }, [
+                }, function () {
+                    return [
                         (new Extend\ApiSerializer(ForumSerializer::class))
                             ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
+                                return ['customConditionalAttribute' => true];
+                            })
                     ];
                 })
-                ])
         );
 
         $this->app();
@@ -113,14 +113,14 @@ class ConditionalTest extends TestCase
             (new Extend\Conditional())
                 ->when(function () {
                     return false;
-                }, [
+                }, function () {
+                    return [
                         (new Extend\ApiSerializer(ForumSerializer::class))
                             ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
+                                return ['customConditionalAttribute' => true];
+                            })
                     ];
                 })
-                ])
         );
 
         $this->app();
@@ -147,32 +147,25 @@ class ConditionalTest extends TestCase
                     if (! $extensions) {
                         throw new Exception('ExtensionManager not injected');
                     }
-                }, [
+                }, function () {
+                    return [
                         (new Extend\ApiSerializer(ForumSerializer::class))
                             ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
+                                return ['customConditionalAttribute' => true];
+                            })
                     ];
                 })
-                ])
         );
 
         $this->app();
     }
 
     /** @test */
-    public function conditional_disabled_extension_not_enabled_applies_extender_array()
+    public function conditional_disabled_extension_not_enabled_applies_invokable_class()
     {
         $this->extend(
             (new Extend\Conditional())
-                ->whenExtensionDisabled('flarum-dummy-extension', [
-                    (new Extend\ApiSerializer(ForumSerializer::class))
-                        ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
-                            ];
-                        })
-                ])
+                ->whenExtensionDisabled('flarum-dummy-extension', TestExtender::class)
         );
 
         $this->app();
@@ -189,20 +182,13 @@ class ConditionalTest extends TestCase
     }
 
     /** @test */
-    public function conditional_disabled_extension_enabled_does_not_apply_extender_array()
+    public function conditional_disabled_extension_enabled_does_not_apply_invokable_class()
     {
         $this->extension('flarum-tags');
 
         $this->extend(
             (new Extend\Conditional())
-                ->whenExtensionDisabled('flarum-tags', [
-                    (new Extend\ApiSerializer(ForumSerializer::class))
-                        ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
-                            ];
-                        })
-                ])
+                ->whenExtensionDisabled('flarum-tags', TestExtender::class)
         );
 
         $this->app();
@@ -219,18 +205,11 @@ class ConditionalTest extends TestCase
     }
 
     /** @test */
-    public function conditional_enabled_extension_disabled_does_not_apply_extender_array()
+    public function conditional_enabled_extension_disabled_does_not_apply_invokable_class()
     {
         $this->extend(
             (new Extend\Conditional())
-                ->whenExtensionEnabled('flarum-dummy-extension', [
-                    (new Extend\ApiSerializer(ForumSerializer::class))
-                        ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
-                            ];
-                        })
-                ])
+                ->whenExtensionEnabled('flarum-dummy-extension', TestExtender::class)
         );
 
         $this->app();
@@ -247,19 +226,12 @@ class ConditionalTest extends TestCase
     }
 
     /** @test */
-    public function conditional_enabled_extension_enabled_applies_extender_array()
+    public function conditional_enabled_extension_enabled_applies_invokable_class()
     {
         $this->extension('flarum-tags');
         $this->extend(
             (new Extend\Conditional())
-                ->whenExtensionEnabled('flarum-tags', [
-                    (new Extend\ApiSerializer(ForumSerializer::class))
-                        ->attributes(function () {
-                            return [
-                                'customConditionalAttribute' => true
-                            ];
-                        })
-                ])
+                ->whenExtensionEnabled('flarum-tags', TestExtender::class)
         );
 
         $this->app();
@@ -274,4 +246,136 @@ class ConditionalTest extends TestCase
 
         $this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']);
     }
+
+    /** @test */
+    public function conditional_does_not_instantiate_extender_if_condition_is_false_using_callable()
+    {
+        $this->extend(
+            (new Extend\Conditional())
+                ->when(false, TestExtender::class)
+        );
+
+        $this->app();
+
+        $response = $this->send(
+            $this->request('GET', '/api', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $payload = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']);
+    }
+
+    /** @test */
+    public function conditional_does_instantiate_extender_if_condition_is_true_using_callable()
+    {
+        $this->extend(
+            (new Extend\Conditional())
+                ->when(true, TestExtender::class)
+        );
+
+        $this->app();
+
+        $response = $this->send(
+            $this->request('GET', '/api', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $payload = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']);
+    }
+
+    /** @test */
+    public function conditional_does_not_instantiate_extender_if_condition_is_false_using_callback()
+    {
+        $this->extend(
+            (new Extend\Conditional())
+                ->when(false, function () {
+                    return [
+                        (new Extend\ApiSerializer(ForumSerializer::class))
+                            ->attributes(function () {
+                                return ['customConditionalAttribute' => true];
+                            })
+                    ];
+                })
+        );
+
+        $this->app();
+
+        $response = $this->send(
+            $this->request('GET', '/api', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $payload = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']);
+    }
+
+    /** @test */
+    public function conditional_does_instantiate_extender_if_condition_is_true_using_callback()
+    {
+        $this->extend(
+            (new Extend\Conditional())
+                ->when(true, function () {
+                    return [
+                        (new Extend\ApiSerializer(ForumSerializer::class))
+                            ->attributes(function () {
+                                return ['customConditionalAttribute' => true];
+                            })
+                    ];
+                })
+        );
+
+        $this->app();
+
+        $response = $this->send(
+            $this->request('GET', '/api', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $payload = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']);
+    }
+
+    /** @test */
+    public function conditional_does_not_work_if_extension_is_disabled()
+    {
+        $this->extend(
+            (new Extend\Conditional())
+                ->whenExtensionEnabled('dummy-extension-id', TestExtender::class)
+        );
+
+        $response = $this->send(
+            $this->request('GET', '/api', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $payload = json_decode($response->getBody()->getContents(), true);
+
+        $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']);
+    }
+}
+
+class TestExtender
+{
+    public function __invoke(): array
+    {
+        return [
+            (new Extend\ApiSerializer(ForumSerializer::class))
+                ->attributes(function () {
+                    return [
+                        'customConditionalAttribute' => true
+                    ];
+                })
+        ];
+    }
 }