Support PHPDoc Types in @param @var @return annotations

I really feel this should be allowed and supported. Just ran into this in #3321718: Inline documentation for `EntityResource::loadEntitiesWithAccess()` is outdated/incorrect.

+1 from me.

Coder issue: #3253472: Support advanced PHPStan data types (general, arrays)

Utilising @phpstan- annotations such as @phpstan-return or @phpstan-param is one way to get around this kind of thing.

Coming here from #3348310: Adopt the phpdoc standard for documenting collection (eg Iterator) key and value types, which I had opened without finding this one (which is dealing with a superset of the problem I was worried about).

There was nothing but support in the other issue. I don't know how much more community involvement we need before we can declare this can/should happen. 😅

Thanks,
-Derek

Per #coding-standards meeting in Slack, we need the issue summary here to include the proposed edits to the official documentation wiki pages. I wasn't sure if it should go here:

In the Parameter and return typehinting section of the PHP coding standards document, under the Notes about specific data types heading.

or: In the Indicating data types in documentation section of "API documentation and comment standards".

Going for the 2nd choice for now, since this is about the phpdoc comments, not the function signatures themselves. Took an initial stab at the proposed documentation changes. Edits / improvements welcome!

Thanks,
-Derek

Documentation looks solid. I don't have any improvements to add

Slight edits to the proposed doc changes for (I hope) more clarity.

As I got it was fixed in 8.3.18 https://git.drupalcode.org/project/coder/-/commit/4ee6a357ba12157d7ea0ef...

Filed upgrade issue as no sniffers are broken #3358739: Update coder to 8.3.18

RTBC’ed, thanks!

Sorry I wasn't clear in #10. I meant that I RTBC'ed the blocker #3358739: Update coder to 8.3.18, not that this issue is necessarily RTBC. I've written most of the proposed changes for this issue, so I don't feel eligible to RTBC this.

Adding current/proposed text to the IS.

After seeing the changes in context I have some suggestions.

1. Having three examples for the Iterator seems unnecessary. I think that the last one is sufficient as long as there is a comment in syntax examples. Such as,

For collections (e.g. arrays, \Iterator, etc) the type for the value and the type for the key may be defined. The form is <code>\Iterator<type for the key, type for the value> If both the key and the value are specified there is a space after the comma.

And then the above can replace, "Collections (e.g. arrays, \Iterator, etc) can specify the type for the values and optionally the keys in the collection."

2. It seems to me that the first sentence of the 'syntax notes' should be changed to include PHPDoc types, with a link.
Data types can be primitive types (int, string, etc.), complex PHP built-in types (array, object, resource), or PHP classes.

3. Shouldn't this, "Instead of only writing @return \Iterator it is preferable to document what kind of iterator is being returned. ..." be in the section about @return?

Setting to needs work for feedback on the above.

I wonder if we could maybe outsource what is allowed to a different source. For example by stating that we allow any declaration understood by PHPStan.

I see a few risks in the current proposed texts:

For an array where the values are all of one particular class/interface type follow the class name by [].
Indicate arrays of built-in PHP types by following the type name by [] (example: int[], string[], etc.).

When handling array-like structures it can be important to differentiate between whether something is a contiguous list, or whether something is more akin to a map. Especially when getting to PHPStan level 5 and up.

For example as of PHPStan 1.9: https://phpstan.org/r/00eeff08-75c2-4a2d-a661-a1c503c85c28

Similarly we should encourage people to use array<key-type, value-type> in case the key type matters (for example because it will be manipulated rather than just preserved).

Collections (e.g. arrays, \Iterator, etc) can specify the type for the values and optionally the keys in the collection. Instead of only writing @return \Iterator it is preferable to document what kind of iterator is being returned. For example, if the keys are strings and the values are Route objects, the documentation should specify: \Iterator. If both the key and the value are known, use a space after the comma between the two types.

Collections are mostly a specific usage of generics. We don't currently use Generics (a lot) in Drupal, but there are a lot of places they could be useful (for example in our entity storage layer). I'm worried that by purely focusing on Iterators here we might make people think that generics are disallowed (whereas I think they should be encouraged).

https://phpstan.org/blog/generics-in-php-using-phpdocs

Very happy to widen the scope here. I regularly get scope decisions wrong. was trying to scratch a specific itch in other code and wanted to improve array docs.

Feel free to edit the proposed resolution to make it more general.

My only concern is not making the standards too verbose. So outsourcing is appealing.

However, if we outsource the docs to phpstan, can we get coder to rely on phpstan to enforce it? Otherwise, we risk documenting a standard that our own tools don’t necessarily support.

However, if we outsource the docs to phpstan, can we get coder to rely on phpstan to enforce it?

This is a good question - I think we would need a phpcs rule that enforces the phpstan rule? It would have to be kept in sync, but it wouldn't be Drupal-specific.

Discussed this briefly with @KingDutch, phpstan level 6 does enforce which types are allowed, so as long as phpcs enforces format and ignores the content of the types, we should be fine here from that point of view.