Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
In #3252386: Use PHP attributes instead of doctrine annotations we added support for attribute based plugin discovery.
As part of that issue we converted block and action plugins.
This issue is to convert \Drupal\Core\Validation\Annotation\Constraint
plugins to use Attributes.
Proposed resolution
- Add a class to represent the new Attribute - Example
- Update the plugin manager constructor to include both the attribute and annotation class names - example
- Convert all plugins that use the annotation to use the new attribute - example
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3420990
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:
- 3420990-convert-constraint-plugin changes, plain diff MR !6759
Comments
Comment #4
godotislate CreditAttribution: godotislate at Digital Polygon commentedI was implementing a custom validation constraint and noticed that Constraint plugins hadn't yet been converted to attributes, so I picked this up. I underestimated level of effort by quite a bit, first in the number of plugins there are, and second the Symfony deprecation notices. Anyway, tests are green, so I assume I caught everything.
Comment #5
godotislate CreditAttribution: godotislate at Digital Polygon commentedDecided I didn't like aliasing
Drupal\Core\Validation\Attribute\Constraint as ConstraintAttribute
, so switched to aliasingSymfony\Component\Validator\Constraint as SymfonyConstraint
.Comment #6
smustgrave CreditAttribution: smustgrave at Mobomo commentedOnly did a partial review but tagged a few out of scope changes.
Comment #7
godotislate CreditAttribution: godotislate at Digital Polygon commentedMethod return typehints were added to address tests failing for Symfony deprecations, so not OOS.
Comment #8
smustgrave CreditAttribution: smustgrave at Mobomo commentedGotcha,
Then in that case searched for @Constraint and all instances appear to be replaced.
Comment #9
longwaveTo backport this to 10.3.x I am not sure we can add the return types, we might have to add them to the baseline or ignore them instead.
Comment #10
godotislate CreditAttribution: godotislate at Digital Polygon commentedNot sure if this suggests whether more typehints can be backported, but there are existing typehints on some constraint plugins: Drupal\Core\Entity\Plugin\Validation\Constraint\BundleConstraint
Drupal\Core\Plugin\Plugin\Validation\Constraint\PluginExistsConstraint
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/BundleConstraint.php?ref_type=heads#L51
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/BundleConstraint.php?ref_type=heads#L58
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
Comment #11
smustgrave CreditAttribution: smustgrave at Mobomo commentedCould it be a 11.x only?
Comment #12
godotislate CreditAttribution: godotislate at Digital Polygon commentedThe alternate approach could be to do something like this:
Comment #13
alexpottWhy is this issue fixing deprecations as well. I feel this should only be doing the attribute conversion. The rest of the changes belong in a separate issue. - no?
Comment #14
godotislate CreditAttribution: godotislate at Digital Polygon commented@alexpott - Tests were failing without fixing the typehints and overridden properties. https://git.drupalcode.org/issue/drupal-3420990/-/jobs/893451
Comment #15
godotislate CreditAttribution: godotislate at Digital Polygon commentedShould the deprecations be taken care of in a separate issue and then block this issue on the deprecations one?
Similar issues: #3233482: [Symfony 6] Add type hints to methods overriding Symfony\Component\Validator\Constraint::getDefaultOption() and ::getRequiredOptions(), #3276196: The "Symfony\Component\Validator\Constraints\Range::$minMessage" property is considered final
Comment #16
alexpott@godotislate why are we suddenly hitting this when we move to attributes? This is already the case. My guess is the conversion is breaking how we are already skipping it so we should fix that here.
Comment #17
godotislate CreditAttribution: godotislate at Digital Polygon commented@alexpott I ran the failing test locally, and the issue is that
AttributeClassDiscovery
instantiates a\ReflectionClass
when getting definitions, at which point the SymfonyDebugClassLoader
is flagging the deprecations.AnnotatedClassDiscovery
does not instantiate\ReflectionClass
, so that's apparently why it hasn't come up before now.Stack trace
Should we suppress the deprecation messages in
.deprecation-ignore.txt
?Comment #18
alexpottYes let's suppress and use documentation - i.e add @return where we can. This is another good thing about attributes - they get checked better by our tools.
Comment #19
godotislate CreditAttribution: godotislate at Digital Polygon commentedAdded @return as needed and suppressed other deprecation warnings and tests are passing again.
Follow up here: #3425150: Symfony deprecations in Constraint plugins
Comment #20
kim.pepperNW for a couple of minor nitpicks.
Comment #21
kim.pepperWe also need to update https://git.drupalcode.org/project/drupal/-/blob/11.x/core/core.api.php?... to replace
@see \Drupal\Core\Validation\Annotation\Constraint
with@see \Drupal\Core\Validation\Attribute\Constraint
Comment #22
kim.pepperSearched for usages of
@Constraint
annotation and found none. All feedback addressed. (Are you able to resolve the comment threads?)Comment #23
alexpott@godotislate nice work and creating the follow-up #3425150: Symfony deprecations in Constraint plugins super nice!
Committed and pushed 045dfe1b96 to 11.x and b2971f464b to 10.3.x. Thanks!