thephpleague / oauth2-server

A spec compliant, secure by default PHP OAuth 2.0 Server
https://oauth2.thephpleague.com
MIT License
6.49k stars 1.12k forks source link

Implement phpstan generics in interface #1281

Closed sylfabre closed 2 years ago

sylfabre commented 2 years ago

Hello

I'm working on leveling up my phpstan setup to level 7 and I'm struggling with my Doctrine entities implementations of your token interfaces.

My OAuthAccessToken entity use the following code:

    /**
     * @var Collection<int, OAuthScope>
     * @ORM\ManyToMany(targetEntity="OAuthScope")
     */
    protected Collection $scopes;

    /**
     * @return OAuthScope[]
     */
    public function getScopes(): array
    {
        return $this->scopes->toArray();
    }

My entity OAuthScope implements ScopeEntityInterface.

It works well with phpstan Doctrine extension as $scopes is actually an array of OAuthScope that is expected by Doctrine. But it breaks contravariance and return types because the phpdoc of AccessTokenEntityInterface is all about ScopeEntityInterface.

A way to correctly fix this is to use @template annotation as explained in https://phpstan.org/blog/generics-in-php-using-phpdocs#these-rules-are-useful%2C-but-sometimes-limiting

In my case, the required code in your package is:

/**
 * @template TScope of ScopeEntityInterface
 */
interface TokenInterface

/**
 * @template TScope of ScopeEntityInterface
 * @extends TokenInterface<TScope>
 */
interface AccessTokenEntityInterface extends TokenInterface

and in my user-land code:

/**
 * @implements AccessTokenEntityInterface<OAuthScope>
 */
class OAuthAccessToken implements AccessTokenEntityInterface

What do you think? Would you accept a PR for this?

And by the way, thank you for this amazing lib 💪

Sephster commented 2 years ago

@sylfabre thank you for the kind words and sorry it has taken me a while to get back to you. I'm working on trying to get version 9 out so at this time, if you have a fix in userland, I'd prefer not to accept a PR for this. Primarily this is because I don't want to distract from other features and I think this will have limited impact for most of our users at present. I hope you understand and thank you very much for the offer of the PR.

sylfabre commented 2 years ago

@Sephster I understand that, good luck with version 9!