Invalid type 'Nette\DI\?T' when accessing `Container#getByType()` from DI config
Closed this issue · 23 comments
Version: 3.0.11, bug present as of 3.0.10
Bug Description
If you use Container#getByType()
inside of NEON configuration, it fails on resolving its return type – from Helpers#getReturnTypeAnnotation()
it is returned Nette\DI\?T
.
More generally, I believe (have not tried) accessing any class with method having generic return type will fail.
Steps To Reproduce
In nette/web-project
, create this class:
namespace App;
final class FakeContainer
{
/**
* copied from Container#getByType()
* @template T
* @param class-string<T> $type
* @return ?T
*/
public function getService(string $type)
{
return null;
}
}
In services.neon
, use it:
services:
- App\FakeContainer::getService(HomepagePresenter::class)
Expected Behavior
Run, not to fail with type check.
What if you declare it as following?
public function getService(string $type): object
{}
This seems like expected behavior, generics are not supported.
DI doesn't support generics and I don't plan to implement it, in fact I plan to remove support for @return annotation completely in version 4. But you can specify the type directly in the config:
services:
- factory: App\FakeContainer::getService(HomepagePresenter::class)
type: HomepagePresenter
Is there any reason to not support object
?
It is always object.
I mean it's bit confusing from the user's point of view that object is not allowed return type of method. But in reality it's okay and we just have to declare more specific type in neon because library is not able to infer it from generics itself. Some hint like "I don't understand object / Foo&Bar / ... type, could you change the type to some specific class or add the type to service definition?" would be helpful
It's not that it doesn't understand it, it understands and doesn't allow it. So return type is not expected to be nullable/union/intersection/built-in type.
@dg Thank you, but the suggestion did not help for the example code – it fails with Return type of App\FakeContainer::getService() is not expected to be nullable/union/intersection/built-in, 'object' given
Invalid App\?T
* (sorry, the long message was for forced object
return type)
It didn't help in real code either as I already have type
already set there. So maybe there is also bug in type
not making any effect?
For illustration, I attach real code:
pages.filesystem:
type: League\Flysystem\Filesystem
factory: @outerContainer::getByType(PageFilesystemProvider)::getPageFilesystem()
autowired: false
Just an explanation of our use-case: we have app and model container – app is outer, model inner. App can use services from inner (model) container, but model container needs some bindings to the app world – e.g. translator, identity etc. – thus this @outerContainer::getByType()
calls.
I understand that such usage of DI can be a bit marginal/edge-case (🤷♂️❓), but I still think it reveals some bug(s) in DI resolver/type checking.
* The message actually changed from Nette\DI\ServiceCreationException: Service (App\FakeContainer::getService()): Invalid type 'App\?T'
(reported in the original post) to Nette\InvalidArgumentException: Invalid type 'App\?T'
so it must failed at different place.
Yeah, I finally get it, the problem is the throwing of the exception.
So it should work that if the return type is 'object', it will ignore the @return annotation and 'object' too, right?
So it should work that if the return type is 'object', it will ignore the @return annotation and 'object' too, right?
Not pretty sure. I don't have such knowledge of DI to tell if this is solution to the root cause, sorry. I'm not even sure I know what is the root cause actually.
But yeah, the problem is that it throws exception instead of returning HomepagePresenter
/PageFilesystemProvider
instance.
Try v3.0-dev, now is should work.
final class FakeContainer
{
/**
* @template T
* @param class-string<T> $type
* @return ?T
*/
public function getService(string $type): ?object // @return ?T is ignored now
{
}
}
@dg I'm sorry, but it's still the same. From the referenced commit, I don't even understand how it should help as there was just added one failing scenario.
But if you plan remove generic annotation from Container#getByType()
in v4 (especially the @return ?T
line), that helps, so I can wait with update until it is published.
@dg I hope I can describe it better now. The problem is that if I call Container#getByType()
in NEON configuration, it fails with Invalid type 'Nette\DI\?T'
instead of returning the requested service. The cause is that Container#getByType()
has generic @return
annotation.
You said you plan to remove it, is that still true? I can still see it in master and I'm stuck because of this bug on 3.0.9
.
Try to create some minimal project, I really don't understand what is not working for you.
Btw what is the hash in Container#getByType()
?
Here you go: https://github.com/dakur/nette-di-274
Clone it, composer install
, run webserver and open in browser. You see the message. If you remove return annotation from Container#getByType()
, it starts working.
(I run it on php 8.1, don't know about 8.2)
Btw what is the hash in Container#getByType()?
What do you mean by hash?
I mean #
in Container#getByType()
Does it have any special meaning?
I think it's a way how to refer to a class method without associating it with static or non-static usage. But I could write Container::getByType()
as well.
Oh, I've never seen that before. I see ::
as way how to refer to a class method without associating it with static or non-static usage. Then it's just a matter of context.
Oh, I see what's going on. The problem is the calling of another method, when DI tries to evaluate over which object is called
services:
- App\FakeContainer::getService(HomepagePresenter::class)::anotherMethod()
It should be working now in both 3.0-dev and 3.1-dev.
Evaluation is done to call the method properly, right?
It would be nice to have PHP-like syntax for constants/properties/(static) methods in neon.
e.g. @Nette\Http\IRequest->getUrl()->getBasePath()
instead of @Nette\Http\IRequest::getUrl()::getBasePath
.
DI would not have to evaluate these calls.
(sorry for asking for complicated changes)
Looks like it works, thank you!