Add Missing Symfony Validators to Drupal Constraint Validator

Created on 7 June 2023, over 1 year ago
Updated 13 February 2024, 10 months ago

Problem/Motivation

There are many validators provided by Symfony that are not by default accessible as Drupal Constraint Validators

Steps to reproduce

Try to use any Symfony Validation Constraint that isn't currently in core - ex Positive, PositiveOrZero, Url (important one), Json, [and about 50 others)
Positive/PositiveOrZero validators are specificly needed for Spambot module. Url for JsonLD module.
Others are more theoretical need, but could get use cases for most of the weirder ones even with a few modules from commerce and address ecosystems I believe.

Proposed resolution

Add missing validators

📌 Task
Status

Needs work

Version

11.0 🔥

Component
Configuration 

Last updated 2 days ago

Created by

🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

Live updates comments and jobs are added and updated live.
  • Contributed project blocker

    It denotes an issue that prevents porting of a contributed project to the stable version of Drupal due to missing APIs, regressions, and so on.

  • Needs release manager review

    It is used to alert the release manager core committer(s) that an issue significantly affects the overall technical debt or release timeline of Drupal, and their signoff is needed. See the governance policy draft for more information.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

  • Issue created by @nickdickinsonwilde
  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Patch attached for review.
    I believe that since this is just making the symfony validators accessible to Drupal core that no tests are directly needed.

  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Done with discussion with Wim Leers and Rosie La Faive

  • Status changed to Needs review over 1 year ago
  • last update over 1 year ago
    Custom Commands Failed
  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)
  • Status changed to Needs work over 1 year ago
  • The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • 🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    Based on #5, we should probably tell cspell that luhn, issn and iban are actually correct words?

  • Status changed to Needs review over 1 year ago
  • last update over 1 year ago
    29,421 pass, 3 fail
  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Never ran cspell locally! Let's try this...

  • last update over 1 year ago
    29,436 pass
  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Fix a couple duplications

  • Status changed to Needs work over 1 year ago
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Looks GREAT!

    Could you please explain why this is a contributed project blocker? Where did you encounter a need for this? 😊

    1. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'class' => '\Symfony\Component\Validator\Constraints\Bic',
      

      Could you update these to be of the format Bic::class? 🙏

      That way any future refactoring would be simpler! :)

    2. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Cidr'),
      

      I think these should all be capitalized? 😅🤓

    3. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('CssColor'),
      

      I think the label should be "CSS Color" 🤓

    4. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Iban'),
      

      I think the label should be "IBAN" 🤓

    5. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Ip'),
      

      I think the label should be "IP address" 🤓

    6. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Isbn'),
      

      I think the label should be "ISBN" 🤓

    7. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Isin'),
      

      I think the label should be "ISIN" 🤓

    8. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Json'),
      

      I think the label should be "JSON" 🤓

    9. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +    $this->getDiscovery()->setDefinition('NotCompromisedPassword', [
      +      'label' => new TranslatableMarkup('Not compromised password'),
      +      'class' => '\Symfony\Component\Validator\Constraints\NotCompromisedPassword',
      +      'type' => 'string',
      +    ]);
      

      🤯 Holy crap!

      This uses https://haveibeenpwned.com/API/v2#SearchingPwnedPasswordsByRange, which is an external API. Let's omit this one 😅

    10. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('NotIdenticalTo'),
      

      I think the label should be "Not identical to" 🤓

    11. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Positive Or Zero'),
      

      I think the label should be "Positive or zero", for consistent capitalization with other similar labels 🤓

    12. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Ulid'),
      

      I think the label should be "ULID" 🤓

    13. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -95,11 +95,221 @@ public function registerDefinitions() {
      +      'label' => new TranslatableMarkup('Url'),
      

      I think the label should be "URL" 🤓

  • Status changed to Needs review over 1 year ago
  • last update over 1 year ago
    29,436 pass
  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Patch updated with adjustments as per #11.
    Note, also changed existing few definitions to use format Bic::class rather than string of current FQCN

  • Status changed to RTBC over 1 year ago
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    🤩

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Oh, arguably we should remove

    /**
     * Implements hook_validation_constraint_alter().
     */
    function ckeditor5_validation_constraint_alter(array &$definitions) {
      // Add the Symfony validation constraints that Drupal core does not add in
      // \Drupal\Core\Validation\ConstraintManager::registerDefinitions() for
      // unknown reasons. Do it defensively, to not break when this changes.
      if (!isset($definitions['Choice'])) {
        $definitions['Choice'] = [
          'label' => 'Choice',
          'class' => Choice::class,
          'type' => FALSE,
          'provider' => 'core',
          'id' => 'Choice',
        ];
      }
    }
    

    and instead do that here too 🤓

    I'm curious what release managers think!

  • last update over 1 year ago
    29,437 pass
  • 🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    I agree with #12, this is a great thing that we get new free validators in core. Ideally everything that is added to core should have at least one implementation, but I think we can forget about that requirement here because it is added code that is tested and used in symfony itself.
    RTBC+1

  • 🇬🇧United Kingdom catch

    I have one concern here which boils down to 📌 Discuss whether to decouple from Symfony Validator Active - Symfony Validator has had the most churn of any Symfony component we use, in a way that often impacts Symfony minor version updates.

    One the one hand, making their validators available means we'd get more benefit from using the component since we'd actually be using more of the code instead of (mostly) just their API, but this is also the drawback, that it exposes more of their code as API surface.

    If contrib starts using one and then we decided we don't want to expose it, how would we handle that given we can't deprecate their code?

    Also if we do eventually decide to move away from Symfony validator, would we then need to re-implement all of these in core compared to currently just the ones we're actually using?

  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Hmmm. I see that now. Currently about uh 5? 6? of the Symfony validators are exposed already - two with PHP annotation/inheritance/overrides and a few existing ones in the current Constraint Manager.

    So not a totally new problem, but could make things significantly worse. Since they are being exposed as plugins, we can deprecate them as per https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... . If that enough of protection? Combined with ability to copy validator code if future need arose, I believe without breaking BC, we could change the PHP path behind a plugin as long as the plugin name was unchanged yes?

  • 🇬🇧United Kingdom catch

    Add @trigger_error('...', E_USER_DEPRECATED) to the plugin constructor.

    We'd need to subclass the Symfony plugins in order to be able to do that ... maybe that is fine?

  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Yeah, that was my thought/hope. As long as we don't have to do that, quicker/less overhead LoC/files to do it in the manager but then if we need to change, subclass and deprecate/do changes.

  • last update over 1 year ago
    29,442 pass
  • 🇬🇧United Kingdom longwave UK

    Is it possible to write a test that ensures all Symfony validators are covered, so if more are added in a future Symfony release we will be automatically notified?

  • 🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    hmm... I don't see why not. Just PHP files. Unit test should be able to do that fine.

    Slight caveat/yes, and... we are intentionally skipping some validators that aren't really relevent or are uh bit of out of scope for core - like the one that checks password hashes against the HaveIBeenPwned database (aka going to an external service). We would need/could easily have a skipped validators list.

    If it helps get confidence for including it now, I'd be very open to writing such a test, but personally thinks that that test would add minimal value.

  • 🇬🇧United Kingdom catch

    Auto-registering with a skip list is tempting, although then we'd get any new validators added any time we update, so maybe the current include list is better anyway?

  • 🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    Auto-registering with a skip list is tempting, although then we'd get any new validators added any time we update, so maybe the current include list is better anyway?

    I agree with this, is there a checklist of things we check when updating to a new symfony version, we should probably add "check for new validators" to that list?

  • 🇬🇧United Kingdom longwave UK

    There isn't really a checklist that anyone is guaranteed to follow, this is why it's a nice idea to have a test that catches it.

  • last update over 1 year ago
    29,448 pass
  • last update over 1 year ago
    29,486 pass
  • last update over 1 year ago
    29,499 pass
  • last update over 1 year ago
    29,499 pass
  • 🇺🇸United States dww

    While we decide if/what we're going to do here, I fixed some capitalization nits in the labels. 😅 Leaving RTBC since core committers really need to decide what's next.

    Some other nitty review thoughts:

    1. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -87,22 +131,222 @@ public function create($name, $options) {
      +      'label' => new TranslatableMarkup('Business Identifier Code'),
      

      If this were "BIC" as an acronym, it'd be all caps. But does each word need to be capitalized like this?

      Also, this is the only acronym that we spell out. Should we leave this as just "BIC"?

      Or both: "Business identifier code (BIC)"?

    2. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -87,22 +131,222 @@ public function create($name, $options) {
      +      'label' => new TranslatableMarkup('CIDR'),
      

      If we're going to "unwind" BIC at all, should we do the same for these others (CIDR, ISBN, etc)?

    3. +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
      @@ -87,22 +131,222 @@ public function create($name, $options) {
      +      'label' => new TranslatableMarkup('Luhn'),
      

      I confess to not knowing about https://en.wikipedia.org/wiki/Luhn_algorithm until now. 🤓 Would "Luhn checksum" or "Luhn algorithm" be a more self-documenting label for this one?

    Thanks!
    -Derek

  • last update over 1 year ago
    29,505 pass
  • last update over 1 year ago
    29,551 pass
  • last update over 1 year ago
    29,553 pass
  • last update over 1 year ago
    29,559 pass
  • last update over 1 year ago
    29,563 pass
  • last update over 1 year ago
    29,571 pass
  • last update over 1 year ago
    29,801 pass
  • last update over 1 year ago
    29,801 pass
  • last update over 1 year ago
    29,802 pass
  • last update over 1 year ago
    29,802 pass
  • last update over 1 year ago
    29,807 pass
  • last update over 1 year ago
    29,811 pass
  • last update over 1 year ago
    29,815 pass
  • last update over 1 year ago
    29,815 pass
  • last update over 1 year ago
    29,822 pass
  • last update over 1 year ago
    29,842 pass
  • last update over 1 year ago
    Patch Failed to Apply
  • Status changed to Needs work over 1 year ago
  • 🇬🇧United Kingdom longwave UK

    #24 doesn't apply and needs rerolling.

  • First commit to issue fork.
  • 🇬🇧United Kingdom longwave UK

    #24.1 the ISO standard seemingly uses the title case "Business Identifier Code" as per https://en.wikipedia.org/wiki/ISO_9362 so we probably should as well.

    #24.2 I think CIDR could be qualified a bit more, something like "CIDR range"?

    #24.3 I like "Luhn checksum" here but open to suggestions.

    +++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
    @@ -87,22 +131,222 @@ public function create($name, $options) {
    -      'label' => new TranslatableMarkup('Email'),
    ...
    +      'label' => new TranslatableMarkup('E-mail'),
    

    This is incorrect, see 📌 Fix spelling of "email" Fixed

  • last update over 1 year ago
    29,961 pass
  • @rpayanm opened merge request.
  • last update over 1 year ago
    29,961 pass
  • Status changed to Needs review over 1 year ago
  • 🇺🇸United States rpayanm

    I rerolled the patch and applied the #27's suggestions. Please review.

  • Status changed to Needs work over 1 year ago
  • 🇨🇳China g089h515r806

    1, Most constraints could used at Drupal, but not all, Isin is the only one that not works, I test them manually in field validation 3.0.0 version. We will get fatal error when using Isin constraint.

    InvalidArgumentException: The passed value must be a typed data object. in Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validate() (line 97 of core\lib\Drupal\Core\TypedData\Validation\RecursiveContextualValidator.php).
    Drupal\Core\TypedData\Validation\RecursiveValidator->validate('30280378331005', Object) (Line: 79)
    Symfony\Component\Validator\Constraints\IsinValidator->isCorrectChecksum('US0378331005') (Line: 63)
    Symfony\Component\Validator\Constraints\IsinValidator->validate('US0378331005', Object) (Line: 116)

    I report the issue at :https://github.com/symfony/symfony/issues/51440.

    IsinValidator use following code in isCorrectChecksum function :

    return 0 === $this->context->getValidator()->validate($number, new Luhn())->count();
    

    drupal has own typed data validator base on symfony validator.Drupal use:

    core\lib\Drupal\Core\TypedData\Validation\RecursiveContextualValidator

    Instead of symfony's. At here $number is not a typed data. It throw a fatal exception.

    This is the only exception.

    Negative is inherit from LessThan, Email use Regex, If symfony validator author change the code, and use for example:

    $this->context->getValidator()->validate($value, new LessThan(['value']=0))->count()

    It still works for most normal symfony project, but it does not works at Drupal at all. We can not prevent them do this change.

    2, If a constraint has a message, and the message has a token, symfony use a different way for text translation, here is the example:

    
    /**
     * NotEqualTo constraint.
     *
     * @Constraint(
     *   id = "EqualTo",
     *   label = @Translation("EqualTo", context = "Validation"),
     * )
     */
    class EqualToConstraint extends EqualTo {
    
      public $message = 'This value should be equal to %compared_value.';
      /**
       * {@inheritdoc}
       */
      public function validatedBy(): string {
        return EqualToValidator::class;
      }
    
    }
    
    

    in symfony, it is :

    public $message = 'This value should be equal to {{ compared_value }}.';
    

    We have to extends the EqualTo, and override the message string. I am not sure Drupal core 11.x support {{ compared_value }} token, if it does not support, EqualTo, LessThan, GreaterThan have this issue, we have to extends them instead register them directly.

    3, Language, Locale, constraints need install symfony/intl and PHP's intl extension, we need change the composer.json to support them and change Docs to let user install PHP's intl extension.

  • First commit to issue fork.
  • last update about 1 year ago
    Custom Commands Failed
  • 🇪🇸Spain nireneko

    Like suggested by @g089h515r806, overrided some constraints to replace the message strings.

    This is the list of Symfony constraints with dependency on intl component:

    • Bic
    • Currency
    • Language
    • Locale
    • Timezone

    I didn't touch that constraints because we must add a new requirement in composer.json, the php extension "intl", and I think, that should be done in Drupal 11, no in 10.X ?

  • Status changed to Needs review about 1 year ago
  • 🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    Tests are green again, back to needs review.

  • Status changed to Needs work about 1 year ago
  • 🇺🇸United States smustgrave

    So when I rebuilt the dictionary.txt file I get a few more changes. Notably a number of deletions.

  • 🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    I hope I did the right thing, updated dictonary.txt.

  • Status changed to Needs review about 1 year ago
  • 🇧🇪Belgium borisson_ Mechelen, 🇧🇪
  • Status changed to RTBC about 1 year ago
  • 🇺🇸United States smustgrave

    Believe you did! Not sure what the proper workflow is.

    Like if another issue lands with dictionary changes. Does the manual merge work or does it need to be generated?

  • last update about 1 year ago
    30,485 pass
  • last update about 1 year ago
    30,486 pass
  • last update about 1 year ago
    30,488 pass
  • Open on Drupal.org →
    Environment: PHP 8.2 & MySQL 8
    last update about 1 year ago
    Not currently mergeable.
  • Open on Drupal.org →
    Environment: PHP 8.2 & MySQL 8
    last update about 1 year ago
    Not currently mergeable.
  • Status changed to Needs work about 1 year ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • First commit to issue fork.
  • Status changed to Needs review about 1 year ago
  • 🇵🇪Peru marvil07

    I merged in mainline at commit d797300b9ec841621a32d3efa2f84657ac46c012, so branch applies cleanly again.
    One trivial merge conflict solved on cspell dictionary, union merge.
    Back to NR.

  • Status changed to RTBC about 1 year ago
  • 🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    Back to rtbc now that it is rerolled.

  • last update about 1 year ago
    30,554 pass
  • Status changed to Needs work about 1 year ago
  • 🇬🇧United Kingdom longwave UK

    Some of the dictionary changes look a bit suspect: dflydev, grasmash, phpowermove, psysh, robo look like the dictionary was not generated from a clean environment.

    Instead of adding words to the dictionary it is OK to add cspell:ignore lines to the file(s) where the new words are used.

  • Status changed to Needs review about 1 year ago
  • 🇵🇪Peru marvil07

    Some of the dictionary changes look a bit suspect: dflydev, grasmash, phpowermove, psysh, robo look like the dictionary was not generated from a clean environment.

    @longwave, indeed, those changes seem to be unrelated.
    They may be related to recent changes on mainline, and there seems to be quite some changes there.

    $ git log --oneline --since="2 months ago" 11.x -- core/misc/cspell/
    7e75964e69 Issue #3395891 by quietone: Fix spelling for words in test modules
    a10e0f8d1f Issue #3328741 by quietone, smustgrave, xjm, catch, alexpott, longwave: Add a dictionary for Drupal-specific words
    3c1a449c37 Issue #3391788 by quietone, xjm: Fix spelling of function names in tests
    0940ba003b Issue #3399123 by Spokje, longwave, smustgrave: Upgrade JavaScript dependencies to latest minor/patch releases
    71f949cb0b Issue #3395913 by quietone: Correct spelling in Config tests
    0852886ca3 Issue #3368102 by finnsky, Gauravvvv, smustgrave, lauriii, e0ipso, andypost, mrinalini9: Create new SDC component for Umami (umami common footer block)
    c7e751a532 Issue #3379121 by quietone, longwave: Skip spellcheck for 20 nonsense words that are parts of hashes
    41b777c6be Issue #3395871 by quietone: Correct spelling of words in test modules and dramallama
    cb123cf554 Issue #3392425 by quietone, smustgrave: Fix variable name spelling in non-tests, part1
    36ab456505 Issue #3389283 by quietone: Fix spelling of words only misspelled in tests, part 1
    eafbd246a9 Issue #3394093 by quietone: Correct spelling of corefake
    6118e0dc37 Issue #3389282 by quietone, smustgrave, alexpott: Fix spelling for words that are only misspelled  in comments
    ae55070f94 Issue #3391268 by quietone: Fix spelling of words only misspelled in tests, part 4
    b78ac3f08a Issue #3390955 by quietone, smustgrave: Fix spelling of words only misspelled in tests, part 3
    a8f52aa253 Issue #3352459 by catch, Bhanu951, Wim Leers, smustgrave, Fabianx, heddn, alexpott: Add OpenTelemetry Application Performance Monitoring to core performance tests
    987548947c Issue #3389286 by quietone: Fix spelling of words only misspelled in tests, part 2
    dbb54acf6e Issue #3312072 by fjgarlin, penyaskito, markconroy, lauriii, smustgrave, ckrina, Spokje: Display category-related recipes when seeing a recipe full page
    8224c90c08 Issue #3387339 by bbrala, longwave: Integrate codequality reports into Gitlab
    

    I went on and revert the dictionary change, and merged mainline again, to be sure to be on the latest status.
    Then, I ran the spell check, and it still passed.

    $ cd core
    $ yarn spellcheck:core
    ...
    CSpell: Files checked: 15156, Issues found: 0 in 0 files
    

    There are actually some valid additions, that are related to the newly added validators.
    I reverted all additions to the dictionary, and the spell check did not pass, as expected.

    $ cd core
    $ yarn spellcheck:core
    ...
    CSpell: Files checked: 15156, Issues found: 9 in 1 files
    

    Looking into the details, it is all coming from the added core/lib/Drupal/Core/Validation/ConstraintManager.php.

    $ yarn cspell -c .cspell.json --root .. "core/lib/Drupal/Core/Validation"
    yarn run v1.22.19
    $ core/node_modules/.bin/cspell -c .cspell.json --root .. 'core/lib/Drupal/Core/Validation'
     1/40 ./lib/Drupal/Core/Validation/Annotation/Constraint.php 299.41ms
     2/40 ./lib/Drupal/Core/Validation/BasicRecursiveValidatorFactory.php 19.78ms
     3/40 ./lib/Drupal/Core/Validation/ConstraintFactory.php 16.03ms
     4/40 ./lib/Drupal/Core/Validation/ConstraintManager.php 83.91ms X
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:26:45 - Unknown word (Issn)
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:31:45 - Unknown word (Luhn)
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:169:42 - Unknown word (IBAN)
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:193:43 - Unknown word (Issn)
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:194:42 - Unknown word (ISSN)
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:195:18 - Unknown word (Issn)
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:218:43 - Unknown word (Luhn)
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:219:42 - Unknown word (Luhn)
    ./core/lib/Drupal/Core/Validation/ConstraintManager.php:220:18 - Unknown word (Luhn)
     5/40 ./lib/Drupal/Core/Validation/ConstraintValidatorFactory.php 20.18ms
     6/40 ./lib/Drupal/Core/Validation/DrupalTranslator.php 35.70ms
     7/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraint.php 9.90ms
     8/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/AllowedValuesConstraintValidator.php 48.61ms
     9/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/CidrConstraint.php 12.12ms
    10/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ComplexDataConstraint.php 32.69ms
    11/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ComplexDataConstraintValidator.php 9.25ms
    12/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/CountConstraint.php 7.68ms
    13/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/DivisibleByConstraint.php 7.41ms
    14/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/EmailConstraint.php 6.89ms
    15/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/EmailValidator.php 7.49ms
    16/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/GreaterThanConstraint.php 7.71ms
    17/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/GreaterThanOrEqualConstraint.php 10.27ms
    18/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/IdenticalToConstraint.php 6.43ms
    19/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ImageConstraint.php 16.00ms
    20/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/IsNullConstraint.php 6.39ms
    21/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/IsNullConstraintValidator.php 8.05ms
    22/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/LengthConstraint.php 11.36ms
    23/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/LessThanConstraint.php 7.69ms
    24/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/LessThanOrEqualConstraint.php 15.47ms
    25/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotEqualToConstraint.php 8.93ms
    26/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotIdenticalToConstraint.php 6.50ms
    27/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotNullConstraint.php 5.44ms
    28/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotNullConstraintValidator.php 7.44ms
    29/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraint.php 5.48ms
    30/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/PrimitiveTypeConstraintValidator.php 16.61ms
    31/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/RangeConstraint.php 8.29ms
    32/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/RangeConstraintValidator.php 14.08ms
    33/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/RegexConstraint.php 8.47ms
    34/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/TypeConstraint.php 6.32ms
    35/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldConstraint.php 6.95ms
    36/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UniqueFieldValueValidator.php 28.14ms
    37/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/UuidConstraint.php 5.73ms
    38/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraint.php 17.82ms
    39/40 ./lib/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php 14.34ms
    40/40 ./lib/Drupal/Core/Validation/TranslatorInterface.php 7.78ms
    CSpell: Files checked: 40, Issues found: 9 in 1 files
    

    I agree that ignoring the few instances on the one expected file using it makes sense.
    So, I have pushed the following changes to the topic branch.

    - a4cc65801f Revert "Update cspell dictionary"
    - e44f62a124 Get mainline changes
    - 830cae3bb9 Revert actual additions to the dictionary
    - fb8d79efa0 Ignore spelling for three symfony validator specific words

    Locally, after the changes it looks like the following.

    $ cd core
    $ yarn spellcheck:core
    ...
    CSpell: Files checked: 15156, Issues found: 0 in 0 files
    

    Let us see if the test run is OK too.

  • 🇵🇪Peru marvil07

    It took an extra bit to make the cspell change OK from phpcs perspective :-)
    Now I see CI approving ;-)

  • 🇨🇳China g089h515r806

    Symfony Isin constraint could not be used in Drupal directly:

    use Symfony\Component\Validator\Constraints\Isin;

    I remind this again.

  • Status changed to Needs work about 1 year ago
  • 🇬🇧United Kingdom catch

    I don't feel like #15 has been addressed here, it's adding a huge amount of API surface directly from Symfony, with no real indication that these are going to be used anywhere.

    What happens when Symfony removes one of these, changes the API etc., are we going to have to start to add bc layers ourselves? We've already got enough API churn from this component as it is.

  • 🇵🇪Peru marvil07

    Direction

    I don't feel like #15 has been addressed here, it's adding a huge amount of API surface directly from Symfony, with no real indication that these are going to be used anywhere.

    @catch, yes, the original intention here is to make general symfony validators available as constraint plugins, so they can be used; but they have not been used directly anywhere, at least yet.

    What happens when Symfony removes one of these, changes the API etc., are we going to have to start to add bc layers ourselves? We've already got enough API churn from this component as it is.

    That is fair to say, I appreciate this surfacing again and reminding the possible consequences.
    I have not followed closely how symfony validator has impacted core, since it was added.

    From what it is mentioned, it sounds like the possible impact can be more expensive than not exposing them by core.
    Maybe the right solution for core is just to not expose them, or at least not all of them, and leave the rest for contrib modules like field_validation .

    Already there

    There is already a subset of symfony constraint, or at least the related validators, that is exposed by core.
    This issue is about exposing all the rest.

    A static set added atDrupal\Core\Validation\ConstraintManager::registerDefinitions().

    And the set at core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/, excluding custom constraints directly extending Symfony\Component\Validator\Constraint.

    • AllowedValuesConstraint, extends Choice; along with AllowedValuesConstraintValidator, that extends ChoiceValidator.
    • CountConstraint, extends Count.
    • EmailConstraint extends Email, and uses EmailValidator, as mentioned on the previous set.
    • IsNullConstraint, extends IsNull, using a custom IsNullConstraintValidator on top of symfony one.
    • LengthConstraint, extends Length.
    • NotNullConstraint, extends NotNull, using a custom NotNullConstraintValidator.
    • RangeConstraint, extends Range, using RangeConstraintValidator.
    • RegexConstraint, extends Regex.
    • UuidConstraint, extends Uuid.

    In other words, currently it seems core exposes 13 symfony constraints to be used, of the 71 that symfony currently provides.

    Next steps

    Likely, a decision about what to do needs to be taken.

    Taking into account the suggested upgrade problems from the near past as stated on #15, it sounds like the best approach may be to not take any action here, and close this issue.

    That may avoid some of the possible burden of needing to add compatibility layers if upstream decides to change not trivially, or at least reduce it, since core already exposes 13 of the symfony validators, directly or indirectly.

  • 🇬🇧United Kingdom catch

    There's some discussion (a mixture of old issues and current ones) in 📌 Discuss whether to decouple from Symfony Validator Active . The validators themselves haven't necessary been the barrier to updating Symfony versions, a lot of it's been 'internal' Symfony classes that we nonetheless have to subclass, but that also could be because we're not exposing that many yet.

  • 🇬🇧United Kingdom 2dareis2do

    Is this related to issue where if config value is set to null you get a UnexpectedTypeException?

    e.g. https://www.drupal.org/project/config_inspector/issues/3416934#comment-1... 🐛 Config inspector stopped working after Drupal 10.2 update Active and https://www.drupal.org/project/search_api_solr/issues/3415861 🐛 ValidKeysConstraintValidator thrown by config inspector Active

    Certain values are expected are set as null, but ValidKeysConstraintValidator is expecting it to be an array.

Production build 0.71.5 2024