phpDocumentor / ReflectionDocBlock

MIT License
9.35k stars 119 forks source link

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

Closed williamdes closed 1 year ago

williamdes commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

jaapio commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 1 year ago

Fixed in https://github.com/phpDocumentor/ReflectionDocBlock/pull/343