[META] Symfony 6.2 compatibility

A "What-Would-Break-If-Symfony-6.2-Would-Be-Released-Today" patch.

Picking a random test failure and clicking on it, I see these two, which look like they'd be the reason there's thousands of test failures:

 5x: Since symfony/http-kernel 6.2: Starting from 7.0, "Symfony\Component\HttpKernel\HttpKernel::handle()" will catch \Throwable exceptions and convert them to HttpFoundation responses. Pass $catchThrowable=true to adapt to this behavior now.
    5x in ActionListTest::testEmptyActionList from Drupal\Tests\action\Functional

  1x: Since symfony/http-foundation 6.2: The "Symfony\Component\HttpFoundation\Request::getContentType()" method is deprecated, use "getContentTypeFormat()" instead.
    1x in ActionListTest::testEmptyActionList from Drupal\Tests\action\Functional

Since we can't fix them until we require Symfony 6.2, we'll need to suppress the deprecation messages until then.

Changes in the argument resolver are yielding this for me for any route that uses the entity routing defaults
ArgumentCountError: Too few arguments to function Drupal\Core\Entity\Controller\EntityController::addTitle()
The entity_type_id argument isn't being resolved from the route.

re-queued #3 as #3304991: [Symfony 6] HttpKernel should opt-in to catching `\Throwable` commited

updated patch from #3

sorry for x-post, wondering why your patch is x2 smaller

Might have something to do with including .orig and .rej in yours?

diff --git a/composer/Metapackage/PinnedDevDependencies/composer.json b/composer/Metapackage/PinnedDevDependencies/composer.json.orig
similarity index 100%
copy from composer/Metapackage/PinnedDevDependencies/composer.json
copy to composer/Metapackage/PinnedDevDependencies/composer.json.orig
diff --git a/composer/Metapackage/PinnedDevDependencies/composer.json.rej b/composer/Metapackage/PinnedDevDependencies/composer.json.rej

674 failed vs 1253 in #11 with only one exclude

Less bad !== good, but we're getting there :)

Re-rolled. Patch is about the same size so hopefully got it right.

Started looking at #3284422-5: [META] Symfony 6.2 compatibility.

https://github.com/symfony/symfony/pull/47363 is a big difference for ArgumentValueResolvers, but after updating our two for that in case it was a bug in the bc layer, still getting the same error.

The other big commit is https://github.com/symfony/symfony/commit/0f2293c983744b744085e2dd67d89d... - haven't really started looking at that yet.

OK that one's the problem. Specifically the removal of this hunk:

https://github.com/symfony/symfony/pull/46001/files#diff-d3747dd395ddb12...

https://github.com/symfony/symfony/pull/47461

My patch in #9 is bigger because more changes in lock-file - much more changes because of stability flags /cc @Spokje

+++ b/composer.json
@@ -27,16 +27,16 @@
-        "symfony/css-selector": "^6.1",
...
+        "symfony/css-selector": "^6.2",

@catch comparing to usage of ^6.2@dev, like it was in previous patches, does not pick latest codebase

+++ b/composer.json
@@ -27,16 +27,16 @@
-        "symfony/css-selector": "^6.1",
...
+        "symfony/css-selector": "^6.2@dev",

earlier patches

Once https://github.com/symfony/symfony/pull/47468 is merged we should do a new test run, with the change from #19 too. The upstream regression was breaking almost every functional test in core so probably responsible for most of the remaining few hundred failures.

regression merged https://github.com/symfony/symfony/commit/0e288486dfa1e46ee2f376ce53592f...

reroll

+------------------------------+--------+-----------+
| Production Changes           | From   | To        |
+------------------------------+--------+-----------+
| symfony/dependency-injection | v6.1.3 | 6.2.x-dev |
| symfony/error-handler        | v6.1.3 | 6.2.x-dev |
| symfony/event-dispatcher     | v6.1.0 | 6.2.x-dev |
| symfony/http-foundation      | v6.1.4 | 6.2.x-dev |
| symfony/http-kernel          | v6.1.4 | 6.2.x-dev |
| symfony/mime                 | v6.1.4 | 6.2.x-dev |
| symfony/process              | v6.1.3 | 6.2.x-dev |
| symfony/routing              | v6.1.3 | 6.2.x-dev |
| symfony/serializer           | v6.1.4 | 6.2.x-dev |
| symfony/validator            | v6.1.4 | 6.2.x-dev |
| symfony/var-dumper           | v6.1.3 | 6.2.x-dev |
| symfony/yaml                 | v6.1.4 | 6.2.x-dev |
| symfony/var-exporter         | NEW    | 6.2.x-dev |
+------------------------------+--------+-----------+

