Override/improve ContainerInterface::get return type hints in ServiceLocatorInterface (generics, class-string)

[InvisibleSmiley]

Feature Request

Q	A
New Feature	yes
RFC	no
BC Break	no

Summary

Currently, ServiceLocatorInterface extends ContainerInterface such that only the build method is added. The get method is inherited by ContainerInterface and not modified. ServiceManager extends ServiceLocatorInterface and uses {@inheritDoc} for get so it inherits whatever PHPDoc ContainerInterface and ServiceLocatorInterface define (FTR @inheritDoc can be dropped safely I think).

I suggest to override the get in ServiceLocatorInterface so that it behaves the same as build for Psalm and PHPStan when the input is a class-string. Should be as simple as adding:

    /**
     * Finds an entry of the container by its identifier and returns it.
     *
     * @template T of object
     * @param string|class-string<T> $id Identifier of the entry to look for.
     * @return mixed Entry.
     * @psalm-return ($id is class-string<T> ? T : mixed)
     * @throws ContainerExceptionInterface Error while retrieving the entry.
     * @throws NotFoundExceptionInterface  No entry was found for **this** identifier.
     */
    public function get(string $id);

The only drawback is that ServiceLocatorInterface then becomes prone to ContainerInterface method signature changes. The last change to the ContainerInterface::get was made in psr/container version 1.1.0, so version 1.0.0 would be incompatible but all other version up to / including 2.0.2 would be compatible.

[Ocramius]

While the convention is indeed to have class-string<T> -> T, nothing prevents mapping A::class to new B(), so I don't think we should do this :|

[InvisibleSmiley]

While the convention is indeed to have class-string<T> -> T, nothing prevents mapping A::class to new B(), so I don't think we should do this :|

Well that is true but then how do you justify that this logic is applied to build (already)?

Personally I feel that anything can be misused but we should really aim at supporting the "sane" use case and help developers reduce "needless" checks, assertions and inline PHPDocs.

[Ocramius]

Agreed, just trying to separate "what's allowed" vs "what's convention".

Since the tooling does not really check much, there's still a lot of space for misunderstanding.

I'd say: send a patch, let's discuss there :+1:

[sasezaki]

[FYI] (for other developers)

There is psalm/phpstan plugin for psr/container to get benefit of ($id is class-string<T> ? T : mixed)

• https://github.com/Lctrs/psalm-psr-container-plugin
• https://github.com/bnf/phpstan-psr-container

[sasezaki]

IMO, If ServiceLocatorInterface is marked as internal, I have no objection

[boesing]

ServiceLocatorInterface is not marked as internal due to the fact that the build method is kinda "open". However, neither for get nor for build (in the ServiceLocatorInterface) we should provide these generics as that is not guaranteed at all that an instance of the class is being returned what is requested.

IMO only verified way would be via PluginManagerInterface where both get and build method have generics. That is due to the fact that default functionality of a plugin manager is to ensure only specific types are being returned (as stated in the generic). I do not see why projects would consume ServiceLocatorInterface and thus, I tend to create an RFC to remove that entirely in v5.0.0 as there should be either ContainerInterface for service manager implementations or PluginManagerInterface for plugins, so no need for ServiceLocatorInterface. 🤷🏼‍♂️

For the sake of having this in-sync within ServiceLocatorInterface, I'd say lets do it.

[InvisibleSmiley]

I do not see why projects would consume ServiceLocatorInterface and thus, I tend to create an RFC to remove that entirely in v5.0.0

Simply for the ability to be able to use the build method without requiring a ServiceManager instance of course.

Example (real-world) use case:

• Given: A database generic schema access service
• Provided functionality:
  • return a DAL for the given schema (with a schema-specific connection as input)
  • return a DAL for a given table (where the DAL then uses the table name to auto-prefix column names)

This service uses a ServiceLocatorInterface object internally to build generic schema DALs and generic table DALs.

I've never used the PluginManagerInterface before; do you imply that I should replace the ServiceLocatorInterface object in this case by one or two plugin managers?

If not, I suggest to have this case in mind when you think about getting rid of the ServiceLocatorInterface in the future.