Leverage LazyProxyTrait in LazySchemaDiffProvider

[nicolas-grekas]

Q	A
Type	improvement
BC Break	no (LazySchemaDiffProvider is internal)
Fixed issues	-

This PR proposes to leverage the new LazyProxyTrait that's going to be shipped with Symfony 6.2 to implement lazy schemas as required by LazySchemaDiffProvider. (See https://symfony.com/blog/revisiting-lazy-loading-proxies-in-php for an introduction to this trait.)

This would require bumping the minimum PHP version to 8.1 in the next minor version.
The main benefit is simplicity. As you can read in the patch, using this trait requires no proxy generation infrastructure, and no calls to eval() either. The other benefit is simplifying dependency management.

[nicolas-grekas]

phpstan report is only about false positives:

Property Doctrine\Migrations\Provider\LazySchema::$lazyObject* is unused

Yes it is: by the trait.
(And no, the trait cannot declare those properties because their type/modifiers can vary in classes using it.)

Result of && is always true.

No it's not: undefined properties are not in objects cast to arrays.

[derrabus]

Add those errors to the ignoreErrors section in phpstan.neon.dist with a comment explaining why you want to ignore them. If you consider the errors PHPStan bugs and want to go the extra mile, report them to PHPStan and add a link to your issue as a comment to phpstan.neon.dist.

[stof]

Merging this is blocked on the stable release of Symfony 6.2 though

[stof]

Looking at https://packagist.org/packages/doctrine/migrations/php-stats#3, PHP 8.1 only represents 45% of the installations for doctrine/migrations 3.x. The bump might be too early (the wait for the stable release of Symfony 6.2 gives us 2.5 months before re-evaluating that ratio anyway)

[nicolas-grekas]

phpstan green, thanks for the hint @derrabus
The remaining failures were already there so 🤷

@stof indeed, but Symfony 6.2 is pretty close (end of November) so we can start considering this now. v3.5 will still be usable for ppl on php < 8.1. The bump is worth it IMHO.

[derrabus]

My attempt at getting the CI green: #1274

[stof]

Property Doctrine\Migrations\Provider\LazySchema::$lazyObject* is unused

Yes it is: by the trait. (And no, the trait cannot declare those properties because their type/modifiers can vary in classes using it.)

@nicolas-grekas This is true for lazyObjectReal. But AFAICT, lazyObjectId is always an int. So couldn't we declare it in the trait ?

[nicolas-grekas]

lazyObjectId is always an int. So couldn't we declare it in the trait ?

I could not add it because it would make the trait incompatible with readonly classes (there's a test case about that in the component)

[stof]

I could not add it because it would make the trait incompatible with readonly classes (there's a test case about that in the component)

if the trait supports making the property readonly, maybe it should always have it readonly ? Non-readonly classes can have readonly properties.

[nicolas-grekas]

Making lazyObjectId readonly breaks cloning... I opened a discussion on php-internals about this and almost nobody dared to care for now... I hope this could be resolved soon...

[derrabus]

Can you please rebase? The CI on 3.6.x should be green now.

[nicolas-grekas]

Rebased. I don't reproduce the deps=low failure locally, I need to understand why it happens. I thought it might be because of composer/composer#11068 but apparently not.

[nicolas-grekas]

I figured out the issue, all green.

[stof]

this method should probably be deprecated instead of being removed

[nicolas-grekas]

the class is marked @internal so I didn't bother doing that :)

[stof]

ah indeed. I missed that

[nicolas-grekas]

Ready for merge!
(failures are unrelated IIUC)

🚀

[greg0ire]

Just did a merge-up, it should fix the SA job (the BC check will still be broken).

[greg0ire]

A lot less code :)

[bobvandevijver]

@nicolas-grekas I'm just seeing this update, but due to the requirement on symfony/var-exporter on ^6.2 combined with the flex functionality to limit the Symfony packages to 5.4.* (symfony.require in the composer file), the latest release isn't installable. Would that be an oversight or was it intentional to require SF 6.2?

[stof]

older versions of var-exporter don't have that LazyProxyTrait

[nicolas-grekas]

Thanks for the report. This should now be fixed. I did so by reporting that var-exporter didn't existing before 6.2, see
symfony-tools/recipes-checker@a50caac

This should allow installing var-exporter at v6.2 while the rest is at 5.4.
This is also important to help Doctrine ORM adopt lazy-proxies faster.
/cc @derrabus since we talked about this issue recently.

[derrabus]

Thank you! That should help indeed.

[bobvandevijver]

This fixed it indeed, thanks!