+------------------------+--------+-----------+
| Dev Changes            | From   | To        |
+------------------------+--------+-----------+
| symfony/browser-kit    | v6.1.3 | 6.2.x-dev |
| symfony/css-selector   | v6.1.3 | 6.2.x-dev |
| symfony/dom-crawler    | v6.1.4 | 6.2.x-dev |
| symfony/filesystem     | v6.1.4 | 6.2.x-dev |
| symfony/finder         | v6.1.3 | 6.2.x-dev |
| symfony/lock           | v6.1.3 | 6.2.x-dev |
| symfony/phpunit-bridge | v6.1.3 | 6.2.x-dev |
+------------------------+--------+-----------+

fix CS

Missed to bump SF console

The last error message is

3x: Since symfony/validator 6.2: The "loose" mode is deprecated. The default mode will be changed to "html5" in 7.0.

it's from https://symfony.com/doc/6.2/reference/configuration/framework.html#email...

related to #3054535: Discuss whether to decouple from Symfony Validator

The warning comes from \Symfony\Component\Validator\Constraints\EmailValidator::__construct()

Core using class_resolver to instantiate constraint so no way to pass new option to constructor, so add to suppressions

We can subclass for that one.

Although that brings up the question - if our constraint is setting mode = strict, is there any point if the validator is instantiated with mode = loose? Wondering if our existing logic (from 2014 (!)) to set mode = strict ever actually worked and it's been loose all this time.

Less strict mode is HTML5 (just a test run)

Moreover the Strict mode requires email validator!

            if (Email::VALIDATION_MODE_STRICT === $this->defaultMode && !class_exists(EguliasEmailValidator::class)) {
                throw new LogicException(sprintf('The "egulias/email-validator" component is required to make the "%s" constraint default to strict mode.', EguliasEmailValidator::class));
            }

PS: fixed class name

3 tests fail

2 - expected
and Drupal\FunctionalTests\Bootstrap\UncaughtExceptionTest not clear

UncaughtExceptionTest
That is hopefully straightforward

The Kernel behaviour change with catching exceptions means the uncaught exception we were looking for is now a caught exception, so we just need to remove that test method altogether.

However it also looks like that upstream change might get reverted in https://github.com/symfony/symfony/pull/47467 - if so we'd need to revert the container changes and test method removal here, and deal with it when we work on Symfony 7 compatibility.

@catch Looks like you're fixing the wrong test function?

Testrun:

There was 1 error:

1) Drupal\FunctionalTests\Bootstrap\UncaughtExceptionTest::testMissingDependencyCustomErrorHandler
Behat\Mink\Exception\ExpectationException: Current response status code is 500, but 418 expected.

