Problem/Motivation

Follow-up of #3252386: Use PHP attributes instead of doctrine annotations, once that is in, we need to convert all the remaining plugin types, likely an issue per plugin type, *maybe* sometimes grouped together, like all the tests and maybe views as well, although there will be a lot there.

Once that is done, the old discovery as well as calling DefaultPluginManager with old arguments should trigger deprecation messages, either then in this issue or another child issue.

#3400458: AttributeClassDiscovery fails while trying to include non valid plugin class is about to fix some critical regressions in the attribute discovery at least when using the BC layer. There are some remaining questions about whether or not we want to support such cases (classes that can not be loaded due to missing traits, base classes, interfaces, ...)

Another step that is needed is updating plugin system documentation, issue for that: #3401822: Document attribute-based discovery in the handbook.

Steps to reproduce

Proposed resolution

Convert all plugin types and provide Rector rules via https://github.com/palantirnet/drupal-rector/pull/257

Remaining tasks

#3265945: Triggering deprecations for plugins using annotations when core plugin type has been converted to attributes
#3400121: Allow opt-out of annotation parsing

Plugin conversions

Done

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3396165

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:

Comments

Berdir created an issue. See original summary.

berdir’s picture

Issue summary: View changes
berdir’s picture

Status: Postponed » Active

The blocker is committed.

andypost’s picture

andypost’s picture

wim leers’s picture

Issue summary: View changes
catch’s picture

Meta question, is this enough of a meta issue to encompass the entirety of moving from annotations to attributes, or should we also open a parent 'plan' issue that encompasses this and any other attribute issues that come up (like deprecations and so on)?

longwave’s picture

I think we should start by opening child issues here to finish off actions (only needs deprecations now?) and blocks (two remaining blocks plus deprecations?) and then go from there.

berdir’s picture

#3265945: Triggering deprecations for plugins using annotations when core plugin type has been converted to attributes will add deprecations for all plugin types that use the BC-discovery automatically, doesn't need a separate issue for actions. Blocks maybe could use a separate issue, although the layout builder one kind of already has one issue.

IMHO this is enough for the meta issue to cover relate tasks as well?

wim leers’s picture

Issue summary: View changes
longwave’s picture

berdir’s picture

Issue summary: View changes

I'm a bit unsure if we should wait on rector to do at least the bigger ones. I guess entity is by far the most complex with 100+ very large attribute definitions. @andypost started something there, but unsure how feasible it is to have something that works in a reasonable timeframe, this is far more complex than most deprecations.

This is something we have to be aware of with this change. As necessary as it is to remove doctrine annotations and as much better the new DX is, this might be the single biggest deprecation that we've had since 8.0 (deliberately excluding some pre 8.0 things). There's barely a module out there with not at least a single plugin and *many* contrib modules defining their own plugin types that other contrib modules will first need to wait to support attributes, sites will break when updating the wrong module first and dependency versions aren't clearly defined. This is going to be painful.

One thing to consider I guess is to fork the doctrine stuff even further so we keep support for Annotations until D12?

catch’s picture

@berdir @longwave just opened a new issue to discuss that #3400121: [policy, no patch] Allow both annotations and attributes in Drupal 11

berdir’s picture

Issue summary: View changes
berdir’s picture

Issue summary: View changes
larowlan’s picture

Issue summary: View changes
larowlan’s picture

Issue summary: View changes
larowlan’s picture

Issue summary: View changes

Added child issues for all plugin types except \Drupal\plugin_test_extended\Plugin\Annotation\PluginExtended which we can sort out later.

Updated issue summary with list

longwave’s picture

Issue summary: View changes
godotislate’s picture

Issue summary: View changes
quietone’s picture

Issue summary: View changes

Unless I am mistaken, there is also the views handler, \Drupal\views\Plugin\ViewsHandlerManager.

andypost’s picture

Issue summary: View changes

Updated IS with link to https://github.com/palantirnet/drupal-rector/pull/257

@bbrala polished 10.2 conversions already

@quietone yes, views plugins are the last and could use own META

mstrelan’s picture

Issue summary: View changes

Added #3425568: Block attribute class expecting wrong ContextDefinition which may also impact other plugins too

mstrelan’s picture

