Remove the generic type for PasswordUpgraderInterface

[stof]

This interface method can be called on a user provider even for user types not supported by that provider (for instance when using a chain provider). The implementation is expected to deal with that case gracefully, which will be enforced by phpstan when the generics are gone.

Closes #427

This reflects the removal done in Symfony 6.4 (reverting the PR adding it initially) so that the change done in Symfony to actually enforce correct implementations of that interface is also taken into account when adding this extension: symfony/symfony#51283

[ondrejmirtes]

/cc @VincentLanglet You originally added this in #318. Do you agree with the removal?

[VincentLanglet]

This interface method can be called on a user provider even for user types not supported by that provider (for instance when using a chain provider). The implementation is expected to deal with that case gracefully, which will be enforced by phpstan when the generics are gone.

This is untrue when the chainProvider is not used, there is at least two implementations which doesn't deal with that case gracefully:

• https://github.com/symfony/symfony/blob/07e020abdd1ab820ca3c2ead8dfda75ea6e84eae/src/Symfony/Component/Ldap/Security/LdapUserProvider.php#L115
• https://github.com/symfony/symfony/blob/07e020abdd1ab820ca3c2ead8dfda75ea6e84eae/src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php#L120

So currently when calling PasswordUpgraderInterface::upgradePassword you're not sure no exception will be thrown.

To be consistent those exception should be removed no @stof ?

/cc @VincentLanglet You originally added this in #318. Do you agree with the removal?

Anyway, seem the original PR #318 was made to be sync with symfony/symfony#48750
since Symfony removed the generics on their side, it's logic to do the removal here too.

[stof]

Looking at the implementation of ChainUserProvider, the actual allowed behavior is either be silent or throw UnsupportedUserException (but not other exceptions) for unsupported users. But the implementation must still check what it gets as input (it cannot assume it always gets a supported user instance).
The core implementations are throwing UnsupportedUserException, which is their way of dealing with getting other kind of users.

[VincentLanglet]

Looking at the implementation of ChainUserProvider, the actual allowed behavior is either be silent or throw UnsupportedUserException (but not other exceptions) for unsupported users. But the implementation must still check what it gets as input (it cannot assume it always gets a supported user instance).

Then the removal of the generic doesn't make sens in symfony/symfony#51283 and I feel wrong the fact the author or the team didn't ping me about the PR.

When you use doctrine collection and write Collection<Dog> somewhere, it doesn't fully guarantee you that

$collection->get(0)

returns a Dog since I can write

$collection->add(new Cat());

without any error, except a static analysis one.

It's the same with PasswordUpgraderInterface. It cannot fully guarantee that the User inside the method is the one expect but at least when calling PasswordUpgraderInterface:: upgradePassword($wrongUser) a static analysis error will be reported to explain.
Without this static analysis error I can end up with production error because a UnsupportedUser exception will be thrown.

You consider PasswordUpgraderInterface to be silent when a wrong user is passed, but it's true only for the ChainUserProvider not all implementations of the PasswordUpgraderInterface. To this to be true, we should change the

throw new UnsupportedUserException(\sprintf('Instances of "%s" are not supported.', get_debug_type($user)));

by a simple return ; in every implementation of PasswordUpgraderInterface.

But even if Symfony is right or wrong on the removal of the Generic, PHPStan just have to follow to be in sync.

[stof]

@VincentLanglet the removal of the generic in core was done so that implementations of the interface are forced to consider that the user they receive might not be the type they support. With your generic type, phpstan would never tell you that your implementation is broken if you assume that $user is always your own user class.
I opened symfony/symfony#60009 to improve the documentation of the interface.

[VincentLanglet]

With the @throws UnsupportedUserException that make sens yeah.

[stof]

@ondrejmirtes any chance to merge this ?