Interdiff:
- public function testMissingDependency() {

Reverting unwanted status change.

Helps when you remove the right test method.

Re-rolled.

Forgot to add a file..

The three failures are expected due to changing stability to dev, so that's everything we need for now.

Split out #3307736: EmailValidator defaults to 'loose' mode which is deprecated in Symfony 6.2 to its own issue.

Everything else probably needs to stay in here?

+++ b/core/lib/Drupal/Core/Controller/ArgumentResolver/Psr7RequestValueResolver.php
@@ -6,12 +6,13 @@
-final class Psr7RequestValueResolver implements ArgumentValueResolverInterface {
+final class Psr7RequestValueResolver implements ArgumentValueResolverInterface, ValueResolverInterface {

@@ -40,8 +41,8 @@ public function supports(Request $request, ArgumentMetadata $argument): bool {
-  public function resolve(Request $request, ArgumentMetadata $argument): iterable {
-    yield $this->httpMessageFactory->createRequest($request);
+  public function resolve(Request $request, ArgumentMetadata $argument): array {
+    return $argument->getType() == ServerRequestInterface::class ? [$this->httpMessageFactory->createRequest($request)] : [];

+++ b/core/lib/Drupal/Core/Controller/ArgumentResolver/RouteMatchValueResolver.php
@@ -6,12 +6,13 @@
-final class RouteMatchValueResolver implements ArgumentValueResolverInterface {
+final class RouteMatchValueResolver implements ArgumentValueResolverInterface, ValueResolverInterface {

@@ -23,8 +24,8 @@ public function supports(Request $request, ArgumentMetadata $argument): bool {
-  public function resolve(Request $request, ArgumentMetadata $argument): iterable {
-    yield RouteMatch::createFromRequest($request);
+  public function resolve(Request $request, ArgumentMetadata $argument): array {
+    return $argument->getType() == RouteMatchInterface::class || is_subclass_of($argument->getType(), RouteMatchInterface::class) ? [RouteMatch::createFromRequest($request)] : [];

I bet it needs new issue

@andypost so we can't do a new issue as a prerequisite of updating to Symfony 6.2 because the new interface isn't available yet. I guess we could suppress the deprecation, update to 6.2, then resolve the deprecation in a new issue though.

Filed new child #3312075: Make \Drupal\Tests\Component\EventDispatcher\ContainerAwareEventDispatcherTest more inline with Symfony 6.2

Symfony 6.2.0-BETA1 is out, https://github.com/symfony/phpunit-bridge/releases/tag/v6.2.0-BETA1

Let's start of with a patch that _only_ updates all our symfony/* dependencies

Let's start of with a patch that _only_ updates all our symfony/* dependencies

Let's retry that after two more mugs of coffee...

Hmm, let's try to suppress the deprecation message which we're doing initially in #3306886: Suppress SF6.2 deprecation message for Request::getContentType()

I hope this is okay, but I'd like to hide patches in favor of a merge request.

Three Symfony packages were not updated from 6.1 to 6.2 beta, pushed changes to composer.* for those.

The remaining test fail is related to this change in upstream HttpKernel: https://github.com/symfony/http-kernel/commit/2edca008dde746f6450c6b0499...

HttpKernel now catches all throwables instead of just exceptions, so this error is now caught there. Locally if I change the $handleAllThrowables argument to HttpKernel from true to false then UncaughtExceptionTest passes again; not sure what else that will break, so sending it for a test run to see.

Uploading a patch of the current MR, as Gitlab is backlogged at the moment and hasn't noticed my commits yet.

Oops, the diff link is out of date as well. Trying again.

Great job mostly done!

+++ b/core/lib/Drupal/Component/DependencyInjection/composer.json
@@ -29,5 +29,6 @@
+    "minimum-stability": "beta"

+++ b/core/lib/Drupal/Component/EventDispatcher/composer.json
@@ -21,5 +21,6 @@
+    "minimum-stability": "beta"

+++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
@@ -32,7 +32,7 @@ class ComposerProjectTemplatesTest extends ComposerBuildTestBase {
-  protected const MINIMUM_STABILITY = 'stable';
+  protected const MINIMUM_STABILITY = 'beta';

I suppose this change is not permanent

+++ b/core/core.services.yml
@@ -743,7 +743,7 @@ services:
-    arguments: ['@event_dispatcher', '@controller_resolver', '@request_stack', '@http_kernel.controller.argument_resolver', true]
+    arguments: ['@event_dispatcher', '@controller_resolver', '@request_stack', '@http_kernel.controller.argument_resolver', false]

this change is debatable because it set as opt-in intentionally in #3304991: [Symfony 6] HttpKernel should opt-in to catching `\Throwable`

Re #60.1 the change from stable to beta will be reverted when the stable release of Symfony 6.2 is out.

Re the change in core.services.yml, we have two options here:

1. Keep the argument as true, remove UncaughtExceptionTest::testMissingDependencyCustomErrorHandler() and disallow custom error handlers to be set in index.php. This was initially added in #2521852: Make it possible to use your own exception handler

2. Change the argument to false and drop support for #3304991: [Symfony 6] HttpKernel should opt-in to catching `\Throwable`

Discussed with @catch, as we need HTTP kernel error handling to provide better errors in REST/JSON/etc and there is probably no real use case for custom error handlers in index.php (but users can still opt in if they wish), I opened #3319170: Change the http_kernel.basic service to use Symfony 6.2's default of catching all throwables for #61.1

When we did #3304991: [Symfony 6] HttpKernel should opt-in to catching `\Throwable` Symfony was going to remove the old behaviour entirely in Symfony 7 so we didn't have a lot of choice, but they've now rolled back the deprecation so that it's more optional. However I do still think that's a good change and the follow-up to finish it off properly seems the best way to deal with it.

+++ b/core/.deprecation-ignore.txt
@@ -21,4 +21,3 @@
-

This is out scope? One blank line removed from the end of the file.

Obviously a nit, the rest seems sensible to me.

Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!

I've opened #3319426: Update to a stable Symfony 6.2 release as a placeholder for the next release update.