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

Support for array shape descriptions - Copy changes from jdpedrie/Sami ? #13

Open
williamdes opened this issue Oct 17, 2020 · 13 comments
Open
Assignees
Labels
question Further information is requested

Comments

@williamdes
Copy link
Member

Hi @jdpedrie
You made changes on https://github.com/jdpedrie/Sami and implemented them for https://github.com/googleapis/google-api-php-client

You you mind implementing them here ?

@jdpedrie
Copy link

Hi @williamdes,

Here is the comparison of changes I made. I'd love to help get it added here, but my free time is limited at the moment. I may be able to work on it in the near future, but not right now. Additionally, I doubt my changes would meet the acceptable standards for the library! It is sort of a hack that I made without deep knowledge of Sami.

For anyone curious, here is an example of what my change handles, and here is the output.

@williamdes williamdes added this to the v5.2.0 milestone Nov 12, 2020
@williamdes
Copy link
Member Author

Hi @jdpedrie
I assigned this to 5.2, nothing sure but I could try to get the changes incorporated and make this non PHP standard support optional.
What do you think about that idea ?

@jdpedrie
Copy link

That would be great! Please let me know if I can help with review or testing, and again if I find some time I'll let you know and try to jump back in with more concrete assistance.

@williamdes williamdes self-assigned this Nov 12, 2020
@williamdes williamdes modified the milestones: v5.2.0, v5.3.0 Nov 29, 2020
@williamdes
Copy link
Member Author

williamdes commented Nov 29, 2020

Psalm also has a custom format, this should be implemented too I think

@williamdes
Copy link
Member Author

phpstan/phpstan#2173

@jdpedrie
Copy link

jdpedrie commented Dec 17, 2020

I'm agnostic as to the format supported, except unless I've missed something, these formats don't appear to support array member descriptions, which is a requirement for us. I'd love to provide documentation in a format recognized by static analyzers, so long as we can also provide the descriptions.

@williamdes williamdes modified the milestones: v5.3.0, v5.3.1 Dec 21, 2020
@williamdes williamdes removed this from the v5.3.1 milestone Dec 30, 2020
@williamdes williamdes added the question Further information is requested label Apr 8, 2021
@williamdes
Copy link
Member Author

I'm agnostic as to the format supported, except unless I've missed something, these formats don't appear to support array member descriptions, which is a requirement for us. I'd love to provide documentation in a format recognized by static analyzers, so long as we can also provide the descriptions.

@muglug @ondrejmirtes do you have some suggestions on this idea ?
I am asking because you "created" the array shape formats :)

@williamdes williamdes changed the title Copy changes from jdpedrie/Sami ? Support for array shape descriptions - Copy changes from jdpedrie/Sami ? Apr 8, 2021
@ondrejmirtes
Copy link

Sorry, I don't know what the question is about, can you rephrase it in such a way so I can respond?

@williamdes
Copy link
Member Author

Sorry, I don't know what the question is about, can you rephrase it in such a way so I can respond?

Sorry for that, here is the question:

Actually the world has array shapes for array phpdocs, but no description.
Because Doctum is a tool that would like to provide the best syntax and help people build better phpdocs I would want to discuss what format would be best to document the array shapes (provide descriptions)

The changes @jdpedrie had made where to support this syntax

Example render: https://googleapis.github.io/google-auth-library-php/master/Google/Auth/AccessToken.html#method_verify
Example code: https://github.com/googleapis/google-auth-library-php/blob/master/src/AccessToken.php#L85-L97

     * @param array $options [optional] Configuration options.
     * @param string $options.audience The indended recipient of the token.
     * @param string $options.issuer The intended issuer of the token.
     * @param string $options.cacheKey The cache key of the cached certs. Defaults to
     *        the sha1 of $certsLocation if provided, otherwise is set to
     *        "federated_signon_certs_v3".
     *

So, what do you think would be best for the PHP world @ondrejmirtes ?

The goal is not to support a different syntax for each dev of course, but more to find a good description syntax.

TLDR: we have array shape type syntax, but no way to describe them

@ondrejmirtes
Copy link

If you have such advanced needs, you should really use objects and properties instead of array shapes. Sorry, I'm not interested in extending the syntax beyond what it does now.

@mad-briller
Copy link
Contributor

Would love to see support for array-shapes using the array{} syntax adopted by many major tools! I don't think inventing a new syntax to allow commenting the meaning of array keys is a good idea either, as it results in fragmentation when really we want to strive for consistency and interoperability throughout PHP tooling.

PHPStan has a great phpdoc-parser library that is separate from the core of the analyzer that could make keeping up with conventions simpler in the long run, whilst also minimizing differences between tools that are consuming the same data source.

It would probably take more work upfront to move to using a separate library for the parsing, but it would be simpler to keep up-to-date and consistent. Perhaps a goal for the next major release?

Anyway, thanks for your continued work on doctum, i look forwards to the addition of support for array shapes, however you decide to make it work :)

@williamdes
Copy link
Member Author

williamdes commented Nov 30, 2021

Well, I already tried to migrate to this cool lib but phpstan/phpdoc-parser#72 was blocking me and it seems like @ondrejmirtes would not want to alter the lib because it is an internal tool, I understand the point.

I would love to see this syntax supported soon, just need to find the right lib to make it work

@williamdes
Copy link
Member Author

This just got merged: phpDocumentor/ReflectionDocBlock#343

So array shapes will be official in Doctum soon (normally).
@jdpedrie except array shape descriptions what is missing to your fork of Sami ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Development

No branches or pull requests

4 participants