TaintAnalysis: Tags of interface take precedence over class declaration

[ohader]

https://psalm.dev/r/818815f379

This seemed to be working with vimeo/psalm:~5.8.0, but change somehow on the way to ~5.9.0.
5.8.0...5.9.0 → git bisect points to commit dfd7ffc

Does not work (~5.9.0, tested with 5.x-dev 2bbfca6)

class HandlerImplementation implements HandlerInterface
{
    /**
     * @psalm-taint-specialize
     * @psalm-taint-sink sql $value
     */
    public function execute(string $value, array $params = []): static {}
}

Removing implements HandlerInterface works in ~5.9.0

class HandlerImplementation
{
    /**
     * @psalm-taint-specialize
     * @psalm-taint-sink sql $value
     */
    public function execute(string $value, array $params = []): static {}
}

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/818815f379

<?php // --taint-analysis

interface HandlerInterface
{
    public function execute(string $value, array $params = []): static;
}

// it works, after removing `implements HandlerInterface` 
class HandlerImplementation implements HandlerInterface
{
    /**
     * @psalm-taint-specialize
     * @psalm-taint-sink sql $value
     */
    public function execute(string $value, array $params = []): static {}
}

/**
 * @return HandlerInterface|HandlerImplementation
 */
function resolveHandler() {}

$subject = resolveHandler()->execute($_GET['inject']);

Psalm output (using commit 2bbfca6):

No issues!

[orklah]

Oh, I see what happened.

Before my change in #9562, the type returned by resolveHandler() was HandlerInterface|HandlerImplementation

HandlerImplementation being a subtype of HandlerInterface, it's now naturally combined as HandlerInterface

So there's two way to see the issue here:

• the first one, you could say that Psalm should see the taint even when the type is HandlerImplementation because one of its child declares a taint so using this interface is risky. This is technically true but Psalm rarely makes this kind of "a subtype may possibly declare this so I need to check them all"
• Psalm could enforce that if any child method declares a taint, every parent method shoulld have this taint declared (the same way as you can't suddenly have a child that returns a type not present in the parent)

I think the second option is the best IMHO so I'll flag this as enhancement.