Allow object-based plugin definitions to be processed in DerivativeDiscoveryDecorator

After a bit of thinking and discussion this seems great. My only reservation at this point is that we need test coverage for the setDeriver method.

Eclipse

Re #6: I personally don't necessarily agree with that standard, but we do generally test getters and setters. You can test both of them neatly in one test method. Don't feel strongly about this, though.

My main reservation about this, is why this is not a dedicated interface. Not all plugin definitions are necessarily derivable, so it seems sensible to keep the current separation of providing a separate DerivablePluginDefinitionInterface.

similarity index 58%
rename from core/lib/Drupal/Core/Layout/DerivablePluginDefinitionInterface.php

rename from core/lib/Drupal/Core/Layout/DerivablePluginDefinitionInterface.php
rename to core/lib/Drupal/Component/Plugin/Definition/DerivablePluginDefinitionInterface.php

It was marked internal so moving it is not a problem. We have a new test which is always great.

Thanks for the quick follow-up, RTBC++!

Note that issues like this should be targeted against 8.4.x, but can be considered for backport once they are committed to that branch. See the updated alpha release policy. Thanks!

1. +++ b/core/lib/Drupal/Component/Plugin/Definition/DerivablePluginDefinitionInterface.php
   @@ -1,22 +1,11 @@
    interface DerivablePluginDefinitionInterface extends PluginDefinitionInterface {
   

   Although this is just a code move, it's a move from an @internal to not, so I think this is the time to ask if we should change DerivablePluginDefinitionInterface to not extend PluginDefinitionInterface. Note that DependentPluginDefinitionInterface in HEAD does not extend PluginDefinitionInterface.

2. +++ b/core/lib/Drupal/Component/Plugin/Discovery/DerivativeDiscoveryDecorator.php
   @@ -203,7 +204,13 @@ protected function getDeriver($base_plugin_id, $base_definition) {
   -    if ((is_array($base_definition) || ($base_definition = (array) $base_definition)) && (isset($base_definition['deriver']) && $class = $base_definition['deriver'])) {
   ...
   +    if ($class) {
          if (!class_exists($class)) {
            throw new InvalidDeriverException(sprintf('Plugin (%s) deriver "%s" does not exist.', $base_definition['id'], $class));
   

   That throw, along with the one a couple lines down so not in the diff, can now run for an object-based $base_definition, so we need to s/$base_definition['id']/$base_definition->id()/ as well. Which is maybe a reason to keep DerivablePluginDefinitionInterface as extending PluginDefinitionInterface? Though if we decide to do that, then perhaps we need a followup to make DependentPluginDefinitionInterface also extend that interface? I don't think it makes sense for those two to follow different patterns.

3. This issue is tagged "Contributed project soft blocker". Which contrib projects are soft blocked? Why can't they just make their 'deriver' property public, in which case, wouldn't HEAD work due to that it's already casting to an array?

The interdiffs since the last RTBC patch are all straightforward, so I feel ok with RTBC'ing this and remaining eligible to commit this.

Adding review credit for #7.

I'm ok with #18.1 and thanks for the fixes and tests for #18.2. Therefore, pushed to 8.4.x.

I'm not super convinced by #18.3, but as this is the last of the outstanding object-based definition issues, and the others all made their way into 8.3, I decided to cherry pick this one to 8.3 as well. In #2822752-22: Allow object-based plugin definitions to be created by non-annotated discovery, I proposed creating a single CR to cover all the object-based definition discovery improvements, so this being part of that is nice.