[symfony 5] Class implements "Symfony\Component\DependencyInjection\ResettableContainerInterface" that is deprecated since Symfony 4.2, use "Symfony\Contracts\Service\ResetInterface" instead

This is the same problem we're trying to resolve in Drupal land. Essentially we don't care about Symfony 4 deprecations in Drupal 8. We do though in Drupal 9.0.0 - that's why when we get this sorted out we file an upstream PR because being able to ignore by version is useful functionality.

While the message is dynamic, it's only used in Drupal\Component\DependencyInjection\Container, and all 17 deprecation failures I see in the symfony 4 thread are all identical. I think we could just add the one message to ignore it for now.

Yeah let's do #6 and add a new section to \Drupal\Tests\Listeners\DeprecationListenerTrait::getSkippedDeprecations()

Something like

      // Deprecations in Symfony 4 for Symfony 5.
      // @todo Remove in Drupal 9.
      'blah'

:( we need to make \Drupal\Tests\Listeners\DeprecationListenerTrait::getSkippedDeprecations() work for unit tests - it's never had to do that before.

I think we might have to do something awful to support it - like use reflection to change the value of \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::$gatheredDeprecations()

I was just driving myself crazy trying to figure out why these didn't go away when I tried this in the main symfony 4 thread.. :-/

A good chunk of the remaining functional test failures seem to be due to escaping Constraint Violation messages. The test is looking for A feed named <em class="placeholder">iD5qjReymv</em> already exists. Enter a unique title. and the response contains A feed named <em class="placeholder">iD5qjReymv</em> already exists. Enter a unique title.I'm only starting to dig into why that might be happening, but I thought I would mention it in case it triggered any ideas with anyone who has been working with this more than I have.

Whoops, wrong issue, ignore me.

Yeah that's not pleasant. We need to wrap the Symfony deprecation error handler to do this. But because that has something that detects if it has been replaced we need to put it back again at the end.

(crosspost with Alex Pott, haven't looked at the latest patch yet).

We can extend Symfony's ResettableContainerInterface and remove the deprecation annotation. This doesn't require any new testing infrastructure.

Absolutely zero core code type hints on this interface, so when we need to actually require Symfony 4.2 or Symfony 5, it should be fine to switch the use statement again to Symfony\Contracts directly (which appears to be brand new from July 2018).

Thinking about Symfony 5 forwards-compatibility. If this patch passes on Symfony 3 (afaict it should), and we're confident no-one type hints on ResettableContainerInterface (no reason why they would), then it might also be enough.

With the composer changes this time.

Here's a patch that makes the skipping work... we might need to do this because they might be something we can't do by including another dependency.

The problem with #18 is that Symfony\Contracts requires PHP 7.1.3. So it would be fine if we required that, but we do not yet.

Another option - just don't implement either interface!

(another crosspost with alexpott)

I think #24 is a good reason #22 is the way to go - skip SF4->SF5 deprecations for now and concentrate on SF4 and getting feedback for that.

I think we can proceed with #23. I find it extremely unlikely that anyone is using the interface. Searching popular contrib - no one is using it - http://grep.xnddx.ru/search?text=ResetInterface&filename= so I think it is fine to go with #23. However @Gábor Hojtsy pointed out that we should add an @todo to re add the interface in Drupal 9 / for when we adopt Sf5

Yeah done indeed - onwards.

and we're confident no-one type hints on ResettableContainerInterface (no reason why they would),

Searching popular contrib - no one is using it - http://grep.xnddx.ru/search?text=ResetInterface&filename= s

Shouldn't we be searching for ResettableContainerInterface and not ResetInterface?

Also, that site is down now, so can't recheck.

I think we need to confirm no one is using ResettableContainerInterface on the basis of the patch and we also need an issue summary update, as it still talks about adding to the allowed deprecations list.

In addition, we need a change record - this is an API break for anyone type-hinting on that interface in client code.

I think we still need to grep contrib for ResettableContainerInterface as earlier comments indicate we were looking for the wrong thing

Awesome - we've done due diligence here. Removing it and adding it back again when we have symfony/contracts seems the best solution.

Another reason why this is a fine approach is that contain reset in Symfony land is now a test-only thing - see https://github.com/symfony/symfony/pull/16609

It's tempting to not the follow-up but deprecate \Drupal\Component\DependencyInjection\Container::reset() and remove it in Drupal 9.

I opened #3037826: Theme contains a complete version of core and its dependencies to address portfolio theme including 8.6.9 core and all of vendor

It's tempting to not the follow-up but deprecate \Drupal\Component\DependencyInjection\Container::reset() and remove it in Drupal 9.

+1

Committed 1036f4c and pushed to 8.7.x. Thanks!

Published change record

Added release notes snippet