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

Generics are always considered as Collection #180

Open
nikophil opened this issue Dec 16, 2022 · 6 comments
Open

Generics are always considered as Collection #180

nikophil opened this issue Dec 16, 2022 · 6 comments
Milestone

Comments

@nikophil
Copy link

nikophil commented Dec 16, 2022

Hello,

whether on branch 1.x or on the last tagged version, generics are always considered as a collection although this is not always true.

Is it made on purpose or are you planing to extend the method TypeResolver::createFromGeneric() to handle the cases where generics are something else than a collection?

thanks for your answer!

@nikophil nikophil changed the title Generics always considered as Collection Generics are always considered as Collection Dec 16, 2022
@jaapio
Copy link
Member

jaapio commented Dec 16, 2022

Disclaimer: I'm writing this without overthinking the impact and possibility of implementing this.

When we implemented the first generics version, there was no good support for this in our codebase. The generic support was kind of hacked into the code. By now, we should be able to change this. However, it will require some extra work. We do not want to introduce any backward compatibility breaks at this point.

This means that a v1 version will still return all generics as a subtype of collection. A v2 version which is unavoidable at this point, may have a breaking change allowing us to introduce a more complex generic support. But as this library cannot resolve the generic part, it might be a more complex issue. We are analyzing the types per PHP-file. Generics are combinations of types over files. This means that the templated values of the generic need to be resolved in another context.

Given this class:

/** @template T **/
class Foo {
   /** @return T */
    public function getValue(): object
}

Here we cannot tell you the value of T. As T is defined at the moment of using the class Foo<MyObject>

So depending on your needs, you might need another layer of reflection to get this information. The other layer will never be added to this library. When possible, we could add it in https://github.com/phpDocumentor/ReflectionDocBlock, But that will never cover all situations because it also does the per-file Reflection.

The final result can only be part of https://github.com/phpDocumentor/Reflection, where we analyze an entire code base. And even then, we will not be able to discover everything, because to discover the actual values you also need to include the vendor directories. Which tools like phpstan are doing.

From a documentation perspective, it might be enough to tell the consuming project that we discovered a generic object notation.

/** @return Foo<MyObject> */
public function getFoo(): Foo {}

This could result in a future version in: new GenericType(new Fqsen('\Foo'), [new Object_(new Fqen('\MyObject'))]);.

@nikophil
Copy link
Author

thanks for this complete answer! I was indeed thinking it is a complex problem.

to give you a little bit of context, I don't know if you have a Symfony knowledge, but this problem prevents to correctly document such case with Symfony's serializer:

MyClass
{
    /** @var SomeGeneric<SomeClass> */
    public SomeGeneric $property;
}

used along with something like SomeGenericNormalizer which is triggered when the serializer encounters a SomeGeneric data.
The property is considered as a collection, and the custom normalizer is skipped because it now seeks for a normalizer for SomeClass[]. (see here and here).

I don't know if there could be a solution in Symfony-land to correctly handle this. Couldn't we at least add a bool property on Type that says it was a generic notation or not?

@jaapio
Copy link
Member

jaapio commented Dec 16, 2022

I know Symfony quite well. I use it on a daily bases. Maybe we should elaborate a bit more on this particular topic because I see different use cases that I would like to see in the Symfony serializer.

If we could add support for generics, it would allow Symfony to serialize in more complex situations. Like serializing

SomeGeneric<SomeClass> could be done differently than SomeGeneric<OtherClass>, right?

Please let me know what you need, and I'm open to collaborating with you on this get improve our loved framework :-)

@nikophil
Copy link
Author

nikophil commented Dec 16, 2022

SomeGeneric could be done differently than SomeGeneric, right?

yes, of course.

I think from the perspective of this lib, indeed a new GenericType would be perfect
From Symfony's perspective, the PropertyInfo would have to evolve, we would need to access to the generic's child class in \Symfony\Component\PropertyInfo\Type. I'm not really sure such a move would be accepted 🤔


I presently use #[Context()] tu pass the child class. ie:

    /** @var SomeGeneric<SomeClass> */ //  <== this breaks the serialization
    #[Context([SomeGenericNormalizer::CONTEXT_GENERIC_CLASS => SomeClass::class])]
    public SomeGeneric $property;

this is actually the case that made me open this issue :)

@schodemeiss
Copy link
Contributor

schodemeiss commented Apr 3, 2023

I'm walking into the same (I think) issue when I use value-of<Type>; which is a Psalm annotation (https://psalm.dev/docs/annotating_code/type_syntax/utility_types/#value-oft).

I don't have a good story for how to avoid this at the moment, which unfortunately stops me being able to upgrade to Symfony 6.2.8 (which includes 1.7.1 of TypeResolver).

@uuf6429
Copy link

uuf6429 commented Jul 14, 2024

I guess things have slightly been changed, but this issue is still relevant. Here's the source of the problem:

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

4 participants