ReflectionDocBlock icon indicating copy to clipboard operation
ReflectionDocBlock copied to clipboard

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

Open williamdes opened this issue 4 years ago • 6 comments

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 avatar Apr 06 '21 20:04 williamdes

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 avatar Apr 06 '21 21:04 williamdes

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

williamdes avatar Apr 06 '21 21:04 williamdes

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?

jaapio avatar Sep 17 '21 07:09 jaapio

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.

ondrejmirtes avatar Sep 17 '21 08:09 ondrejmirtes

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 avatar Sep 17 '21 08:09 jaapio

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 avatar Sep 27 '21 20:09 jaapio

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

jaapio avatar Nov 18 '22 09:11 jaapio