berdir’s picture

I realized that we forgot about another generic plugin feature beside deriver, the provider. It isn't used a lot in core for obvious reasons, but it's a valid use case to implement a plugin if another module exists. There are some challenges with it in case it depends on base classes/traits like \Drupal\layout_builder\Plugin\Block\InlineBlock (which doesn't use provider because the deriver kind of takes care of that but it would be if not using a deriver)

\Drupal\book\Plugin\migrate\destination\Book is another example in core, which has been made redundant due to the move to book module, and it's being removed in #3421015: Convert MigrateDestination plugin discovery to attributes.

The most common use case is contrib modules providing plugins for core or other contrib modules that may or may not be there. entity_browser does that for some of its plugins, linkit does it, pathauto for forum module, and our monitoring project. Those cases are all for their own plugin types, so those modules are absolutely free to define it, but it won't be possible to do it directly in the attribute for core plugin types. There are not many examples that would need that, one that I found is https://git.drupalcode.org/project/crop_image/-/blob/1.0.x/src/Plugin/Fi..., which provides a field widget plugin if entity_browser is enabled.

Not sure what we should do about that :)

bbrala’s picture

bbrala’s picture

Released here:

https://github.com/palantirnet/drupal-rector/releases/tag/0.20.1

Includes usage docs for Core conversions. Hopefully it helps speed things up.

mstrelan’s picture

Issue summary: View changes

Added 7 more views plugin types for conversion

bbrala’s picture

Fun fact, you can run the rector on an existing MR to check if something was missed. It will skip over any classes already implementing the attribute.

feuerwagen’s picture

Issue summary: View changes

Sorted the task list a bit

godotislate’s picture

feuerwagen’s picture

Issue summary: View changes
quietone’s picture

Issue summary: View changes
naveenvalecha’s picture

Issue summary: View changes
godotislate’s picture

Issue summary: View changes
naveenvalecha’s picture

Issue summary: View changes
quietone’s picture

Issue summary: View changes

Issue summary updates: Remove Tip from the issue summary because it will be done in contrib. And added #3265945: Triggering deprecations for plugins using annotations when core plugin type has been converted to attributes.

quietone’s picture

Issue summary: View changes
quietone’s picture

Issue summary: View changes
bbrala’s picture

Earlier this week I did a check using a branch with all converted plugins in rector against head. Source of the list was the change record.

https://github.com/palantirnet/drupal-rector/tree/feature/annotation-rules

Good news, there were no conversions missing.

bbrala’s picture

@Berdir sent me a message there were a few missing.

Examples:

PreviewAwareBlock
AnnounceBlock

So i've looked into why this is. Might be me doing not smart things while scripting.

bbrala’s picture

Ok, i've done a booboo when making the script. Aftersome tweaks:

AnnounceBlock.php
InlineBlock.php
PreviewAwareBlock.php
TestXSSTitleBlock.php

The BooBoo were a few things:

My config added classes instead of the annotations. Rector didnt like backslashes before the classes, so did some extra normization actions. I'll push how i ran on core to a branch here.

bbrala’s picture

andypost’s picture

@bbrala great job! Looks only formatting left to apply and it could be reused for contrib!

quietone’s picture

Issue summary: View changes
quietone’s picture

Issue summary: View changes
naveenvalecha’s picture

Issue summary: View changes
quietone’s picture

godotislate’s picture

Issue summary: View changes
naveenvalecha’s picture

Issue summary: View changes
godotislate’s picture

catch’s picture

Issue tags: +beta target

Tagging this so it's easier to find in 'things that would be good to do for 11.0/10.3 beta'.

joegl’s picture

This may be the wrong place for this (apologies if so).

The change record and documentation state Drupal 10.2.0 plugins should start using attribute-based instead of annotation-based plugins, but the core plugins don't actually support attribute-based discovery until 10.3.x. Can there be more clear documentation stating it's not well-supported yet until 10.3.x?

Relevant links:
- https://www.drupal.org/node/3403921 (10.2)
- https://www.drupal.org/node/3395575 (10.2)
- https://www.drupal.org/node/3229001 (10.3)

godotislate’s picture

Issue summary: View changes
larowlan’s picture

quietone’s picture

Issue summary: View changes