Convert Constraint plugin discovery to attributes

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

Decided I didn't like aliasing Drupal\Core\Validation\Attribute\Constraint as ConstraintAttribute, so switched to aliasing Symfony\Component\Validator\Constraint as SymfonyConstraint.

Only did a partial review but tagged a few out of scope changes.

Method return typehints were added to address tests failing for Symfony deprecations, so not OOS.

Gotcha,

Then in that case searched for @Constraint and all instances appear to be replaced.

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

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

Could it be a 11.x only?

The alternate approach could be to do something like this:

/**
   * {@inheritdoc}
   *
   * phpcs:ignore Drupal.Commenting.FunctionComment.MissingReturnComment
   * @return string
   */
  public function validatedBy() {

Why 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?

@alexpott - Tests were failing without fixing the typehints and overridden properties. https://git.drupalcode.org/issue/drupal-3420990/-/jobs/893451

---- Drupal\Tests\Core\TypedData\RecursiveContextualValidatorTest ----
Status Group Filename Line Function
--------------------------------------------------------------------------------
Fail Other phpunit-727.xml 0 Drupal\Tests\Core\TypedData\Recursi
PHPUnit Test failed to complete; Error: PHPUnit 9.6.15 by Sebastian
Bergmann and contributors.

Testing Drupal\Tests\Core\TypedData\RecursiveContextualValidatorTest
............... 15 / 15
(100%)

Time: 00:00.179, Memory: 6.00 MB

OK (15 tests, 40 assertions)

Remaining self deprecation notices (11)

1x: Method "Symfony\Component\Validator\Constraint::getDefaultOption()"
might add "?string" as a native return type declaration in the future. Do
the same in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraint"
now to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData

1x: Method "Symfony\Component\Validator\Constraint::getRequiredOptions()"
might add "array" as a native return type declaration in the future. Do the
same in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\ValidKeysConstraint"
now to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData

1x: Method "Symfony\Component\Validator\Constraint::validatedBy()" might
add "string" as a native return type declaration in the future. Do the same
in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\UuidConstraint" now to
avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData

1x: Method "Symfony\Component\Validator\Constraint::validatedBy()" might
add "string" as a native return type declaration in the future. Do the same
in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\RegexConstraint" now
to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData

1x: Method "Symfony\Component\Validator\Constraint::validatedBy()" might
add "string" as a native return type declaration in the future. Do the same
in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\LengthConstraint" now
to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData

1x: Method "Symfony\Component\Validator\Constraint::getDefaultOption()"
might add "?string" as a native return type declaration in the future. Do
the same in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\EntityBundleExistsConstraint"
now to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData

1x: Method "Symfony\Component\Validator\Constraint::getRequiredOptions()"
might add "array" as a native return type declaration in the future. Do the
same in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\EntityBundleExistsConstraint"
now to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: Method "Symfony\Component\Validator\Constraint::validatedBy()" might
add "string" as a native return type declaration in the future. Do the same
in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\EmailConstraint" now
to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: Method "Symfony\Component\Validator\Constraint::validatedBy()" might
add "string" as a native return type declaration in the future. Do the same
in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\CountConstraint" now
to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: Method "Symfony\Component\Validator\Constraint::getDefaultOption()"
might add "?string" as a native return type declaration in the future. Do
the same in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\ComplexDataConstraint"
now to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: Method "Symfony\Component\Validator\Constraint::getRequiredOptions()"
might add "array" as a native return type declaration in the future. Do the
same in child class
"Drupal\Core\Validation\Plugin\Validation\Constraint\ComplexDataConstraint"
now to avoid errors or add an explicit @return annotation to suppress this
message.
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
Remaining direct deprecation notices (10)
1x: The "Symfony\Component\Validator\Constraints\Regex::$message"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\RegexConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Length::$maxMessage"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\LengthConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Length::$minMessage"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\LengthConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Length::$exactMessage"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\LengthConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Count::$minMessage"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\CountConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Count::$maxMessage"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\CountConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Count::$exactMessage"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\CountConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Choice::$strict"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\AllowedValuesConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Choice::$minMessage"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\AllowedValuesConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData
1x: The "Symfony\Component\Validator\Constraints\Choice::$maxMessage"
property is considered final. You should not override it in
"Drupal\Core\Validation\Plugin\Validation\Constraint\AllowedValuesConstraint".
1x in
RecursiveContextualValidatorTest::testBasicValidateWithoutConstraints from
Drupal\Tests\Core\TypedData

Should 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

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

@alexpott I ran the failing test locally, and the issue is that AttributeClassDiscovery instantiates a \ReflectionClass when getting definitions, at which point the Symfony DebugClassLoader is flagging the deprecations. AnnotatedClassDiscovery does not instantiate \ReflectionClass, so that's apparently why it hasn't come up before now.

Stack trace

DebugClassLoader.php:653, Symfony\Component\ErrorHandler\DebugClassLoader->checkAnnotations()
DebugClassLoader.php:334, Symfony\Component\ErrorHandler\DebugClassLoader->checkClass()
DebugClassLoader.php:307, Symfony\Component\ErrorHandler\DebugClassLoader->loadClass()
AttributeClassDiscovery.php:125, ReflectionClass->__construct()
AttributeClassDiscovery.php:125, Drupal\Component\Plugin\Discovery\AttributeClassDiscovery->parseClass()
AttributeDiscoveryWithAnnotations.php:98, Drupal\Core\Plugin\Discovery\AttributeDiscoveryWithAnnotations->parseClass()
AttributeClassDiscovery.php:84, Drupal\Component\Plugin\Discovery\AttributeClassDiscovery->getDefinitions()
AttributeDiscoveryWithAnnotations.php:67, Drupal\Core\Plugin\Discovery\AttributeDiscoveryWithAnnotations->getDefinitions()
DerivativeDiscoveryDecorator.php:86, Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator->getDefinitions()
StaticDiscoveryDecorator.php:56, Drupal\Component\Plugin\Discovery\StaticDiscoveryDecorator->getDefinitions()
DefaultPluginManager.php:329, Drupal\Core\Plugin\DefaultPluginManager->findDefinitions()
DefaultPluginManager.php:205, Drupal\Core\Plugin\DefaultPluginManager->getDefinitions()
DiscoveryCachedTrait.php:22, Drupal\Core\Plugin\DefaultPluginManager->getDefinition()
ConstraintFactory.php:21, Drupal\Core\Validation\ConstraintFactory->createInstance()
PluginManagerBase.php:83, Drupal\Component\Plugin\PluginManagerBase->createInstance()
ConstraintManager.php:85, Drupal\Core\Validation\ConstraintManager->create()
ContextDefinition.php:366, Drupal\Core\Plugin\Context\ContextDefinition->getConstraintObjects()
ContextDefinition.php:316, Drupal\Core\Plugin\Context\ContextDefinition->isSatisfiedBy()
ContextHandler.php:76, Drupal\Core\Plugin\Context\ContextHandler->Drupal\Core\Plugin\Context\{closure:/var/www/html/web/core/lib/Drupal/Core/Plugin/Context/ContextHandler.php:75-77}()
ContextHandler.php:77, array_filter()
ContextHandler.php:77, Drupal\Core\Plugin\Context\ContextHandler->getMatchingContexts()
ContextHandler.php:64, Drupal\Core\Plugin\Context\ContextHandler->checkRequirements()
ContextHandlerTest.php:74, Drupal\Tests\Core\Plugin\ContextHandlerTest->testCheckRequirements()
TestCase.php:1614, PHPUnit\Framework\TestCase->runTest()
TestCase.php:1220, PHPUnit\Framework\TestCase->runBare()
TestResult.php:728, PHPUnit\Framework\TestResult->run()
TestCase.php:970, PHPUnit\Framework\TestCase->run()
TestSuite.php:684, PHPUnit\Framework\TestSuite->run()
TestSuite.php:684, PHPUnit\Framework\TestSuite->run()
TestRunner.php:651, PHPUnit\TextUI\TestRunner->run()
Command.php:145, PHPUnit\TextUI\Command->run()
Command.php:98, PHPUnit\TextUI\Command::main()
phpunit:107, include()
phpunit:122, {main}()

Should we suppress the deprecation messages in .deprecation-ignore.txt?

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

Added @return as needed and suppressed other deprecation warnings and tests are passing again.
Follow up here: #3425150: Symfony deprecations in Constraint plugins

NW for a couple of minor nitpicks.

We 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

Searched for usages of @Constraint annotation and found none. All feedback addressed. (Are you able to resolve the comment threads?)

@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!