Test AclProvider change

[ondrejmirtes]

Hi @VincentLanglet,
I had to change the stubs you contributed because of generic covariance error (41fff98). Can you please test phpstan/phpstan-symfony:dev-master to see if it works as expected in real-world projects? Thank you.

[VincentLanglet]

The change you made is fixing the issue: #149

But now I'm getting the following errors:

Property Sonata\AdminBundle\Security\Handler\AclSecurityHandler::$aclProvider with generic interface Symfony\Component\Security\Acl\Model\MutableAclProviderInterface does not specify its types: TAcl                    
         💡 You can turn this off by setting checkGenericClassInNonGenericObjectType: false in your phpstan.neon.dist.                                                                                                              
  75     Method Sonata\AdminBundle\Security\Handler\AclSecurityHandler::__construct() has parameter $aclProvider with generic interface Symfony\Component\Security\Acl\Model\MutableAclProviderInterface but does not specify its  
         types: TAcl               

This is not natural to fix because when I click on the classes, I end up in the Symfony code, with no template.

Also, if I add the generic type, I end up with a psalm error

psalm: TooManyTemplateParams: Symfony\Component\Security\Acl\Model\MutableAclProviderInterface<Symfony\Component\Security\Acl\Model\MutableAclInterface> has too many template params, expecting 0

So it requires to write

@phpstan-var MutableAclProviderInterface<MutableAclInterface>
@psalm-var MutableAclProviderInterface

Or the same stub should be added to the psalm symfony plugin. (which is not a bad idea since I have an issue similar to #149 with psalm)

[ondrejmirtes]

So the problem I'm trying to solve here is this error:

 ------ ----------------------------------------------------------------------------- 
  Line   stubs/Symfony/Component/Security/Acl/Model/MutableAclProviderInterface.stub  
 ------ ----------------------------------------------------------------------------- 
  18     Return type                                                                  
         (SplObjectStorage<Symfony\Component\Security\Acl\Model\ObjectIdentity        
         Interface, Symfony\Component\Security\Acl\Model\MutableAclInterface>)        
         of method                                                                    
         Symfony\Component\Security\Acl\Model\MutableAclProviderInterface::fin        
         dAcls() should be compatible with return type                                
         (SplObjectStorage<Symfony\Component\Security\Acl\Model\ObjectIdentity        
         Interface, Symfony\Component\Security\Acl\Model\AclInterface>) of            
         method                                                                       
         Symfony\Component\Security\Acl\Model\AclProviderInterface::findAcls()        
 ------ ----------------------------------------------------------------------------- 

This is easily reproducible with plain PHPStan: https://phpstan.org/r/e4afbbd9-8a90-489c-82ca-a071c759f52f

The error is valid - SplObjectStorage doesn't have @template-covariant because of the attach() method. So I'm trying to solve it with the linked commit. The downside is that now the AclProviderInterface and MutableAclProviderInterface are generic. Which might have an upside as well - the user that's typehinting these interfaces can specify what should be in them.

WDYT?

[VincentLanglet]

The error is valid - SplObjectStorage doesn't have @template-covariant because of the attach() method. So I'm trying to solve it with the linked commit. The downside is that now the AclProviderInterface and MutableAclProviderInterface are generic. Which might have an upside as well - the user that's typehinting these interfaces can specify what should be in them.

WDYT?

Sure, that was the first solution I had in mind to solve this issue: #92
You wanted to avoid this,
cf #149 (comment)
and #92 (comment)
but I'm not sure we can avoid it now

I just think the same PR should be done to psalm-symfony in order to avoid the TooManyTemplateParams situation.

Best would be of course to add the template directly to symfony code, but I'm not sure they start to added some @template to their code.

[ondrejmirtes]

Yeah, because I thought that it wasn't necessary to make the interfaces generic, but apparently it is.

Please open an issue in https://github.com/psalm/psalm-plugin-symfony/issues, ask them what they think and link this issue there. Thank you.

[VincentLanglet]

I opened psalm/psalm-plugin-symfony#189

[VincentLanglet]

I also opened symfony/security-acl#83 ; I assume it would avoid phpstan to use a stub for these interface, isnt it ?

[VincentLanglet]

@ondrejmirtes symfony/security-acl#83 was merged.
Should we remove the stub here ?

[ondrejmirtes]

Removed and released, thanks.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.