Problem/Motivation

Method "Symfony\Component\Validator\Context\ExecutionContextInterface::getViolations()" will return "ConstraintViolationListInterface" as of its next major version. Doing the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" will be required when upgrading.

Method "Symfony\Component\Validator\Context\ExecutionContextInterface::getValidator()" will return "ValidatorInterface" as of its next major version. Doing the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" will be required when upgrading.

Method "Symfony\Component\Validator\Context\ExecutionContextInterface::getRoot()" will return "mixed" as of its next major version. Doing the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" will be required when upgrading.

Method "Symfony\Component\Validator\Context\ExecutionContextInterface::getValue()" will return "mixed" as of its next major version. Doing the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" will be required when upgrading.

Steps to reproduce

Proposed resolution

Add the type hints.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#15 3231688-15.patch2.36 KBlongwave
#2 3231688-2.patch1.41 KBdaffie

Issue fork drupal-3231688

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Active » Needs review
FileSize
3231688-2.patch1.41 KB

The fix.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Related issues: +#3231683: [Symfony 6] A number of methods of the class Drupal\Core\TypedData\Validation\ExecutionContext are considered internal and Drupal should not override them.

As per my comments in #3231683: [Symfony 6] A number of methods of the class Drupal\Core\TypedData\Validation\ExecutionContext are considered internal and Drupal should not override them. I wonder if we can refactor this away entirely.

daffie’s picture

daffie CreditAttribution: daffie commented
Priority: Normal » Critical
Issue tags: +Drupal 10

I could not find any instances were contrib is extending the class Drupal\Core\TypedData\Validation\ExecutionContext. The adding of the return type hints do not change anything as the methods already only returning what the added type hints are demanding. See: http://grep.xnddx.ru/search?text=ExecutionContext&filename=.

Part of the Symfony 6 in D10 initiative.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

The patch adds the typehints as per the deprecation.

  • larowlan committed 1b90015 on 9.3.x 
    Issue #3231688 by daffie: [Symfony 6] Add type hints to Drupal\Core\...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Fixed

Committed 1b90015 and pushed to 9.3.x. Thanks!

Luke.Leber’s picture

Luke.Leber
Primary language English
Location Pennsylvania
CreditAttribution: Luke.Leber commented

I just saw a strange test run with a contrib project that might somewhat be related to this. If this is a red herring, sorry for the noise!

https://www.drupal.org/pift-ci-job/2182122

1) Drupal\Tests\responsive_image_preload\Functional\ResponsiveImagePreloadWebpTest::testResponsiveImageWebpPreload
TypeError: Return value of Drupal\Core\TypedData\Validation\ExecutionContext::getValue() must be an instance of Drupal\Core\TypedData\Validation\mixed, null returned

/var/www/html/core/lib/Drupal/Core/TypedData/Validation/ExecutionContext.php:203
/var/www/html/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:122
/var/www/html/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:100
/var/www/html/core/lib/Drupal/Core/TypedData/Validation/RecursiveValidator.php:92
/var/www/html/core/lib/Drupal/Core/TypedData/TypedData.php:132
/var/www/html/core/modules/user/user.module:211
/var/www/html/core/lib/Drupal/Core/Installer/Form/SiteConfigureForm.php:285
/var/www/html/core/lib/Drupal/Core/Form/FormValidator.php:82
/var/www/html/core/lib/Drupal/Core/Form/FormValidator.php:275
/var/www/html/core/lib/Drupal/Core/Form/FormValidator.php:118
/var/www/html/core/lib/Drupal/Core/Form/FormBuilder.php:588
/var/www/html/core/lib/Drupal/Core/Form/FormBuilder.php:498
/var/www/html/core/includes/install.core.inc:966
/var/www/html/core/includes/install.core.inc:617
/var/www/html/core/includes/install.core.inc:570
/var/www/html/core/includes/install.core.inc:118
/var/www/html/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:299
/var/www/html/core/tests/Drupal/Tests/BrowserTestBase.php:554
/var/www/html/core/tests/Drupal/Tests/BrowserTestBase.php:378
/var/www/html/modules/contrib/responsive_image_preload/tests/src/Functional/ResponsiveImagePreloadBrowserTestBase.php:70
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:722
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Version: 9.3.x-dev » 10.0.x-dev
Status: Fixed » Needs work

A yeah, mixed is PHP8.0 only.

Rolling this one back.

I think that makes this D10 only.

  • larowlan committed 70d6441 on 9.3.x 
    Revert "Issue #3231688 by daffie: [Symfony 6] Add type hints to Drupal\...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Thanks @Luke.Leber - rolled that back

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

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje as a volunteer commented
Status: Needs work » Closed (outdated)

