Problem/Motivation

Use PHP attributes instead of annotations for plugins, see change record "Plugin types should use PHP attributes instead of annotations".

Proposed resolution

Use PHP attributes instead of annotations for plugins.

Remaining tasks

Implement.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork webform-3537358

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

primsi created an issue. See original summary.

primsi’s picture

Used 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.

jrockowitz made their first commit to this issue’s fork.

jrockowitz’s picture

Status: Active » Needs review

I 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

jrockowitz’s picture

Status: Needs review » Needs work

I am able to replicate the failing tests but can't determine why they are broken with these changes.

idebr made their first commit to this issue’s fork.

idebr’s picture

Title: Use PHP attributes instead of doctrine annotations for plugin type definitions » Support PHP attributes for plugin definitions
Status: Needs work » Needs review

The merge request now contains the completed conversion and includes the following changes:

  1. The api property is now null when it is missing in the Plugin declaration similar to description
  2. Webform::getHandlers() filters by the results as integer, but results is coded as a boolean in the plugin declaration. This is only revealed now that the results property is typecasted. See https://git.drupalcode.org/project/webform/-/merge_requests/659/diffs?co...
  3. Plugin dependencies work 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...
idebr’s picture

Title: Support PHP attributes for plugin definitions » Support PHP attributes for plugin definitions and convert Webform's own plugins
joshi.rohit100’s picture

some minor PHP nitpicks but looks good to me

idebr’s picture

Updated the merge request with the following changes:

  1. The Attribute class is now referenced as a class constant in the Plugin managers
  2. Updated nullable data types to use the question mark syntax instead of a union type
acbramley’s picture

FYI it will be required to provide attribute support in D12 https://www.drupal.org/node/3522776

liam morland’s picture

Issue summary: View changes
Status: Needs review » Needs work

Tests are not passing.

berdir’s picture

Posted 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.

acbramley’s picture

Status: Needs work » Needs review

I 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 redundant

So overall very minor issues, this should be good to go now.

liam morland’s picture

There are still some unresolved threads.

acbramley’s picture

@liam morland all threads have been actioned, they just need to be resolved by the commenter or maintainer.