Problem/Motivation

In #3555534: Since symfony/validator 7.4: Support for evaluating options in the base Constraint class is deprecated. Initialize properties in the constructor instead., passing any value that is not an associative array or NULL as $options to the addConstraint() methods and ConstraintManager::create() was deprecated.

This issue is for the purpose of:

  • Adding the ?array type declaration for $options argument
  • Adding the string type declarations for the $name argument
  • Adding the correct return type declaration
  • Adding an array_is_list($option) check inside the methods and throwing an exception if the array is non associative
  • The #[HasNamedArgument] attribute is also removed from Symfony 8, so that attribute and the BC layer in ConstraintManager/ConstraintFactory will need to be removed as well
  • Any other relevant clean up

Steps to reproduce

Proposed resolution

Remaining tasks

Test MR branch with a Symfony 8 update to see if there any remaining incompatibilities.

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3569092

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:

About issue forks

Comments

godotislate created an issue. See original summary.

godotislate’s picture

godotislate
he/him
commented
Issue summary: View changes

godotislate opened merge request !14602

godotislate’s picture

godotislate
he/him
commented
Status: Active » Needs review

All instances of HasNamedArgument attribute have been removed.

To make things simpler, I also made ExtensionNameConstraint not have any required parameters.

Also, I left the ComplexData constraint deprecations in FieldConfigBase::getItemDefinition() and BaseFieldDefinition::getPropertyConstraints() because they might be better handled in #3570464: ComplexDataConstraint does not work correctly with typed data having a property named properties.

Couple of thoughts for follow ups:

  • Should support for constraint plugins that don't extend the Symfony Constraint class be deprecated?
  • The fallback in EntityDataDefinition::getEntityTypeId() probably could use a deprecation added to 11.x
  • The check in StringItem::generateSampleValue of $field_definition->getConstraint('UniqueField') should probably be changed to $field_definition->getConstraint('UniqueField') !== NULL. DataDefinitionInterface::getConstraint documentation should probably updated to include NULL in its return type and that it returns the constraint options (e.g., passed in from addConstraint()). And StringItemTest::testGenerateSampleValue should also be updated to match
godotislate’s picture

godotislate
he/him
commented
Title: [12.x] Update method signatures for addConstraint() and ConstraintManager::create() » [12.x] Update method signatures for addConstraint() and ConstraintManager::create() and remove HasNamedArguments use.
Priority: Normal » Major

Bumping to at least Major, since this is a Symfony 8 blocker.

smustgrave’s picture

smustgrave commented

May be related? https://www.drupal.org/project/drupal/issues/3553503

godotislate’s picture

godotislate
he/him
commented
Issue summary: View changes
godotislate’s picture

godotislate
he/him
commented
Issue tags: +Needs Review Queue Initiative
Related issues: +#3570077: Update to Symfony 8

Rebased for merge conflict.

Linking to #3570077: Update to Symfony 8 because it's either a blocker or a lot of the changes here need to be added there.

Last commit for the script byte changes were to get tests passing, because its seems like HEAD was broken upstream: #3443571-85: Mobile version of Navigation should have focus trap. Upstream fixed and MR rebased.

smustgrave’s picture

smustgrave commented

Not entirely sure how to test this one. Fact the pipeline is green?

godotislate’s picture

godotislate
he/him
commented

Not entirely sure how to test this one. Fact the pipeline is green?

Yes. And check that there are no remaining usages of HasNamedArguments.

godotislate’s picture

godotislate
he/him
commented

All deprecation references to https://www.drupal.org/node/3554746 should be gone as well.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Going to go on a limb then. Tests are passing and all instances of HasNamedArguments are gone.

godotislate’s picture

godotislate
he/him
commented
Priority: Major » Normal

Thanks!

Separately, noting that this is not a Symfony 8 blocker, at least not anymore, because AFAICT everything needed from here is already in the MR for #3570077: Update to Symfony 8. Downgrading to Normal.

quietone’s picture

quietone commented
Title: [12.x] Update method signatures for addConstraint() and ConstraintManager::create() and remove HasNamedArguments use. » Update method signatures for addConstraint() and ConstraintManager::create() and remove HasNamedArguments use
godotislate’s picture

godotislate
he/him
commented

Rebased for merge conflict.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !14602

catch closed merge request !14602

  • catch committed aea47fe1 on main 
    refactor: #3569092 Update method signatures for addConstraint() and...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.