Make WordPress Core

Opened 7 years ago

Closed 4 years ago

Last modified 3 years ago

#41756 closed enhancement (fixed)

Document arrays of a known type as such

Reported by: johnbillion's profile johnbillion Owned by: janak007's profile janak007
Milestone: 5.4 Priority: low
Severity: normal Version:
Component: General Keywords:
Focuses: docs Cc:

Description

A parameter that accepts an array (or avariable that contains an array) usually has its type documented with the array keyword:

@param array $args Arguments.

phpDocumentor and IDEs such as PhpStorm support a secondary notation for arrays which specifies the type of the elements that the array contains. Example:

@param string[] $args Arguments.

This form of notation makes it clearer which type the application expects in an array's elements. WordPress core should switch to documenting arrays of known types to this format.

Ref: https://docs.phpdoc.org/references/phpdoc/types.html#arrays


Addendum: phpDocumentor also supports documenting arrays containing mixed types with the following syntax, but this could get messy:

@param (int|string)[] $args Arguments.

Attachments (6)

41756.diff (3.7 KB) - added by johnbillion 7 years ago.
front-end-render-before.png (61.9 KB) - added by GaryJ 6 years ago.
Front end render, before
front-end-render-after.png (61.5 KB) - added by GaryJ 6 years ago.
Front end render, after
wp_set_link_cats.png (88.4 KB) - added by GaryJ 6 years ago.
View of JSON for wp_set_link_cats()
bulk_post_updated_messages-param.png (21.1 KB) - added by GaryJ 6 years ago.
View of JSON for param for bulk_post_updated_messages filter
41756.patch (10.9 KB) - added by janak007 6 years ago.
I have changed doc parameter for array types as johnbillion suggested.

Download all attachments as: .zip

Change History (28)

@johnbillion
7 years ago

#1 @johnbillion
7 years ago

41756.diff is a patch of a handful of occurrences.

#2 @pbiron
7 years ago

I think this change should definitely happen!

Does phpdoc-parser support "typed arrays", or will it need to be modified in conjunction with this, in order for the CodeRef to work correctly?

#3 @GaryJ
7 years ago

The draft PSR-5 (which phpDocumentor generally follows) also allows for an inline PHPDoc for arrays contained mixed type items, or there's mixed[].

So long as the parser can (be made to) understand it, and that popular IDEs understand the formats (or it at least has no detrimental effects), then +1.

#4 @johnbillion
6 years ago

  • Keywords dev-feedback added; 2nd-opinion removed

@GaryJ Are you in a position to test the docs parser with this format for docs? Or do you know someone who is? I've unfortunately not been able to get the docs parser to work at all on my local setup (the generated JSON file is always empty).

@GaryJ
6 years ago

Front end render, before

@GaryJ
6 years ago

Front end render, after

@GaryJ
6 years ago

View of JSON for wp_set_link_cats()

@GaryJ
6 years ago

View of JSON for param for bulk_post_updated_messages filter

#5 @GaryJ
6 years ago

Documenting my process, so others can verify.

  1. Checkout https://github.com/WordPress/phpdoc-parser.git into the plugins directory of a fresh WP install.
  2. Active plugin - need PHP 5.4 or later. Open PRs suggest may be issues with running PHP 7.0.
  3. Pulled down theme: svn checkout https://meta.svn.wordpress.org/sites/trunk/wordpress.org/public_html/wp-content/themes/pub/wporg-developer/ wp-content/themes/wporg-developer
  4. Activated theme, and replaced references to WPORGPATH files with header and footer markup from Twenty Seventeen theme.
  5. Ran wp parser create wp-admin/includes/bookmark.php --user=admin - file chosen as it was one of the files in the patch.
  6. Front end rendered as:

/raw-attachment/ticket/41756/front-end-render-before.png
Note that $link_categories is an (array).

  1. Manually changed the docs for wp_set_link_cats() as per patch.
  2. Front end now rendered as:

/raw-attachment/ticket/41756/front-end-render-after.png
Note that $link_categories is an (int[]).

  1. Realised you wanted the JSON file. Ran wp parser export wp-admin/includes/bookmark.php --user=admin
  2. Ran cat phpdoc.json and relevant JSON section is:
    {
        "name": "param",
        "content": "Array of link category IDs to add the link to.",
        "types": [
            "int[]"
        ],
        "variable": "$link_categories"
    }
    

