Fixed Symfony 7.4 incompatibility

[Steveb-p]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes? 😅
Fixed tickets
License	Apache2

Description

This PR changes how ValidationExtractor interacts with metadata returned from MetadataFactory, fixing an issue that shows up after recent Symfony 7.4 release.

Before, internal properties were used (which were only public to reduce serialization size). With these changes, Symfony 7.4 metadata - which now has private properties instead of public - should be handled properly.

All methods used in this new version are also present since at least Symfony 5.4, so they should be safe.

See:

• https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/Validator/Mapping/GenericMetadata.php#L192
• https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/Validator/Mapping/ClassMetadata.php#L386

Todos

• Tests
• Documentation
• Changelog

[goetas]

Hi, thanks for your contribution.
Is there a chance to have a test for your change?
Some tests are failing as well

[Steveb-p]

Is there a chance to have a test for your change?

The tests that are existing should cover it (specifically, JMS\TranslationBundle\Tests\Translation\Extractor\File\ValidationExtractorTest), since the primary change in this PR is using interface methods instead of public properties (which were marked as internal somewhere during Symfony lifecycle).

I'll give it a look and see if enough cases are covered. Probably not today though, tomorrow at earliest.

Some tests are failing as well

An error occurred inside PHPUnit.

Message:  Subscriber "PHPUnit\TextUI\Output\Default\ProgressPrinter\ChildProcessErroredSubscriber" does not implement any known interface - did you forget to register it?

I'll see what I can do about it, but I haven't been able to pinpoint why this happens. It doesn't seem to be related to the changes themselves, this will probably also blow up on 2.x and main in a similar fashion. Will debug it tomorrow. It's possible this will resolve "itself" since Symfony got a release recently, and maybe it's just a bug that got introduced.

EDIT: When I checked locally I realized that two versions of PHPUnit were installed: v11 for simple-phpunit, and v12 due to composer direct dependencies:

$ symfony composer why phpunit/phpunit
doctrine/deprecations                            1.1.5  conflicts phpunit/phpunit (<=7.5 || >=13)              
matthiasnoback/symfony-config-test               v6.1.0 requires  phpunit/phpunit (^10.5 || ^11.0 || ^12.0)    
matthiasnoback/symfony-dependency-injection-test 6.2.0  requires  phpunit/phpunit (^10.5.11 || ^11.5 || ^12.0)

Added PHPStan 11.5 directly as dev dependency to composer.json to ensure correct version is installed, because otherwise they were mixing when initializing:

SYMFONY_PHPUNIT_VERSION=11.5 SYMFONY_DEPRECATIONS_HELPER=weak vendor/bin/simple-phpunit


An error occurred inside PHPUnit.

Message:  Subscriber "PHPUnit\TextUI\Output\Default\ProgressPrinter\ChildProcessErroredSubscriber" does not implement any known interface - did you forget to register it?
Location: /.../JMSTranslationBundle/vendor/bin/.phpunit/phpunit-11.5-0/src/Event/Dispatcher/DirectDispatcher.php:55

#0 /.../JMSTranslationBundle/vendor/bin/.phpunit/phpunit-11.5-0/src/Event/Dispatcher/DeferringDispatcher.php(36): PHPUnit\Event\DirectDispatcher->registerSubscriber()
#1 /.../JMSTranslationBundle/vendor/bin/.phpunit/phpunit-11.5-0/src/Event/Facade.php(74): PHPUnit\Event\DeferringDispatcher->registerSubscriber()
#2 /.../JMSTranslationBundle/vendor/bin/.phpunit/phpunit-11.5-0/src/Event/Facade.php(60): PHPUnit\Event\Facade->registerSubscriber()
#3 /.../JMSTranslationBundle/vendor/phpunit/phpunit/src/TextUI/Output/Default/ProgressPrinter/ProgressPrinter.php(304): PHPUnit\Event\Facade->registerSubscribers()
#4 /.../JMSTranslationBundle/vendor/phpunit/phpunit/src/TextUI/Output/Default/ProgressPrinter/ProgressPrinter.php(61): PHPUnit\TextUI\Output\Default\ProgressPrinter\ProgressPrinter->registerSubscribers()
#5 /.../JMSTranslationBundle/vendor/phpunit/phpunit/src/TextUI/Output/Facade.php(166): PHPUnit\TextUI\Output\Default\ProgressPrinter\ProgressPrinter->__construct()
#6 /.../JMSTranslationBundle/vendor/phpunit/phpunit/src/TextUI/Output/Facade.php(55): PHPUnit\TextUI\Output\Facade::createProgressPrinter()
#7 /.../JMSTranslationBundle/vendor/bin/.phpunit/phpunit-11.5-0/src/TextUI/Application.php(147): PHPUnit\TextUI\Output\Facade::init()
#8 /.../JMSTranslationBundle/vendor/bin/.phpunit/phpunit-11.5-0/phpunit(104): PHPUnit\TextUI\Application->run()
#9 /.../JMSTranslationBundle/vendor/symfony/phpunit-bridge/bin/simple-phpunit.php(459): include('...')
#10 /.../JMSTranslationBundle/vendor/symfony/phpunit-bridge/bin/simple-phpunit(13): require('...')
#11 /.../JMSTranslationBundle/vendor/bin/simple-phpunit(119): include('...')
#12 {main}

☝️ you can see above that initialization is done from v11.5, but Facade is called from main vendor (v12), and tries to use wrong setup.

[Steveb-p]

Any chances for a release @goetas ? :)

[JinbeiStudio]

@goetas Do you know when this will be added to a release ? The bundle is broken because of this on Symfony 7.4.

Thank you 👍

[goetas]

I have tagged now a new release(2.7.0)