Typehint className in instantiate method

[Valouleloup]

I think this was not Typehinted for BC break reasons but since in src/Doctrine/Instantiator/Instantiator.php the method instantiate calls the buildAndCacheFromFactory method where className is Typehinted as string , I don't see in which scenario $className could not be a string.

Moreover, in the interface the $className parameter is already described as string in the annotation.

If we accept classes that implement InstantiatorInterface but don't use a string as parameter (unlikely) then we need to remove the annotation. Right ?

[Ocramius]

This is a major BC break, as it breaks implementations :-\

[Valouleloup]

Hence my comment above (PR comment)

[Majkl578]

This is a major BC break, as it breaks implementations

Note: Not with PHP 7.2+.

But then it should have : object return type, which does break implementations (but can be made forward compatible with both Instantiator 1.x and 2.x).

[alcaeus]

If we accept classes that implement InstantiatorInterface but don't use a string as parameter (unlikely) then we need to remove the annotation. Right ?

Why? The interface clearly defines that the $className argument should be a string. Why would it be anything else, and why would we widen the interface or the default implementation for it?

[Valouleloup]

Why? The interface clearly defines that the $className argument should be a string. Why would it be anything else, and why would we widen the interface or the default implementation for it?>

What is the point saying "It can be a string or it can be not" ?
Then it could have been @param array $className or @param int $className.
Why bother add an annotation if it doesn't give any information on who it must (not should) work ?

[Ocramius]

The only reason why this doesn't have a type hint is that it was introduced as a PHP 5.x polyfill. If bumping a major version for this particular change made sense, I would do it. For now, the PR simply stays in a limbo 👍

[derrabus]

Closing in favor of #94.