Convert form and render element plugin discovery to attributes

Thanks, there might be others I missed when adding all the issues, I looked for annotations that extended from Plugin perhaps these don't and that's why I missed them

📌 Convert Layout DisplayVariant, PageDisplayVariant discovery to attributes Needs review is related to this one in that both need the same solution to handle subclassing. Per #4 📌 Convert Layout DisplayVariant, PageDisplayVariant discovery to attributes Needs review there:

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.

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 is merged, so this is unblocked.

Added FormElement attribute, fixed some namespace issues, and updated some docblocks. Tests are now passing.

One thing to note: the DX having the attributes named RenderElement and FormElement while also often needing to extend plugin classes named RenderElement and FormElement is less than ideal. Maybe \Drupal\Core\Render\Element\RenderElement \Drupal\Core\Render\Element\FormElement can be deprecated and renamed \Drupal\Core\Render\Element\RenderElementBase and \Drupal\Core\Render\Element\FormElementBase? Have RenderElement and FormElement extend RenderElementBase and FormElementBase for now, then remove them altogether later?

Left some small questions.

Do you think we need a follow up to remove "as CoreRenderElement;" from D12?

Added declare strict.

Do you think we need a follow up to remove "as CoreRenderElement;" from D12?

I'd like to get some agreement about removing as CoreRenderElement and as CoreFormElement by replacing/deprecating the RenderElement and FormElement abstract classes with RenderElementBase and FormElementBase.

(I had a similar struggle extending the Constraint class for validation plugins, but that is a Symfony class, so ¯\_(ツ)_/¯.)

If we do think it's a good idea to deprecate, I think I'd prefer to do it in this issue, but that is scope expansion, so I'm fine doing it in a follow up.

Created the follow up 📌 Rename RenderElement and FormElement plugin abstract classes to RenderElementBase and FormElementBase Fixed so that the class renaming question isn't a blocker here.

Sounds like a good plan to move to a follow up.

use Drupal\Core\Render\Attribute\RenderElement;
use Drupal\Core\Render\Element\RenderElement as CoreRenderElement;

The as here doesn't make much sense - both the attribute and the abstract base class are provided by core. Given the follow-up created I think we should change this to

use Drupal\Core\Render\Attribute\RenderElement;
use Drupal\Core\Render\Element\RenderElement as RenderElementBase;

Same for FormElement were we are using use statement aliasing.

renamed

I worked on this MR, but if the only outstanding thing was to fix the alias names, I am going to move this to RTBC, since @sorlov's changes address that.

We're already doing #[ViewsArea("broken")] so let's make the change that @longwave suggests.

Updated to one line attributes

Left a comment/question on the MR

TrustedCallback attribute removed.

Appears all feedback has been addressed on this one.

Committed and pushed d1c993acdf to 11.x and f6a07d869c to 10.3.x. Thanks!

quietone → closed merge request !6934

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