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

DocBlock\Tags\Method does not work nicely with an old php array syntax #271

Closed
williamdes opened this issue Apr 6, 2021 · 7 comments
Closed
Assignees

Comments

@williamdes
Copy link
Contributor

Here is my solution: https://regex101.com/r/nySDm6/1

                (?:
-                    \(([^\)]*)\)
+                    \((.*(?=\)))\)
                )?

Test data: static array[] ISO8859_1(array $d = [], string $s="", $f=array()) foo bar baz

@williamdes
Copy link
Contributor Author

Also all the logic for variadics and references would be nice on the @method class
Like in https://github.com/phpDocumentor/ReflectionDocBlock/blob/5.2.2/src/DocBlock/Tags/Param.php

@williamdes
Copy link
Contributor Author

This is problematic because Doctum needs to know if it is a variadic to display correctly @methods

@jaapio
Copy link
Member

jaapio commented Sep 17, 2021

Hi,

Thanks for reporting this. I would like start with the fact that right now the @method tag doesn't support defaults. So I'm not sure how you are using this in doctum.

I see the value of aligning the @method tag more with the normal method signatures.

@ondrejmirtes, @muglug, @mvriel what do you think of this? I think psalm and phpstan would benefit from this as well, how-ever you are not using this library to process docblocks as far as I know. Maybe phpstan does via better-reflection?
The suggestion is to align the @method tag with normal method signatures, so it can support all notations of native php methods.

My opinion on this is that if you need more than a simple notation, a native method would fit the purpose better. Since documenting the parameters is also impossible using the @method. And I don't want to come up with a notation that supports documenting method arguments. However, this could also open doors to support callable and Closure in a more decent way?

//cc @ashnazg needs direction from PSR-19?

@ondrejmirtes
Copy link

Hi,

Maybe phpstan does via better-reflection

It doesn't, PHPStan uses its own https://github.com/phpstan/phpdoc-parser.

And it already supports default parameter values in @method as you can see in the parser implementation: https://github.com/phpstan/phpdoc-parser/blob/d639142cf83e91a07b4862217a6f8aa65ee10d08/src/Parser/PhpDocParser.php#L313-L340

If you're proposing some specific new syntax to solve a problem, I'd need to see how it looks to form an opinion about it.

@jaapio
Copy link
Member

jaapio commented Sep 17, 2021

Thanks @ondrejmirtes for your quick response.

I don't want to introduce something new. Especially not when PHPStan already has support for the requested changes here. I think we have both way too much work to care about an edge case like this. In the years this library exists, nobody ever complained about the way @method works.

If a full new notation of @method is needed, I think we need to involve more people because the current notation is quite old and widely used in the php-ecosystem. Just like this library.

@jaapio
Copy link
Member

jaapio commented Sep 27, 2021

I started an effort to get this thing done #304

I'm not sure yet if I will keep it this way, nothing changed in the notation of @method it will be just like the implementation of other tooling.

@jaapio
Copy link
Member

jaapio commented Nov 18, 2022

Fixed in #343

@jaapio jaapio closed this as completed Nov 18, 2022
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

3 participants