Issue #3458177 by mondrake, catch, godotislate, longwave, xjm: Changing...

Issue #3458177 by mondrake, catch, godotislate, longwave, xjm: Changing plugins from annotations to attributes in contrib leads to error if plugin extends from a missing dependency

(cherry picked from commit 7a331d50)

core/lib/Drupal/Component/Plugin/Discovery/AttributeClassDiscovery.php

@@ -80,17 +80,30 @@ public function getDefinitions() {
               $sub_path = $iterator->getSubIterator()->getSubPath();
               $sub_path = $sub_path ? str_replace(DIRECTORY_SEPARATOR, '\\', $sub_path) . '\\' : '';
               $class = $namespace . '\\' . $sub_path . $fileinfo->getBasename('.php');
-
-              ['id' => $id, 'content' => $content] = $this->parseClass($class, $fileinfo);
-
-              if ($id) {
-                $definitions[$id] = $content;
-                // Explicitly serialize this to create a new object instance.
-                $this->fileCache->set($fileinfo->getPathName(), ['id' => $id, 'content' => serialize($content)]);
+              try {
+                ['id' => $id, 'content' => $content] = $this->parseClass($class, $fileinfo);
+                if ($id) {
+                  $definitions[$id] = $content;
+                  // Explicitly serialize this to create a new object instance.
+                  $this->fileCache->set($fileinfo->getPathName(), ['id' => $id, 'content' => serialize($content)]);
+                }
+                else {
+                  // Store a NULL object, so that the file is not parsed again.
+                  $this->fileCache->set($fileinfo->getPathName(), [NULL]);
+                }
               }
-              else {
-                // Store a NULL object, so the file is not parsed again.
-                $this->fileCache->set($fileinfo->getPathName(), [NULL]);
+              // Plugins may rely on Attribute classes defined by modules that
+              // are not installed. In such a case, a 'class not found' error
+              // may be thrown from reflection. However, this is an unavoidable
+              // situation with optional dependencies and plugins. Therefore,
+              // silently skip over this class and avoid writing to the cache,
+              // so that it is scanned each time. This ensures that the plugin
+              // definition will be found if the module it requires is
+              // enabled.
+              catch (\Error $e) {
+                if (!preg_match('/(Class|Interface) .* not found$/', $e->getMessage())) {
+                  throw $e;
+                }
               }
             }
           }
@@ -118,6 +131,7 @@ public function getDefinitions() {
    *   'content' is the plugin definition.
    *
    * @throws \ReflectionException
+   * @throws \Error
    */
   protected function parseClass(string $class, \SplFileInfo $fileinfo): array {
     // @todo Consider performance improvements over using reflection.

core/phpstan.neon.dist

@@ -31,10 +31,11 @@ parameters:
     - scripts/generate-d?-content.sh
     # Skip data files.
     - lib/Drupal/Component/Transliteration/data/*.php
-    # Below extends on purpose a non existing class for testing.
+    # The following classes deliberately extend non-existent classes for testing.
     - modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/fruit/ExtendingNonInstalledClass.php
     - modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/UsingNonInstalledTraitClass.php
     - modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/ExtendingNonInstalledClass.php
+    - tests/Drupal/Tests/Component/Plugin/Attribute/Fixtures/Plugins/PluginNamespace/AttributeDiscoveryTest2.php
 
   ignoreErrors:
     # new static() is a best practice in Drupal, so we cannot fix that.

core/tests/Drupal/Tests/Component/Plugin/Attribute/AttributeClassDiscoveryCachedTest.php

@@ -4,6 +4,7 @@
 
 namespace Drupal\Tests\Component\Plugin\Attribute;
 
+use Composer\Autoload\ClassLoader;
 use Drupal\Component\Plugin\Discovery\AttributeClassDiscovery;
 use Drupal\Component\FileCache\FileCacheFactory;
 use PHPUnit\Framework\TestCase;
@@ -20,6 +21,7 @@ class AttributeClassDiscoveryCachedTest extends TestCase {
    */
   protected function setUp(): void {
     parent::setUp();
+
     // Ensure FileCacheFactory::DISABLE_CACHE is *not* set, since we're testing
     // integration with the file cache.
     FileCacheFactory::setConfiguration([]);
@@ -28,7 +30,10 @@ protected function setUp(): void {
 
     // Normally the attribute classes would be autoloaded.
     include_once __DIR__ . '/Fixtures/CustomPlugin.php';
-    include_once __DIR__ . '/Fixtures/Plugins/PluginNamespace/AttributeDiscoveryTest1.php';
+
+    $additionalClassLoader = new ClassLoader();
+    $additionalClassLoader->addPsr4("com\\example\\PluginNamespace\\", __DIR__ . "/Fixtures/Plugins/PluginNamespace");
+    $additionalClassLoader->register(TRUE);
   }
 
   /**
@@ -41,6 +46,8 @@ public function testGetDefinitions(): void {
     $discovery_path = __DIR__ . '/Fixtures/Plugins';
     // File path that should be discovered within that directory.
     $file_path = $discovery_path . '/PluginNamespace/AttributeDiscoveryTest1.php';
+    // Define a file path within the directory that should not be discovered.
+    $non_discoverable_file_path = $discovery_path . '/PluginNamespace/AttributeDiscoveryTest2.php';
 
     $discovery = new AttributeClassDiscovery(['com\example' => [$discovery_path]]);
     $this->assertEquals([
@@ -50,11 +57,22 @@ public function testGetDefinitions(): void {
       ],
     ], $discovery->getDefinitions());
 
-    // Gain access to the file cache so we can change it.
+    // Gain access to the file cache.
     $ref_file_cache = new \ReflectionProperty($discovery, 'fileCache');
     $ref_file_cache->setAccessible(TRUE);
     /** @var \Drupal\Component\FileCache\FileCacheInterface $file_cache */
     $file_cache = $ref_file_cache->getValue($discovery);
+
+    // The valid plugin definition should be cached.
+    $this->assertEquals([
+      'id' => 'discovery_test_1',
+      'class' => 'com\example\PluginNamespace\AttributeDiscoveryTest1',
+    ], unserialize($file_cache->get($file_path)['content']));
+
+    // The plugin that extends a missing class should not be cached.
+    $this->assertNull($file_cache->get($non_discoverable_file_path));
+
+    // Change the file cache entry.
     // The file cache is keyed by the file path, and we'll add some known
     // content to test against.
     $file_cache->set($file_path, [

core/tests/Drupal/Tests/Component/Plugin/Attribute/AttributeClassDiscoveryTest.php

@@ -4,6 +4,7 @@
 
 namespace Drupal\Tests\Component\Plugin\Attribute;
 
+use Composer\Autoload\ClassLoader;
 use Drupal\Component\Plugin\Discovery\AttributeClassDiscovery;
 use Drupal\Component\FileCache\FileCacheFactory;
 use PHPUnit\Framework\TestCase;
@@ -22,6 +23,7 @@ class AttributeClassDiscoveryTest extends TestCase {
    */
   protected function setUp(): void {
     parent::setUp();
+
     // Ensure the file cache is disabled.
     FileCacheFactory::setConfiguration([FileCacheFactory::DISABLE_CACHE => TRUE]);
     // Ensure that FileCacheFactory has a prefix.
@@ -29,7 +31,10 @@ protected function setUp(): void {
 
     // Normally the attribute classes would be autoloaded.
     include_once __DIR__ . '/Fixtures/CustomPlugin.php';
-    include_once __DIR__ . '/Fixtures/Plugins/PluginNamespace/AttributeDiscoveryTest1.php';
+
+    $additionalClassLoader = new ClassLoader();
+    $additionalClassLoader->addPsr4("com\\example\\PluginNamespace\\", __DIR__ . "/Fixtures/Plugins/PluginNamespace");
+    $additionalClassLoader->register(TRUE);
   }
 
   /**

core/tests/Drupal/Tests/Component/Plugin/Attribute/Fixtures/Plugins/PluginNamespace/AttributeDiscoveryTest2.php

+<?php
+
+declare(strict_types=1);
+
+namespace com\example\PluginNamespace;
+
+use Drupal\a_module_that_does_not_exist\Plugin\Custom;
+
+/**
+ * Provides a custom test plugin that extends from a missing dependency.
+ */
+#[CustomPlugin(
+  id: "discovery_test_2",
+  title: "Discovery test plugin 2"
+)]
+class AttributeDiscoveryTest2 extends Custom {}