phpDocumentor / fig-standards

Standards either proposed or approved by the Framework Interop Group
http://www.php-fig.org/
Other
361 stars 85 forks source link

Nested type in @return #133

Closed teohhanhui closed 5 years ago

teohhanhui commented 8 years ago

Please allow nested type in @return for documenting associative array.

mindplay-dk commented 8 years ago

"nested type"?

zonuexe commented 8 years ago

I don't know his thinking, but I have also have an opinion.

PHP's array can representation as tuple. (I wrote an article about list in Japanese.)

If simplify it, It is the case, such as:

<?php

class VendingMachine
{
    /**
     * @return array (string $name, float $price, int $stock)
     */
    function get($id) :array
    {
        // ["Coca-Cola", 1.5, 12]
        return [$name, $price, $stock];
    }
}

$vendor = new VendingMachine();
list($name, $price, $stock) = $vendor->get(10);

We might should declare Item class, but please forget that. The problem is that the description method to standardize the data structure is not included in the current proposal.

I think "nested type" mean an example, such as:

<?php

class VendingMachine
{
    /**
     * @return array ('name' => string $name, 'price' => float $price, 'stock' => int $stock)[]
     */
    function getAll() :array
    {
        return [
            [
                'name'  => $name1,
                'price' => $price1,
                'stock' => $stock1
            ],
            [
                'name'  => $name2,
                'price' => $price2,
                'stock' => $stock2
            ],
            // ...
        ];
    }
}
mindplay-dk commented 8 years ago

We might should declare Item class, but please forget that.

Yes, you should - an array with known keys is merely a pseudo-object with no class.

This is a complex feature requiring complex code to implement structural sub-typing in type-checkers and documentation generators.

Support for this feature is going to be quite hard to implement, so the argument needs to be a lot better than supporting an anti-pattern.

teohhanhui commented 8 years ago

What I mean is exactly the same as what is currently known as "Inline PHPDoc" in the draft. Basically the same as #103 the same as for @param but for @return.

The use case I have in mind is returning an associative array, but yes, returning a numeric-indexed array for use with list is a common pattern despite its downsides. PHP 7.1 will allow using associative arrays with list so I think this is only going to become more commonplace.

ashnazg commented 5 years ago

Given that inline PHPDoc is available for param tag, I can't say I see an argument to not also allow it for return tag.

Ping @ondrejmirtes @muglug @neuro159 @mindplay-dk @GaryJones @mvriel @jaapio for opinions.

muglug commented 5 years ago

Psalm and Phan support a syntax using curly braces and no space after array like so:

/**
 * @param array{bar: array{0: int, 1: string}} $arr
 * @return array{int, string} - equivalent to array{0: int, 1: string}
 */
function foo(array $arr) : array {
  return $arr['bar'];
}

foo(['bar' => [5, "hello"]]); // passes
foo(['bat' => [5, "hello"]]); // fails
foo(['bar' => ["hello", 5]]); // fails

See here: https://getpsalm.org/r/edf8391576 (edit: fixed link)

muglug commented 5 years ago

I use this in combination with vendor-specific type aliasing to produce horrifically large array types: https://github.com/vimeo/psalm/blob/99c9be34e18f6025b7174aec4375776cd498708a/src/Psalm/Codebase/Analyzer.php#L16-L40

TysonAndre commented 5 years ago

@muglug - https://getpsalm.org/r/67d8cfddef seems like it might be the wrong link.

Aside: Phan doesn't support array{int, string} yet, but does support array{0: int, 1:string}

Also, if nested array shapes do end up in the standard, it may be useful to have some form of single-line annotation, capable of representing information equivalent to the below:

  1. in @property and @method - e.g. @property array{0:int,1:string} $somePair
  2. Annotations such as generic/template types (e.g. array<string,array{key:string}> for values such as [$strVal => ['key' => $otherStrVal]])

The multi-line syntax I've seen in the draft doesn't mention how those would work.

neuro159 commented 5 years ago

This or effectively this is much required feature according to PhpStorm issue tracker and feedback we collected. IMO its CRUCIAL to find a way for people to express this. HOW EAXCTLY - open question.

https://youtrack.jetbrains.com/issue/WI-6558 support for more specific array PHPDoc annotations 118 votes

https://youtrack.jetbrains.com/issue/WI-3423 Add ability to explicitly define array keys for array intellisense autocomplete 50 votes

muglug commented 5 years ago

All I can say is we’ve been using the custom object-like array syntax (that is, array{x: T1, y:T2}) at Vimeo and it hasn’t caused issues so far. It supports multiline annotations e.g https://getpsalm.org/r/641d7950e7 and makes sense to anyone who’s used flow/typescript as it uses the same essential syntax.

We could replace braces for regular brackets, but brackets feel more function-y. I don’t mind terribly, though.

Alternatively we could copy Hack’s syntax for shapes directly (but maybe using array keyword instead): https://docs.hhvm.com/hack/shapes/introduction

ashnazg commented 5 years ago

Everyone please have a look at the "Inline PHPDoc" example in the PSR-19 param tag section.

Will this format suffice? If so, then all we have to do is specific that such "Inline PHPDoc" usage is allowed for the @return tag syntax.

muglug commented 5 years ago

Everyone please have a look at the "Inline PHPDoc" example

For reference:

/**
 * Initializes this class with the given options.
 *
 * @param array $options {
 *     @var bool   $required Whether this element is required
 *     @var string $label    The display name for this element
 * }
 */
public function __construct(array $options = array())

Personally not a massive fan of that @var nesting.

Edit: but it wouldn't be too hard to have Psalm support that too.

GaryJones commented 5 years ago

Personally not a massive fan of that @var nesting.

One alternative would be @type, as WordPress does.

ashnazg commented 5 years ago

One of the earliest changes Mike and I discussed way back when was to deprecate @var and replace it with a better named @type tag.

That did not go over well.......................

Since it's the same job being done here (denoting type), I'd prefer to keep using @var for consistency.

My apologies to WordPress if our original @type idea was what got them doing it.

GaryJones commented 5 years ago

My apologies to WordPress if our original @type idea was what got them doing it.

Since it was likely me that pushed them towards it after seeing it added to the draft PSR-5, apology accepted :-)

Incidentally, they only use it for nested DocBlocks, and not for standard vars. That's a deviation from what was documented in the draft PSR-5, which maybe has some value, and less of a mental shift than applying it to all instances.

ashnazg commented 5 years ago

@teohhanhui , if you wish to pursue this further, please bring this up as a new thread on the FIG mailing list for discussion -- https://groups.google.com/forum/#!forum/php-fig