Class \Drupal\Core\TypedData\Validation\ExecutionContext is @deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. See [#3238432]

Since that's the only class changes were made for this issue, and this issue can only be committed in Drupal 10 (in which this class will be deleted), I'm going to put this issue on "Closed (outdated)".
Please correct this if I'm wrong.

daffie’s picture

daffie CreditAttribution: daffie commented

+1 for marking this as outdated. The class is deprecated and will be removed in 10.0.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Component: base system » typed data system
Status: Closed (outdated) » Needs review
Related issues: +#3255245: [Symfony 6] Revert 3231603 to use our own TranslatorInterface
FileSize
3231688-15.patch2.36 KB

Reactivating this following the revert in #3255245: [Symfony 6] Revert 3231603 to use our own TranslatorInterface

The latest SF 5.4 run in #3197482: Update Drupal 10 to depend on Symfony 5.4 (as a stepping stone to Symfony 6, for deprecation checking support) shows a few more typehints are needed, fixed in the attached patch.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Title: [Symfony 6] Add type hints to Drupal\Core\TypedData\Validation\ExecutionContext::getViolations(), ::getValidator(), ::getRoot() and ::getValue() » [Symfony 6] Add type hints to Drupal\Core\TypedData\Validation\ExecutionContext
Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions commented
Status: Needs review » Reviewed & tested by the community
Related issues: +#3197482: Update Drupal 10 to depend on Symfony 5.4 (as a stepping stone to Symfony 6, for deprecation checking support)

Seems like all required typehints have been added (match Symfony 6 as well). Looks good to me.

Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions commented

just for the reference, fails from the #3197482-53: Update Drupal 10 to depend on Symfony 5.4 (as a stepping stone to Symfony 6, for deprecation checking support)

  1x: Method "Symfony\Component\Validator\Context\ExecutionContextInterface::getViolations()" might add "ConstraintViolationListInterface" as a native return type declaration in the future. Do the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" now to avoid errors or add an explicit @return annotation to suppress this message.
    1x in ContextHandlerTest::testCheckRequirements from Drupal\Tests\Core\Plugin

  1x: Method "Symfony\Component\Validator\Context\ExecutionContextInterface::getValidator()" might add "ValidatorInterface" as a native return type declaration in the future. Do the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" now to avoid errors or add an explicit @return annotation to suppress this message.
    1x in ContextHandlerTest::testCheckRequirements from Drupal\Tests\Core\Plugin

  1x: Method "Symfony\Component\Validator\Context\ExecutionContextInterface::getRoot()" might add "mixed" as a native return type declaration in the future. Do the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" now to avoid errors or add an explicit @return annotation to suppress this message.
    1x in ContextHandlerTest::testCheckRequirements from Drupal\Tests\Core\Plugin

  1x: Method "Symfony\Component\Validator\Context\ExecutionContextInterface::getValue()" might add "mixed" as a native return type declaration in the future. Do the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" now to avoid errors or add an explicit @return annotation to suppress this message.
    1x in ContextHandlerTest::testCheckRequirements from Drupal\Tests\Core\Plugin

  1x: Method "Symfony\Component\Validator\Context\ExecutionContextInterface::isConstraintValidated()" might add "bool" as a native return type declaration in the future. Do the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" now to avoid errors or add an explicit @return annotation to suppress this message.
    1x in ContextHandlerTest::testCheckRequirements from Drupal\Tests\Core\Plugin

  1x: Method "Symfony\Component\Validator\Context\ExecutionContextInterface::isGroupValidated()" might add "bool" as a native return type declaration in the future. Do the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" now to avoid errors or add an explicit @return annotation to suppress this message.
    1x in ContextHandlerTest::testCheckRequirements from Drupal\Tests\Core\Plugin

  1x: Method "Symfony\Component\Validator\Context\ExecutionContextInterface::isObjectInitialized()" might add "bool" as a native return type declaration in the future. Do the same in implementation "Drupal\Core\TypedData\Validation\ExecutionContext" now to avoid errors or add an explicit @return annotation to suppress this message.
    1x in ContextHandlerTest::testCheckRequirements from Drupal\Tests\Core\Plugin

  • catch committed b12d11a on 10.0.x 
    Issue #3231688 by daffie, longwave, Spokje, larowlan, Taran2L: [Symfony...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed b12d11a and pushed to 10.0.x. Thanks!

Status: Fixed » Closed (fixed)

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