Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugins implementing FunctionReturnTypeProviderInterface are not triggered anymore #10275

Open
boesing opened this issue Oct 10, 2023 · 4 comments

Comments

@boesing
Copy link
Contributor

boesing commented Oct 10, 2023

Starting with v5.13.0, plugins providing FunctionReturnTypeProviderInterface are not properly triggered anymore.

To reproduce this, create a plugin like this:

final class ReturnTypeProvider implements FunctionReturnTypeProviderInterface
{
    private const FUNCTION_SPRINTF = 'sprintf';

    /**
     * @psalm-return non-empty-list<lowercase-string>
     */
    public static function getFunctionIds(): array
    {
        return [self::FUNCTION_SPRINTF];
    }

    public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $event): ?Type\Union
    {
        die('here');
    }
}

final class Plugin implements PluginEntryPointInterface
{
    public function __invoke(RegistrationInterface $registration, ?SimpleXMLElement $config = null): void
    {
        $registration->registerHooksFromClass(ReturnTypeProvider::class);
    }
}
@psalm-github-bot
Copy link

Hey @boesing, can you reproduce the issue on https://psalm.dev ?

@boesing
Copy link
Contributor Author

boesing commented Oct 10, 2023

Oh, as I can see, they are actually not triggered in case some other return type provider was providing a return type.
It seems that with 5.13.0, an sprintf return type provider was added and therefore, the FunctionReturnTypeProvider is early returning...

Any ideas on how plugins can get a higher priority than those handlers registered in the __construct?
My plugin does actually provide a more precise return type for sprintf.

Maybe FunctionReturnTypeProvider::registerClosure could use array_unshift?
Thoughts?

@kkmuffme
Copy link
Contributor

For inbuilt (default PHP) functions this is kind of on purpose (by accident :D) - if you have more specific types a PR would be welcome and make more sense than having this in a plugin, as everybody would benefit from that (if you have a better sprintf return type provider)

@boesing
Copy link
Contributor Author

boesing commented Oct 19, 2023

I prefer plugins as they are somewhat opionated and having this part of psalm itself would shift anything regarding issues to this repository which then also means maintenance.

I can add some parts to psalm but overall I'd prefer plugins having more power over internal stuff is why plugins exist in the first place. 🤔

So I'd be fine with migrating some stuff to psalm while having my plugin for some more specific/opinionated/to be tested stuff on top. Which will then still require psalm to execute plugins afterwards.

It was some what unexpected that suddenly my plugin return type provider was not called anymore.

Maybe even call any plugin and then use type combinator to merge results 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants