Issue #3399036 follow-up by Wim Leers, godotislate: CKEditor5PluginManager:...

Issue #3399036 follow-up by Wim Leers, godotislate: CKEditor5PluginManager: use PHP attributes instead of doctrine annotations

core/modules/ckeditor5/src/Attribute/CKEditor5Plugin.php

@@ -6,6 +6,8 @@
 
 use Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition;
 use Drupal\Component\Plugin\Attribute\Plugin;
+use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException;
+use Drupal\Core\StringTranslation\TranslatableMarkup;
 
 /**
  * The CKEditor5Plugin attribute.
@@ -55,6 +57,39 @@ public function __construct(
     array|DrupalAspectsOfCKEditor5Plugin|null $drupal = NULL,
     public readonly ?string $deriver = NULL,
   ) {
+    // If either of the two aspects of the plugin definition is in array form,
+    // then this is a YAML-defined CKEditor 5 plugin definition. To avoid errors
+    // due to violating either Attribute class constructor, verify basic data
+    // shape requirements here. This provides a better DX for YAML-defined
+    // plugins, and avoids the need for a PHP IDE or debugger.
+    // @see \Drupal\ckeditor5\Plugin\CKEditor5PluginManager::processDefinition()
+    // @see \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::validateCKEditor5Aspects()
+    // @see \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::validateDrupalAspects()
+    if (!$drupal instanceof DrupalAspectsOfCKEditor5Plugin) {
+      if ($drupal === NULL) {
+        throw new InvalidPluginDefinitionException($id, sprintf('The "%s" CKEditor 5 plugin definition must contain a "drupal" key.', $id));
+      }
+      // TRICKY: $this->deriver is incorrect due to AttributeBridgeDecorator!
+      // If there's no deriver, validate here. Otherwise: the base definition is
+      // allowed to be incomplete; let CKEditor5PluginManager::processDefinition
+      // perform the validation.
+      // @see \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::getDeriver()
+      // @see \Drupal\Component\Plugin\Discovery\AttributeBridgeDecorator::getDefinitions()
+      if (!isset($drupal['deriver'])) {
+        if (isset($drupal['label']) && !is_string($drupal['label']) && !$drupal['label'] instanceof TranslatableMarkup) {
+          throw new InvalidPluginDefinitionException($id, sprintf('The "%s" CKEditor 5 plugin definition has a "drupal.label" value that is not a string nor a TranslatableMarkup instance.', $id));
+        }
+        if (!$ckeditor5 instanceof CKEditor5AspectsOfCKEditor5Plugin) {
+          if ($ckeditor5 === NULL) {
+            throw new InvalidPluginDefinitionException($id, sprintf('The "%s" CKEditor 5 plugin definition must contain a "ckeditor5" key.', $id));
+          }
+          if (!isset($ckeditor5['plugins'])) {
+            throw new InvalidPluginDefinitionException($id, sprintf('The "%s" CKEditor 5 plugin definition must contain a "ckeditor5.plugins" key.', $id));
+          }
+        }
+      }
+    }
+
     $this->ckeditor5 = is_array($ckeditor5) ? new CKEditor5AspectsOfCKEditor5Plugin(...$ckeditor5) : $ckeditor5;
     $this->drupal = is_array($drupal) ? new DrupalAspectsOfCKEditor5Plugin(...$drupal) : $drupal;
   }

core/modules/ckeditor5/tests/src/Kernel/CKEditor5PluginManagerTest.php

@@ -225,15 +225,16 @@ public static function providerTestInvalidPluginDefinitions(): \Generator {
 
     yield 'only plugin ID, nothing else' => [
       <<<YAML
-foo_bar: {}
+ckeditor5_invalid_plugin_foo_bar: {}
 YAML,
       InvalidPluginDefinitionException::class,
-      'The "foo_bar" CKEditor 5 plugin definition must have a plugin ID that starts with "ckeditor5_invalid_plugin_".',
+      'The "ckeditor5_invalid_plugin_foo_bar" CKEditor 5 plugin definition must contain a "drupal" key.',
     ];
 
-    yield 'fixed plugin ID' => [
+    yield 'added drupal' => [
       <<<YAML
-ckeditor5_invalid_plugin_foo_bar: {}
+ckeditor5_invalid_plugin_foo_bar:
+  drupal: {}
 YAML,
       InvalidPluginDefinitionException::class,
       'The "ckeditor5_invalid_plugin_foo_bar" CKEditor 5 plugin definition must contain a "ckeditor5" key.',
@@ -243,22 +244,13 @@ public static function providerTestInvalidPluginDefinitions(): \Generator {
       <<<YAML
 ckeditor5_invalid_plugin_foo_bar:
   ckeditor5: {}
-YAML,
-      \ArgumentCountError::class,
-      NULL,
-    ];
-
-    yield 'added ckeditor5.plugins' => [
-      <<<YAML
-ckeditor5_invalid_plugin_foo_bar:
-  ckeditor5:
-    plugins: {}
+  drupal: {}
 YAML,
       InvalidPluginDefinitionException::class,
-      'The "ckeditor5_invalid_plugin_foo_bar" CKEditor 5 plugin definition must contain a "drupal" key.',
+      'The "ckeditor5_invalid_plugin_foo_bar" CKEditor 5 plugin definition must contain a "ckeditor5.plugins" key.',
     ];
 
-    yield 'added drupal' => [
+    yield 'added ckeditor5.plugins' => [
       <<<YAML
 ckeditor5_invalid_plugin_foo_bar:
   ckeditor5:
@@ -277,7 +269,8 @@ public static function providerTestInvalidPluginDefinitions(): \Generator {
   drupal:
     label: {}
 YAML,
-      \TypeError::class,
+      InvalidPluginDefinitionException::class,
+      'The "ckeditor5_invalid_plugin_foo_bar" CKEditor 5 plugin definition has a "drupal.label" value that is not a string nor a TranslatableMarkup instance.',
     ];
 
     yield 'fixed drupal.label' => [
@@ -347,6 +340,21 @@ public static function providerTestInvalidPluginDefinitions(): \Generator {
 YAML,
     ];
 
+    yield 'change plugin ID to something invalid' => [
+      <<<YAML
+foo_bar:
+  ckeditor5:
+    plugins: {}
+  drupal:
+    label: "Foo bar"
+    elements:
+      - <foo>
+      - <bar>
+YAML,
+      InvalidPluginDefinitionException::class,
+      'The "foo_bar" CKEditor 5 plugin definition must have a plugin ID that starts with "ckeditor5_invalid_plugin_".',
+    ];
+
     yield 'alternative fix for drupal.elements' => [
       <<<YAML
 ckeditor5_invalid_plugin_foo_bar: