phpDocumentor / ReflectionDocBlock

MIT License
9.34k stars 117 forks source link

Add @template & Conditional return types #372

Open ahjdev opened 2 months ago

ahjdev commented 2 months ago

https://phpstan.org/writing-php-code/phpdoc-types#conditional-return-types

@template T of int|array @return (T is int ? static : array)

I am not sure that you support psalm-types too

ahjdev commented 2 months ago

Recently I find out it doesnot detect @implements tag too. https://phpstan.org/blog/generics-in-php-using-phpdocs#class-level-generics

uuf6429 commented 2 months ago

I was about to create a new issue about this library detecting generics as a collections. It turns out there is an issue on this already: https://github.com/phpDocumentor/TypeResolver/issues/180

Longer story: this library uses phpDocumentor/TypeResolver, which is limited to PSR-5 (which doesn't recognize generics).

Side note: I'm working on my own type-resolver (based on phpstan/phpdoc-parser) to fill these gaps: https://github.com/uuf6429/phpstan-phpdoc-type-resolver

jaapio commented 2 months ago

Well, this library has served the php world for many years, we always tried to be backward compatible and didn't change our api in all those years.

It's completely understandable that you decided to develop a newer modern version. As you could have seen I wanted to add generics in v2. But as I have limited time, I didn't get that far yet.

I must say that the wording in your readme felt a bit offending. telling that this project doesn't fit - your needs is ok. But please choose some different words.

uuf6429 commented 2 months ago

I must say that the wording in your readme felt a bit offending. telling that this project doesn't fit - your needs is ok. But please choose some different words.

Will do, but just to be clear: PSR-5 and PSR-19 completely left out generics, the phpstan phpdoc parser library (which this library builds on top of) supports generics perfectly, BUT phpdocumentor in general doesn't. Meanwhile, people are coming here because they're confused about what they're seeing. I've spent a lot of my free time building on top of phpdocumentor/reflection-docblock (after I abandoned my earlier project), only to find that it won't work and I need to go back to the earlier project...so yeah, quite frustrating - especially when one would expect that phpDocumentor (or PHP-FIG) to be "the authority" on this matter.

jaapio commented 2 months ago

Generics are post PSR-5, and that's still in draft. Static analyzers are way more advanced on some parts that phpDocumentor could serve. As this project focuses on the information you get from 1 source file. So does reflection-docblock.

That's a limitation, that makes it harder to support generics as that requires information from multiple files. The reason to switch form our own parser to phpstan wasn't to support generics, it was to support callables and other more advanced type definitions.

I do get your confusion.

ahjdev commented 2 months ago

Generics are post PSR-5, and that's still in draft. Static analyzers are way more advanced on some parts that phpDocumentor could serve. As this project focuses on the information you get from 1 source file. So does reflection-docblock.

That's a limitation, that makes it harder to support generics as that requires information from multiple files. The reason to switch form our own parser to phpstan wasn't to support generics, it was to support callables and other more advanced type definitions.

I do get your confusion.

Well as far I know type resolver has Expression, Iterable_ classes but the problem is about to detect tags right? I can open a pr and add them

jaapio commented 2 months ago

Any contribution is welcome, I would be happy to guide any contributor to implement new features. I don't know directly what is needed to implement generics. But I think we would need to introduce something that reflects the structure of a generic.

If we need to do breaking changes for this that's also fine with me. After so many years we can do this. Especially when adding so many important new types.

Please keep in mind that this library is just about types and not about tags. So you might miss some information. If that's the case we might want to go downstream to reflection-docblock.

Maybe start with some draft pr, so I can help you shaping the implementation. Thanks in advance 🙏

uuf6429 commented 2 months ago

Just a small note, the confusion came about from a problematic design decision (in my opinion) in phpDocumentor/TypeResolver: to an outsider, the objective of that library seemed to be to resolve types, i.e. turning Something<Else> into \Some\Project\Something<\Some\Project\Else> - same classes, but fully-qualified names. Instead, it went one step further and reformed (and merged) the original phpstan classes into something specific to phpDocumentor (and strangely enough, picking something from PSR-5 which was later removed, probably because of historical reasons).

The part where we worry about the impact of generics to phpDocumentor should have been in phpDocumentor itself, not at the type-resolver or the reflection-docblock level.

I'm not saying this to complain or argue, I'm hoping that it might be helpful when thinking how this can go forward.

jaapio commented 2 months ago

I'm not sure I fully got your point about type resolving. But let's go back where this all started.

Since the beginning phpDocumentor itself was the only tool around doing something with types in php. Yes we had array and object in method headers but that's about it. So there was a need from a documentation perspective to notate types. Mostly scalar types which are very easy to resolve in the context of a single docblock. Because they are content independent. Back in those days we didn't have namespaces. Also classes were fairly easy.

When namespaces were introduced we introduced the context on type resolving and that's when the original purpose started to fade away. More and more types were added and we never took the time to rethink the way this library works. Because it wasn't really needed. Also the most modern code bases could simply work with the current design. If we leave out types that require more context.

When static analyzers entered the stage we needed to implement more and more types. While we were holding back the introduction of new types in PSR-5 to just standardize the way this was working for years. And some design decisions are also limiting the correct resolvement of types. For example this library would have blocked you from creating a class named Iteratable. Today this is a keyword, but it wasn't before. We would resolve this incorrectly as we would see this as a predefined type..

For static analyzers or tools like phpDocumentor resolving types over multiple files it is not an issue. Those tools will read your whole codebase and do their trick. While this library has focused on a single docblock. Not even a whole file.

Then we have the story of a very widely used library. This project has more than 560 million downloads, according to packagist. Used in major tools and frameworks, which is huge. I must admit that the impact of those numbers were also holding me back from large refactoring. Rethinking the way this library works does impact a lot of people. I do feel the responsibility to keep others from breaking their projects. One the reasons for example I reintroduced php 7.3 support. There's a certain pressure making changes to this project. And in all honesty, I'm not always able to handle that just on my own.

I have been playing with the thought of abandoning this project because it might not cover the modern needs of a project like phpDocumentor anymore. I see that more and more people are coming in to talk about modernizing the project. Because it doesn't cover all we need anymore. While projects with more resources are steaming forward very hard. I cannot keep up with the speed of those projects.

While writing this I might get your point. I would not explain it like a bad design. But more of a project that served the world for a long time and should be rethought. And maybe the time is there to start from scratch.

Reason to translate the output of phpstan's parser is because this project is so widely used. It allowed me to add support for more complex types like callables and array shapes. It was either that or write a new parser myself. I think the latter would have been a waste of time, phpstan does the job. And now we just provide a layer to keep the existing integrations working.

If I would have designed phpDocumentor today I would still abstract the ast from phpstan into more descriptive objects. I think phpstan is doing a great job in parsing, but the actual AST isn't very usable by other tools. As it requires a lot of interpretation. I prefer types over string definitions. That's why we have all those type classes in this project.

I hope this very long answer helps to understand the scope of where we are right now. please let me know when you have any questions or concerns about how to continue.

uuf6429 commented 2 months ago

OK, I understand the historical context better - I'm also seeing that phpdocumentor/type-resolver actually predates the integration with phpstan/phpdoc-parser (it was introduced later here). In that sense, there wasn't even a chance to design it differently.

That's why we have all those type classes in this project.

For the record, that did make it easier to work with :)

From my (an outsider's) perspective, these libraries look quite reusable and a time saver, and to an extent they really are, but I should have also read the fine print - that's on me.

Oh, and thanks for taking the time to explain it in detail.