AttributeClassDiscovery should throw an exception if it finds more than one plugin type annotation

Sandeep Sanwale → made their first commit to this issue’s fork.

Thanks for the MR.

LGTM!

updated the docblock for InvalidPluginDefinitionException .

What is the impact of having more than one attribute? What actually breaks?

> What is the impact of having more than one attribute? What actually breaks?

Nothing breaks, but only the first attribute is read:

    if ($attributes = $reflection_class->getAttributes($this->pluginDefinitionAttributeName, \ReflectionAttribute::IS_INSTANCEOF)) {
      /** @var \Drupal\Component\Plugin\Attribute\AttributeInterface $attribute */
      $attribute = $attributes[0]->newInstance();

This change is just for DX, so developers understand why the code they've added does nothing.

This is also failing static analysis:

 ------ ------------------------------------------------------------------------ 
  Line   core/lib/Drupal/Component/Plugin/Discovery/AttributeClassDiscovery.php  
 ------ ------------------------------------------------------------------------ 
  131    Instantiated class                                                      
         Drupal\Component\Plugin\Discovery\InvalidPluginDefinitionException      
         not found.                                                              
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols     
 ------ ------------------------------------------------------------------------ 

Both adding test coverage for the expected exception and fixing #10 could be good novice contributor tasks.

Fixed the missing class.

Not sure what to do about tests, as the current MR is actually causing AttributeClassDiscoveryTest::testGetDefinitions() to fail, because it's doing **PRECISELY** what this MR sets out to prevent from happening!

The problem is that we have:

class CustomPlugin extends Plugin {

and

#[Plugin(
  id: "discovery_test_1",
)]
#[CustomPlugin(
  id: "discovery_test_1",
  title: "Discovery test plugin"
)]
class AttributeDiscoveryTest1 {}

Some git digging is required to understand what that test is trying to test and why it was set up that way. Is it specifically trying to test a class with two different attributes (and why??)? Or was that just a space-saving shortcut which we can change?