Issue #3400458 by alexpott, Berdir, longwave, ReINFaTe, mglaman, larowlan, Charlie ChX Negyesi, kristiaanvandeneynde, cmlara: AttributeClassDiscovery fails while trying to include non valid plugin class

core/lib/Drupal/Component/Annotation/Doctrine/StaticReflectionClass.php

@@ -79,6 +79,18 @@ class StaticReflectionClass extends ReflectionClass
         return $this->staticReflectionParser->getUseStatements();
     }
 
+    /**
+     * Determines if the class has the provided class attribute.
+     *
+     * @param string $attribute The attribute to check for.
+     *
+     * @return bool
+     */
+    public function hasClassAttribute(string $attribute)
+    {
+        return $this->staticReflectionParser->hasClassAttribute($attribute);
+    }
+
     /**
      * {@inheritDoc}
      */

core/lib/Drupal/Component/Annotation/Doctrine/StaticReflectionParser.php

@@ -132,6 +132,13 @@ class StaticReflectionParser
      */
     protected $parentStaticReflectionParser;
 
+    /**
+     * The class attributes.
+     *
+     * @var string[]
+     */
+    protected array $classAttributes = [];
+
     /**
      * Parses a class residing in a PSR-0 hierarchy.
      *
@@ -178,6 +185,7 @@ class StaticReflectionParser
         $tokenParser = new TokenParser($contents);
         $docComment  = '';
         $last_token  = false;
+        $attributeNames = [];
 
         while ($token = $tokenParser->next(false)) {
             switch ($token[0]) {
@@ -187,7 +195,17 @@ class StaticReflectionParser
                 case T_DOC_COMMENT:
                     $docComment = $token[1];
                     break;
+                case T_ATTRIBUTE:
+                    while ($token = $tokenParser->next()) {
+                        if ($token[0] === T_NAME_FULLY_QUALIFIED || $token[0] === T_NAME_QUALIFIED || $token[0] === T_NAME_RELATIVE || $token[0] === T_STRING) {
+                            $attributeNames[] = $token[1];
+                            break 2;
+                        }
+                    }
+                    break;
                 case T_CLASS:
+                    // Convert the attributes to fully qualified names.
+                    $this->classAttributes = array_map(fn($name) => strtolower($this->fullySpecifyName($name)), $attributeNames);
                     if ($last_token !== T_PAAMAYIM_NEKUDOTAYIM && $last_token !== T_NEW) {
                         $this->docComment['class'] = $docComment;
                         $docComment                = '';
@@ -223,31 +241,7 @@ class StaticReflectionParser
                     $docComment                              = '';
                     break;
                 case T_EXTENDS:
-                    $this->parentClassName = $tokenParser->parseClass();
+                    $this->parentClassName = $this->fullySpecifyName($tokenParser->parseClass());
-                    $nsPos                 = strpos($this->parentClassName, '\\');
-                    $fullySpecified        = false;
-                    if ($nsPos === 0) {
-                        $fullySpecified = true;
-                    } else {
-                        if ($nsPos) {
-                            $prefix  = strtolower(substr($this->parentClassName, 0, $nsPos));
-                            $postfix = substr($this->parentClassName, $nsPos);
-                        } else {
-                            $prefix  = strtolower($this->parentClassName);
-                            $postfix = '';
-                        }
-                        foreach ($this->useStatements as $alias => $use) {
-                            if ($alias !== $prefix) {
-                                continue;
-                            }
-
-                            $this->parentClassName = '\\' . $use . $postfix;
-                            $fullySpecified        = true;
-                        }
-                    }
-                    if (! $fullySpecified) {
-                        $this->parentClassName = '\\' . $this->namespace . '\\' . $this->parentClassName;
-                    }
                     break;
             }
 
@@ -341,4 +335,53 @@ class StaticReflectionParser
         }
         throw new ReflectionException('Invalid ' . $type . ' "' . $name . '"');
     }
+
+    /**
+     * Determines if the class has the provided class attribute.
+     *
+     * @param string $attribute The fully qualified attribute to check for.
+     *
+     * @return bool
+     */
+    public function hasClassAttribute(string $attribute): bool
+    {
+        $this->parse();
+        return in_array('\\' . ltrim(strtolower($attribute), '\\'), $this->classAttributes, TRUE);
+    }
+
+    /**
+     * Converts a name into a fully specified name.
+     *
+     * @param string $name The name to convert.
+     *
+     * @return string
+     */
+    private function fullySpecifyName(string $name): string
+    {
+        $nsPos          = strpos($name, '\\');
+        $fullySpecified = false;
+        if ($nsPos === 0) {
+            $fullySpecified = true;
+        } else {
+            if ($nsPos) {
+                $prefix  = strtolower(substr($name, 0, $nsPos));
+                $postfix = substr($name, $nsPos);
+            } else {
+                $prefix  = strtolower($name);
+                $postfix = '';
+            }
+            foreach ($this->useStatements as $alias => $use) {
+                if ($alias !== $prefix) {
+                    continue;
+                }
+
+                $name = '\\' . $use . $postfix;
+                $fullySpecified        = true;
+            }
+        }
+        if (! $fullySpecified) {
+            $name = '\\' . $this->namespace . '\\' . $name;
+        }
+        return $name;
+    }
 }

