Convert Layout DisplayVariant, PageDisplayVariant discovery to attributes

This is similar to 📌 Convert form and render element plugin discovery to attributes Active in that PageDisplayVariant extends DisplayVariant here (FormElement extending RenderElement in the other issue), and the same instance of VariantManager gets all the definitions for both types of plugins together.

The tricky part is this in Drupal\Core\Plugin\Discovery\AttributeDiscoveryWithAnnotations::parseClass():

    // 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);
    }

hasClassAttribute returns TRUE if the Class .php file contains an attribute that matches $this->pluginDefinitionAttributeName, case-insensitive. This means that classes with the PageDisplayVariant attribute fail this check, and the class is not picked up as a plugin.

The approach in the MR is to add a hasImplementingClassAttribute() method to use here instead of hasClassAttribute, and instead of a case-insensitive exact string match, is_a() is used to check that the attribute class is the same class or subclass of $this->pluginDefinitionAttributeName.

I'm not sure whether this is the ideal approach, so I have not added tests for hasImplementingClassAttribute(), or even if it is OK, whether hasImplementingClassAttribute() should replace hasClassAttribute altogether since the latter method is not used anywhere else. Pushing to Needs Review though to get feedback.

Needs work to research why outdated comment was added, and not sure about tests

Removed @todo comment that seems to no longer apply. Added test for hasImplementingClassAttritbute().

Since this appears to be doing more then converting to attributes can the issue summary be updated to include what's being done/why it's needed in this ticket.

Discussed with @berdir on Slack that hasClassAttribute() can be re-implemented with is_a, so updated MR.

Per suggestion from @alexpott on Slack, handling the StaticReflectionClass changes by themselves 📌 Update Drupal\Component\Annotation\Doctrine\StaticReflectionParser::hasClassAttribute() to allow attribute subclasses Active . Blocking this on that issue.

📌 Update Drupal\Component\Annotation\Doctrine\StaticReflectionParser::hasClassAttribute() to allow attribute subclasses Active was merged, so this is unblocked now. Rebased and also updated some docblocks referencing the annotation.

Issue summary appears to be updated.

But was still tagged for tests so leaving in review for those.

Tests were done in 📌 Update Drupal\Component\Annotation\Doctrine\StaticReflectionParser::hasClassAttribute() to allow attribute subclasses Active , so removing tag.

Could a follow up be opened for

Maybe there should be a follow-up issue about investigating usage of DisplayVariant vs PageDisplayVariant, and whether there even needs to be the two classes?

Then probably good to mark this one.

Created follow up: 📌 Consolidate layout DisplayVariant, PageDisplayVariant plugin types Active

Thanks! Going to mark it now.

Post a review to the MR - I think we should require an admin label because these things have to be listed in the UI.

Changed admin_label to required and updated docblock.

Appears feedback from @alexpott has been addressed

Committed and pushed 99fc430601 to 11.x and f267065695 to 10.3.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.

godotislate → closed merge request !6818