psalm-plugin-symfony icon indicating copy to clipboard operation
psalm-plugin-symfony copied to clipboard

Templated ServiceLocator to be used for tagged services

Open zmitic opened this issue 3 years ago • 4 comments

With service locator anti-pattern being deprecated long ago, we can now use ServiceLocator class for tagged services. If you check compiler passes, it is only used for that anyway.

Use case:


Tagged services must have interface. So fictional exporter:


#[AutoconfigureTag(name: self::class)]
interface ExporterInterface
{
    public static function getName(): string;

    public function export(User $user): string;
}

Simple implementation:

class JsonExporter implements ExporterInterface
{
    public static function getName(): string
    {
		return 'json';	
    }

    public function export(User $user): string
	{
		// convert $user to json string
	}
}

Real usage with autowiring (other cases also work in 100% same way):



class Exporter
{
    /**
     * @param ServiceLocator<ExporterInterface> $strategies
     */
    public function __construct(
        #[TaggedLocator(tag: ExporterInterface::class, defaultIndexMethod: 'getName')]
        private ServiceLocator $strategies,
    )
    {
    }

    public function export(User $user, string $name): string
    {
        $exporter = $this->strategies->get($name); // psalm now knows it is instance of ExporterInterface
        
        return $exporter->export($user);
    }
}

So in the above case, site visitors would pick name via some dropdown or button, and that value sent to Exporter class.

Potential problem:

None found for last 6+ months. The only issue I can think of is if someone uses ServiceLocator class in undocumented ways.

I.e. Symfony still allows public services but they are discouraged, and all documentation is using ContainerInterface; something that ServiceLocator extends.

But after testing it for long time, I think it is safe to stub it.

Updated:

This is the feature I mentioned, if someone is not familiar with it: https://github.com/symfony/symfony/issues/39924

zmitic avatar Apr 07 '22 13:04 zmitic

@seferov @VincentLanglet

Can you guys take a look at idea? Do you think it is worth it?

zmitic avatar May 14 '22 11:05 zmitic

Can you guys take a look at idea? Do you think it is worth it?

Symfony project started to add some template in their code. For instance: https://github.com/symfony/symfony/pull/45696. I think you should try to make the PR on this repo first. The more generic are added to symfony the less work it five for plugins maintainer.

Moreover, every new generic added to psalm repository, need to be added to phpstan repository too (for developers which are using both tools). So it simpler if it's on symfony repo.

VincentLanglet avatar May 14 '22 11:05 VincentLanglet

@VincentLanglet Agreed, but how do you test it? Are those even needed?

zmitic avatar May 14 '22 11:05 zmitic

@VincentLanglet Agreed, but how do you test it? Are those even needed?

Your example

class Exporter
{
    /**
     * @param ServiceLocator<ExporterInterface> $strategies
     */
    public function __construct(
        #[TaggedLocator(tag: ExporterInterface::class, defaultIndexMethod: 'getName')]
        private ServiceLocator $strategies,
    )
    {
    }

    public function export(User $user, string $name): string
    {
        $exporter = $this->strategies->get($name); // psalm now knows it is instance of ExporterInterface
        
        return $exporter->export($user);
    }
}

seems to be a good one.

So a template T of object might be ok.

VincentLanglet avatar May 14 '22 11:05 VincentLanglet