core/lib/Drupal/Core/Plugin/Discovery/AttributeDiscoveryWithAnnotations.php

@@ -84,15 +84,23 @@ class AttributeDiscoveryWithAnnotations extends AttributeClassDiscovery {
     $finder = MockFileFinder::create($fileinfo->getPathName());
     $parser = new StaticReflectionParser($class, $finder, TRUE);
 
+    $reflection_class = $parser->getReflectionClass();
     // @todo Handle deprecating definitions discovery via annotations in
     // https://www.drupal.org/project/drupal/issues/3265945.
     /** @var \Drupal\Component\Annotation\AnnotationInterface $annotation */
-    if ($annotation = $this->getAnnotationReader()->getClassAnnotation($parser->getReflectionClass(), $this->pluginDefinitionAnnotationName)) {
+    if ($annotation = $this->getAnnotationReader()->getClassAnnotation($reflection_class, $this->pluginDefinitionAnnotationName)) {
       $this->prepareAnnotationDefinition($annotation, $class);
       return ['id' => $annotation->getId(), 'content' => $annotation->get()];
     }
 
-    return parent::parseClass($class, $fileinfo);
+    // Annotations use static reflection and are able to analyze a class that
+    // extends classes or uses traits that do not exist. Attribute discovery
+    // will trigger a fatal error with such classes, so only call it if the
+    // class has a class attribute.
+    if ($reflection_class->hasClassAttribute($this->pluginDefinitionAttributeName)) {
+      return parent::parseClass($class, $fileinfo);
+    }
+    return ['id' => NULL, 'content' => NULL];
   }
 
   /**

core/modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/Example3.php

@@ -7,7 +7,7 @@ use Drupal\plugin_test\Plugin\Attribute\PluginExample;
 /**
  * Provides a test plugin with a custom attribute.
  */
-#[PluginExample(
+#[/* comment */PluginExample(
   id: "example_3",
   custom: "George"
 )]

core/modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/Example4.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace Drupal\plugin_test\Plugin\plugin_test\custom_annotation;
+
+use Drupal\plugin_test\Plugin;
+
+/**
+ * Provides a test plugin with a custom attribute.
+ *
+ * This plugin ensures that StaticReflectionParser::parse() correctly determines
+ * the fully qualified attribute name.
+ *
+ * @see \Drupal\Component\Annotation\Doctrine\StaticReflectionParser::parse()
+ */
+#[Plugin\Attribute\PluginExample(
+  id: "example_4",
+  custom: "Example 4"
+)]
+#[\Attribute]
+class Example4 {}

core/modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/Example5.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace Drupal\plugin_test\Plugin\plugin_test\custom_annotation;
+
+/**
+ * Provides a test plugin with a custom attribute.
+ *
+ * This plugin ensures that StaticReflectionParser::parse() correctly determines
+ * the fully qualified attribute name.
+ *
+ * @see \Drupal\Component\Annotation\Doctrine\StaticReflectionParser::parse()
+ */
+#[\Attribute]
+#[\Drupal\plugin_test\Plugin\Attribute\PluginExample(
+  id: "example_5",
+  custom: "Example 5"
+)]
+class Example5 {}

core/modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/ExtendingNonInstalledClass.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace Drupal\plugin_test\Plugin\plugin_test\custom_annotation;
+
+use Drupal\Core\Security\Attribute\TrustedCallback;
+use Drupal\non_installed_module\NonExisting;
+
+/**
+ * This class does not have a plugin attribute or plugin annotation on purpose.
+ */
+#[\Attribute]
+class ExtendingNonInstalledClass extends NonExisting {
+
+  #[TrustedCallback]
+  public function testMethod() {}
+
+}

core/modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/UsingNonInstalledTraitClass.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace Drupal\plugin_test\Plugin\plugin_test\custom_annotation;
+
+use Drupal\Core\Security\Attribute\TrustedCallback;
+use Drupal\non_installed_module\NonExistingTrait;
+
+/**
+ * This class does not have a plugin attribute or plugin annotation on purpose.
+ */
+class UsingNonInstalledTraitClass {
+  use NonExistingTrait;
+
+  #[TrustedCallback]
+  public function testMethod() {}
+
+}

