Create a way to declare a plugin as deprecated

Skimmed.

But seems to be a failure in the MR 2821 for D10.

rebased, failure looks unrelated

This seems like a big change.

The 2 open threads seem to be resolved as @quietone changed to "some deprecation message"

The two remaining tasks

Identify a list of relevant places that all need to be updated and create follow-up issues for that.
Update deprecation policy

Have those been completed?

For the 2 remaining tasks if they can be identified.

We have some old layouts (in my_module.layouts.yml) that we want to deprecate and use a more general use case layout instead. It would be nice to make the IS more explicit that this will cover those yaml plugins as well.

The CR has an example of deprecating a YAML plugin.

But the CR leaves a lot unsaid and is confusing in other ways:

> Methods are added to check for the deprecation when a plugin is constructed, to determine if a plugin is deprecated and to get the deprecation messaged.

Whose responsibility is it to check for a plugin being deprecated? Will core's plugin managers take care of checking their plugins, or is it up to the code that gets a plugin instance?

Created new MR for meaningful 7 commits against 11.x branch by cherry-picking and fixed PHP 8.3 compatibility in test

ATM mostly migration plugins awaiting this deprecation
Also attributes conversion may need it 📌 Triggering deprecations for plugins using annotations when core plugin type has been converted to attributes Needs work

RE #151

at least 2 blocked
- 📌 Deprecate tracker migration Postponed
- #3209220: Deprecate and remove action settings migration →

updates has policy issue #2922451: [policy no patch] Make it possible to mark plugins as deprecated →

PS: probably makes sense to file follow-up to introduce #Deprecated attribute like https://wiki.php.net/rfc/deprecated_attribute but for classes

Attempt to use the current code for migration https://git.drupalcode.org/project/drupal/-/merge_requests/6075

smustgrave → changed the visibility of the branch 9.3.x to hidden.

smustgrave → changed the visibility of the branch 3039240-create-a-way to hidden.

smustgrave → changed the visibility of the branch 3039240-plugin_deprecation to hidden.

smustgrave → changed the visibility of the branch 3039240-9.5-plugin_deprecation to hidden.

Left some small comments.

Hid the old branches for clarity.

rebased and addressed feedback

Feedback appears to be addressed! Thanks!

This seems to only add support for annotations and it relies on undocumented, undefined annotation properties to do so.

We need to make sure this works on attributes as well, which in its current state, I believe it does not, because for example the just committed Mail attribute class in 📌 Convert Mail plugin discovery to attributes Fixed has a fixed, hardcoded set of properties defined in its constructor and it is not possible to just pass in additional stuff the way this expects it to.

I think we need to do either or both of:

a) Standardize on using ...$var to pass all extra constructor arguments through to the parent. See for example ConfigEntityBase in 📌 Convert entity type discovery to PHP attributes Needs review , so that the attribute base class can introduce and define the deprecated_message property and it works an all child classes too.
b) Support an explicit additional key, which is considered here in the trait in the parent class, so that extra stuff can be put there. We discussed that a bit in the original issue but postponed its introduction on having use cases for it, more or less defining that it would be done for specific plugin types that are likely to be extended.

I agree with #166. While PluginDefinition declares AllowDynamicProperties we should not be relying on that for deprecations and should be introducing specific properties, and I think AttributeBase and AnnotationBase also need properties and setters adding to match.

To me option A from #166 seems simpler, as it means we have to explicitly declare all our properties and we will not be tempted just to dump extra keys into the "additional" property unless absolutely necessary (e.g. in the case of entity types, at least to start with).

I also tried prototyping a separate attribute, e.g.

#[Mail(
  id: 'php_mail',
  label: new TranslatableMarkup('Default PHP Mailer'),
  description: new TranslatableMarkup("Sends the message as plain text, using PHP's native mail() function."),
)]
#[Deprecated('php_mail is deprecated in drupal:11.1.0 and is removed from drupal:12.0.0. Use the symfony_mailer plugin instead.')]

but this doesn't really seem to gain us much - static analysis could find all deprecated things more easily if they all shared one attribute, but it complicates discovery as we have to check the second attribute and doesn't map well to YAML plugin definitions.

Is this going to be extensible by specific plugin types?

For example, in the menu system, we need to be able to not just say plugin 'foo' is deprecated, but to use plugin 'bar' *instead*.

@joachim in a way that needs to be understood by code and handled automatically, or only in a message to be read by humans?

> In a way that needs to be understood by code and handled automatically, or only in a message to be read by humans?

By code.

Suppose node module defines a menu link 'foo'. It wants to replace that with 'bar'.

Contrib and custom code declare their own menu link items and say 'parent: foo' because they want to place their pages within Node module's admin UI.

The menu system needs to be able to go 'Oh, I'm going to use 'bar' as a parent for that item instead because 'foo' is deprecated'.

The use case is 📌 add an entity Links handler, to provide menu links/tasks/actions for entity types Needs work , where we need to deprecate hardcoded menu links/tasks/actions and replace then with automatically-created ones.

In services this can be done with aliases; you create the new service and alias the old service name to it. Aliases can be deprecated too. Do we need this concept in plugins too?

A specific plugin system can always define additional keys on top of this, like a replaced_by or something. For most other plugins that's likely not needed or more complex, as plugins usually have configuration and the configuration of the replacement might not apply 1:1 and so on. So I'm not sure if that we need to build this into the base functionality.

The more I think about it the more I wonder what is a plugin manager, if not much more than a service container/locator that only contains a subset of services that conform to a specific interface?

@longwave: Plugins aren't (single-instance) services. Every node you load is a plugin object, so are fields and blocks and so on.

> A specific plugin system can always define additional keys on top of this, like a replaced_by or something. For most other plugins that's likely not needed or more complex

Agreed - this is not going to be needed for most plugin types. I just wanted to make sure it'll be possible to add that on top of what's being done here. This issue is the last blocker to the entity links handler issue :)

Maybe better to address Attribute and "attribution-discovery" deprecations in follow-up as ATM core needs deprecation for annotation discovery

Additionally to #167 there's PHP RFC for the #[Deprecated] attribute which has stuck
- https://github.com/php/php-src/pull/11293
- https://wiki.php.net/rfc/deprecated_attribute

But PhpStorm has implementation which has growing adoption
- https://packagist.org/packages/jetbrains/phpstorm-attributes/stats
- https://github.com/JetBrains/phpstorm-attributes