Problem/Motivation

The plugin attribute/annotation deprecation currently has two separate, staged deprecations.

The current plan is/was to remove support for plugin managers that do not provide an Attribute class (in most cases, additionally to an existing annotation class for BC).

And the second stage is to remove support for not using the attribute class in a plugin, that's scheduled for removal in D13.

The reason for this is that contrib projects providing plugins for a plugin type provided by another contrib module (for example a webform handler) first need that contrib module to add support for attributes.

By forcing the plugin-type-providing contrib modules to add compatibility now, we ease the upgrade path/complexity for D13. there are a lot of modules providing plugin types, https://search.tresbien.tech/search?q=%22extends%20DefaultPluginManager%... has 1700 results.

The problem is that tooling (upgrade_status, phpstan, rector) do not yet support identifying or converting this, so it's missing visibility, will not be part of automated patches and so on (assuming it is not implemented until then).

We *could* postpone this to ease updating to D12, but we'll likely pay the price for that later.

Either way, BC support will work for contrib like it does for core, other modules can define both the attribute and the annotation. It would even possible to define an attribute class based on an assumption or not-yet-committed merge requests, but there's a risk that properties would not exist or have different names or something like that.

Steps to reproduce

Proposed resolution

Remaining tasks

Decide about removing the deprecated code path now or postpone it to D13.

I guess it still makes sense to do it, but wanted to make sure we make a deliberate decision on this.

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3577900

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

berdir created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented

A possible middle ground might be unsilencing the deprecation, I can't remember what the exact effect on CI pipelines is but it would allow sites to ignore it while being less ignorable for module maintainers.

smustgrave’s picture

smustgrave commented

This one may be above my head but +1 to unsilencing and pushing to 13, there are some modules that still haven't written attributes for their custom plugins, example entity_embed

berdir opened merge request !16483

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

> example entity_embed

embed/entity_embed have been minimally maintained for many years, so that's not exactly surprising.

Unfortunately, neither rector nor phpstan have support yet for either of the two types of annotation/attribute conversion, so there might be modules already out there that claim to be D12 compatible as the bot told them so but that are missing this.

Starting with the unsilence approach. we have deprecation tests in core, so I guess we'll see what that means.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review

Didn't figure out how to make test for an E_USER_DEPRECATED or E_USER_WARNING, possibly through a custom error handler or something.

For now, I've updated to MR to throw an exception and assert that. I think I'd prefer that option. We really want contrib to pick this up in D12 IMHO.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Lets see what committers think about adding to D12. May be good to add now

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

The issue title is a double negative, and it does not say what must provide support. Would "Require plugin managers to support attribute-based discovery" be accurate?

Can someone fill in the "Proposed resolution" section of the issue summary?

I think this issue needs a change record. Maybe it can be added to an existing one.

catch’s picture

catch
he/him
Primary language English
commented

Require isn't quite right either because you could have YAML-only discovery.

It would need to be something like 'Require attribute discovery when annotation discovery is implemented'.

godotislate’s picture

godotislate
he/him
commented
Title: Removing support for not supporting attribute-based discovery » Require plugin managers that support annotation discovery to support attribute discovery

Hopefully this title works.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Title works for me.

We already have a CR from when the deprecation was added, this just does what we said we'll do. See https://www.drupal.org/node/3522776, which is the now-you-have-to-do-it CR for the existing CR https://www.drupal.org/node/3395582 from 2023.

  • catch committed 460789dd on main 
    task: #3577900 Require plugin managers that support annotation discovery...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

I added this issue to the issue links on https://www.drupal.org/node/3522776 Agreed that this has been announced enough times. Not really different to removing a deprecated function which we also do without a new CR.

Committed/pushed to main and 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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