core/phpstan.neon.dist

@@ -29,6 +29,8 @@ parameters:
     - lib/Drupal/Component/Transliteration/data/*.php
     # Below extends on purpose a non existing class 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
 
   ignoreErrors:
     # new static() is a best practice in Drupal, so we cannot fix that.

core/tests/Drupal/KernelTests/Core/Plugin/DefaultPluginManagerTest.php

@@ -6,6 +6,7 @@ use Drupal\Core\Plugin\DefaultPluginManager;
 use Drupal\KernelTests\KernelTestBase;
 use Drupal\plugin_test\Plugin\Annotation\PluginExample as AnnotationPluginExample;
 use Drupal\plugin_test\Plugin\Attribute\PluginExample as AttributePluginExample;
+use org\bovigo\vfs\vfsStream;
 
 /**
  * Tests the default plugin manager.
@@ -28,12 +29,28 @@ class DefaultPluginManagerTest extends KernelTestBase {
     $namespaces = new \ArrayObject(['Drupal\plugin_test' => $base_directory]);
     $module_handler = $this->container->get('module_handler');
 
+    // Ensure broken files exist as expected.
+    try {
+      $e = NULL;
+      new \ReflectionClass('\Drupal\plugin_test\Plugin\plugin_test\custom_annotation\ExtendingNonInstalledClass');
+    }
+    catch (\Throwable $e) {
+    } finally {
+      $this->assertInstanceOf(\Throwable::class, $e);
+      $this->assertSame('Class "Drupal\non_installed_module\NonExisting" not found', $e->getMessage());
+    }
+    // Ensure there is a class with the expected name. We cannot reflect on this
+    // as it triggers a fatal error.
+    $this->assertFileExists($base_directory . '/' . $subdir . '/UsingNonInstalledTraitClass.php');
+
     // Annotation only.
     $manager = new DefaultPluginManager($subdir, $namespaces, $module_handler, NULL, AnnotationPluginExample::class);
     $definitions = $manager->getDefinitions();
     $this->assertArrayHasKey('example_1', $definitions);
     $this->assertArrayHasKey('example_2', $definitions);
     $this->assertArrayNotHasKey('example_3', $definitions);
+    $this->assertArrayNotHasKey('example_4', $definitions);
+    $this->assertArrayNotHasKey('example_5', $definitions);
 
     // Annotations and attributes together.
     $manager = new DefaultPluginManager($subdir, $namespaces, $module_handler, NULL, AttributePluginExample::class, AnnotationPluginExample::class);
@@ -41,13 +58,31 @@ class DefaultPluginManagerTest extends KernelTestBase {
     $this->assertArrayHasKey('example_1', $definitions);
     $this->assertArrayHasKey('example_2', $definitions);
     $this->assertArrayHasKey('example_3', $definitions);
+    $this->assertArrayHasKey('example_4', $definitions);
+    $this->assertArrayHasKey('example_5', $definitions);
 
     // Attributes only.
+    // \Drupal\Component\Plugin\Discovery\AttributeClassDiscovery does not
+    // support parsing classes that cannot be reflected. Therefore, we use VFS
+    // to create a directory remove plugin_test's plugins and remove the broken
+    // plugins.
+    vfsStream::setup('plugin_test');
+    $dir = vfsStream::create(['src' => ['Plugin' => ['plugin_test' => ['custom_annotation' => []]]]]);
+    $plugin_directory = $dir->getChild('src/' . $subdir);
+    vfsStream::copyFromFileSystem($base_directory . '/' . $subdir, $plugin_directory);
+    $plugin_directory->removeChild('ExtendingNonInstalledClass.php');
+    $plugin_directory->removeChild('UsingNonInstalledTraitClass.php');
+
+    $namespaces = new \ArrayObject(['Drupal\plugin_test' => vfsStream::url('plugin_test/src')]);
     $manager = new DefaultPluginManager($subdir, $namespaces, $module_handler, NULL, AttributePluginExample::class);
     $definitions = $manager->getDefinitions();
     $this->assertArrayNotHasKey('example_1', $definitions);
     $this->assertArrayNotHasKey('example_2', $definitions);
     $this->assertArrayHasKey('example_3', $definitions);
+    $this->assertArrayHasKey('example_4', $definitions);
+    $this->assertArrayHasKey('example_5', $definitions);
+    $this->assertArrayNotHasKey('extending_non_installed_class', $definitions);
+    $this->assertArrayNotHasKey('using_non_installed_trait', $definitions);
   }
 
 }

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/AttributeClass.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute;
+
+#[\Attribute]
+final class AttributeClass
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/FullyQualified.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute;
+
+#[/* Comment */\Drupal\Tests\Component\Annotation\Doctrine\Fixtures\ExtraAttributes\ExampleAttribute]
+final class FullyQualified
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/MultipleAttributes.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute;
+
+#[\Attribute /* Comment */]
+#[/* Comment */AttributeClass]
+final class MultipleAttributes
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/Qualified.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute;
+
+use Drupal\Tests\Component\Annotation\Doctrine\Fixtures\ExtraAttributes;
+#[/* Comment */ExtraAttributes\ExampleAttribute]
+final class Qualified
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/Relative.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute;
+
+#[/* Comment */SubDir\SubDirAttribute]
+final class Relative
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/SubDir/SubDirAttribute.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute\SubDir;
+
+#[\Attribute]
+final class SubDirAttribute
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/Used.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute;
+
+use Drupal\Tests\Component\Annotation\Doctrine\Fixtures\ExtraAttributes\ExampleAttribute;
+
+#[/* Comment */ExampleAttribute]
+final class Used
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/UsedAs.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute;
+
+use Drupal\Tests\Component\Annotation\Doctrine\Fixtures\ExtraAttributes\ExampleAttribute as ClassAttribute;
+
+#[/* Comment */ClassAttribute]
+final class UsedAs
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/Attribute/UsedAsQualified.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\Attribute;
+
+use Drupal\Tests\Component\Annotation\Doctrine\Fixtures\ExtraAttributes as ClassAttributes;
+
+#[/* Comment */ClassAttributes\ExampleAttribute]
+final class UsedAsQualified
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures/ExtraAttributes/ExampleAttribute.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine\Fixtures\ExtraAttributes;
+
+#[\Attribute]
+final class ExampleAttribute
+{
+}

