Add an assert that ensures all attribute plugins support derivers

Caught a missing deriver in PluginExample 👌

Seemed to cause a kernel failure.

Shall we add a change record and point to it in the assertion?

Definitely
I did a search for contrib uses of the Plugin attribute but didn't find any

As expected PHPCS doesn't like this.

I think keeping $id and $deriver in the docs is more useful than adding $base

Will see if I can find a way to do both

If you satisfy PHPCS by adding $base, then this breaks auto-complete in IDEs, both `id` and `deriver` aren't suggested. They are type checked if you add @param string ...$base

Added a draft change record

Ready for review

larowlan → credited Berdir → .

Crediting @Berdir as this was his idea

I think this is a good idea. What i notice right now is when I read $base I keep asking myself 'base what?' Can $base be more specific? Maybe $base_definitions?

Concerns about variadics per reviews

In two minds about this. On one hand this future proofs us for adding more base properties. On the other hand the only reason we are doing this is that since the invention of plugins when we had two properties, we are only now looking to add a third, so is this adding DX complexity for something we might never use again?

We could also detect future missing properties at discovery time by reflecting the plugin type class and raising a deprecation if an expected constructor argument (such as $deprecation or whatever else we add) is missing.

Also how will this work with plugin types where we intend to use variadics already to handle any number of additional properties? (such as entity types)

I'm not sure about this. What happens to IDE completion with this? I think IDE completion of plugin attributes is more than just a nice to have.

I think we could detect missing base variables in a different way - like we could change the validator here to ensure that the plugin constructor contains a superset of the arguments of \Drupal\Component\Plugin\Attribute\Plugin::__construct() and preserve the IDE capabilities.

#20: Yes, both a dynamic internal and dynamic parent list of properties doesn't work for EntityType. That said, I'm not sure if I even implement that yet, I think it would also be acceptable to require that extra stuff is manually placed inside additional when defined on as an Attribute.

If we want to won't fix this then fair, it just means that we can't introduce generic new features to the plugin system the way 📌 Create a way to declare a plugin as deprecated Needs work currently does, each plugin type that wants it will need to define it. That might be OK, because it will only actually do anything if there is logic for it, for example what we had in the block UI, where it didn't show deprecated plugins unless they were preconfigured.

Returned to the original intent, which was an assert to catch plugin attribute classes that forget the deriver argument

Back to needs review

The MR is fine but I am concerned about the implication for deprecating as explained in #22. It seems like we could make our lives easier in the future if we avoided having to create a way to deprecate for according to each plugin type. Could we not add a 'deprecated' and a 'deprecation_message' property now? What are the implications of doing that now? I would like to understand the pros and cons of that.
Thanks

Following

So would the new properties need to be added to each plugin or be generic?

With the current state, each plugin

is there a way to add test (expect assertion)? it maybe tricky after 📌 Remove mentions of assert.active from .htaccess Needs work

Would the addition of new properties to each plugins be a big lift?

As mentioned in slack this seems like something that probably should go in as probably more then 50% of the attribute tickets have made it in.

Only question I have is should a follow up be opened up to discuss the deprecation question #24 from @quietone?

A few type hints to fix. Probably worth a rebase to see if the tests still pass.