Needs review
Project:
Webform
Version:
6.3.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
22 Jul 2025 at 13:19 UTC
Updated:
7 Oct 2026 at 21:15 UTC
Jump to comment: Most recent
Use PHP attributes instead of annotations for plugins, see change record "Plugin types should use PHP attributes instead of annotations".
Use PHP attributes instead of annotations for plugins.
Implement.
None.
None.
None.
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
Comment #3
primsi commentedUsed rector for the initial conversion, but there was still quite a bit of work still needed to get the code style and other things right.
Comment #5
jrockowitz commentedI merged the latest dev release. Let's see if all the tests pass with this change.
#3 Yep, I saw that rector did not do a great job with the attribute code style. Thank you for doing the manual cleanup up
Comment #6
jrockowitz commentedI am able to replicate the failing tests but can't determine why they are broken with these changes.
Comment #8
idebr commentedThe merge request now contains the completed conversion and includes the following changes:
apiproperty is nownullwhen it is missing in the Plugin declaration similar todescriptionWebform::getHandlers()filters by theresultsas integer, butresultsis coded as a boolean in the plugin declaration. This is only revealed now that theresultsproperty is typecasted. See https://git.drupalcode.org/project/webform/-/merge_requests/659/diffs?co...dependencieswork a little differently with Attributes. I have kept the data structure identical for backwards compatibility: https://git.drupalcode.org/project/webform/-/merge_requests/659/diffs?co...Comment #9
idebr commentedComment #10
joshi.rohit100some minor PHP nitpicks but looks good to me
Comment #11
idebr commentedUpdated the merge request with the following changes:
Comment #12
acbramley commentedFYI it will be required to provide attribute support in D12 https://www.drupal.org/node/3522776
Comment #13
liam morlandTests are not passing.
Comment #14
berdirPosted a review of why it fails. maybe the annotation changed in the year since this was started. considering the size of webform, it probably should have been split up, but would be quite a bit of work to do that now.
also considering that this was done a year ago and rebased, it might need a review to see if any new plugins were added that need to be converted too. but that can also be done as a follow-up. It's not a huge deal if not all have been converted. The most important part is landing the attribute classes so that all other contrib and custom modules can start to adapt to this.
Comment #15
acbramley commentedI had claude review all 128 plugins to check for inconsistencies mentioned in #14, findings:
- Only 2 plugins still had annotations, I've removed those
- The only drift between annotation and attribute values were the RemotePostHandler (this was causing the test failrues) and SettingsWebformHandler (missing a comma in the description)
- Fixed a missing import in the ProcessedText plugin
- WebformAudioFile, WebformDocumentFile and WebformImageFile had
api: ''which was redundantSo overall very minor issues, this should be good to go now.
Comment #16
liam morlandThere are still some unresolved threads.
Comment #17
acbramley commented@liam morland all threads have been actioned, they just need to be resolved by the commenter or maintainer.