core/tests/Drupal/Tests/Component/Annotation/Doctrine/StaticReflectionParserTest.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace Drupal\Tests\Component\Annotation\Doctrine;
+
+use Drupal\Component\Annotation\Doctrine\StaticReflectionParser;
+use Drupal\Component\Annotation\Reflection\MockFileFinder;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @coversDefaultClass \Drupal\Component\Annotation\Doctrine\StaticReflectionParser
+ *
+ * @group Annotation
+ */
+class StaticReflectionParserTest extends TestCase {
+
+  /**
+   * @testWith ["AttributeClass", "\\Attribute", true]
+   *           ["AttributeClass", "Attribute", true]
+   *           ["AttributeClass", "\\DoesNotExist", false]
+   *           ["MultipleAttributes", "Attribute", true]
+   *           ["MultipleAttributes", "Drupal\\Tests\\Component\\Annotation\\Doctrine\\Fixtures\\Attribute\\AttributeClass", true]
+   *           ["MultipleAttributes", "DoesNotExist", false]
+   *           ["FullyQualified", "Drupal\\Tests\\Component\\Annotation\\Doctrine\\Fixtures\\ExtraAttributes\\ExampleAttribute", true]
+   *           ["Used", "Drupal\\Tests\\Component\\Annotation\\Doctrine\\Fixtures\\ExtraAttributes\\ExampleAttribute", true]
+   *           ["UsedAs", "Drupal\\Tests\\Component\\Annotation\\Doctrine\\Fixtures\\ExtraAttributes\\ExampleAttribute", true]
+   *           ["UsedAsQualified", "Drupal\\Tests\\Component\\Annotation\\Doctrine\\Fixtures\\ExtraAttributes\\ExampleAttribute", true]
+   *           ["Qualified", "Drupal\\Tests\\Component\\Annotation\\Doctrine\\Fixtures\\ExtraAttributes\\ExampleAttribute", true]
+   *           ["Relative", "Drupal\\Tests\\Component\\Annotation\\Doctrine\\Fixtures\\Attribute\\SubDir\\SubDirAttribute", true]
+   */
+  public function testAttribute(string $class, string $attribute_class, bool $expected) {
+    $finder = MockFileFinder::create(__DIR__ . '/Fixtures/Attribute/' . $class . '.php');
+    $parser = new StaticReflectionParser('\\Drupal\\Tests\\Component\\Annotation\\Doctrine\\Fixtures\\Attribute\\' . $class, $finder);
+    $this->assertSame($expected, $parser->hasClassAttribute($attribute_class), "'$class' has '$attribute_class'");
+    // Attribute names and namespaces are case-insensitive in PHP. Practically
+    // Composer autoloading makes this untrue but builtins like \Attribute are
+    // case-insensitive so we should support that.
+    $this->assertSame($expected, $parser->hasClassAttribute(strtoupper($attribute_class)), "'$class' has '$attribute_class'");
+  }
+
+}