/raw-attachment/ticket/41756/wp_set_link_cats.png

  1. Repeated for an array in bulk_post_updated_messages filter hook, and got an equivalant result:

/raw-attachment/ticket/41756/bulk_post_updated_messages-param.png

  1. Repeated with @param string|array $body_classes from setup_config_display_header(), but changed it to string|string[]. Resulted in:
{
    "name": "param",
    "content": "",
    "types": [
        "string",
        "string[]"
    ],
    "variable": "$body_classes"
}	

  1. Just for fun, repeated 12. but with string|foobar[]. Resulted in:
    {
        "name": "param",
        "content": "",
        "types": [
            "string",
            "\\foobar[]"
        ],
        "variable": "$body_classes"
    }	
    
  1. Tried the exotic format in the OP, on wp_generate_attachment_metadata filter. Result:
    {
        "name": "param",
        "content": "An array of attachment meta data.",
        "types": [
            "\\(int",
            "\\string)[]"
        ],
        "variable": "$metadata"
    },
    

Conclusion

  • The basic alternative format seems to be accepted without throwing errors.
  • The exotic format (array-expression) seems to be thrown off by the non-recognised / non-valid keywords ("foobar", or exactly "(int" or exactly "string)[]"). More specifically, it seems to be treating them as class names.
Last edited 6 years ago by GaryJ (previous) (diff)

#6 @johnbillion
6 years ago

  • Keywords needs-patch added; dev-feedback removed

Thanks for that, Gary. I think we can go ahead and begin switching over some known parameters.

#7 @johnbillion
6 years ago

  • Keywords good-first-bug added

@janak007
6 years ago

I have changed doc parameter for array types as johnbillion suggested.

#8 @DrewAPicture
6 years ago

  • Owner set to janak007
  • Status changed from new to assigned

Assigning to mark the good-first-bug as "claimed".

#9 @DrewAPicture
6 years ago

  • Keywords has-patch added; needs-patch removed

#10 @johnbillion
6 years ago

In 42679:

Docs: First pass at switching some array parameter documentation to typed notation.

Props janak007

See #41756

#11 @johnbillion
6 years ago

  • Keywords needs-patch added; good-first-bug has-patch removed

Thanks for the patch, @janak007! A few of the instances that you changed were incorrect, so I corrected them or left them out. The changes will appear on the developer.wordpress.org site once WordPress 5.0 is released.

#12 @janak007
6 years ago

@johnbillion Yes, correct them so I can work more on this ticket.

#13 @johnbillion
6 years ago

In 42870:

Docs: Document WP_Roles properties with typed array notation.

Props stevenlinx

Fixes #38732
See #41756

#14 @johnbillion
6 years ago

In 42871:

Docs: Document many more parameters and properties using typed array notation.

See #41756

#15 @johnbillion
6 years ago

In 42872:

Docs: Revert some sneaky debugging code.

See #41756

#16 @johnbillion
6 years ago

In 42875:

Docs: Document more parameters and properties using typed array notation.

See #41756

#17 @johnbillion
6 years ago

In 42876:

Docs: Document more parameters and properties using typed array notation.

See #41756

#18 @johnbillion
6 years ago

In 43177:

Docs: Update and correct various inline documentation.

See #42505, #41756

#19 @johnbillion
4 years ago

In 46596:

Docs: Switch more docs over to typed array notation, plus some fixes.

See #48303, #41756

#20 @johnbillion
4 years ago

  • Keywords ongoing needs-patch removed
  • Milestone changed from Awaiting Review to 5.4
  • Resolution set to fixed
  • Status changed from assigned to closed

Closing this as mostly fixed. Other instances can be fixed in followup tickets or on #48303 etc.

#21 @johnbillion
3 years ago

In 49672:

Docs: Document parameters that accept an array of integers using typed array notation.

While many of these parameters also technically accept an array of numerical strings, they are all ultimately cast to an array of integers. Documenting them as such assists developers in understanding the expected types.

See #51800, #41756

#22 @johnbillion
3 years ago

In 49693:

Docs: Upgrade more parameters in docblocks to used typed array notation.

See #51800, #41756

Note: See TracTickets for help on using tickets.