Missing @return in Doc comment

[subrataemfluence]

File name: wp-admin/includes/plugin.php

Function/Methods missing @return in Doc comment

get_plugin_page_hookname and user_can_access_admin_page

I have uploaded a patch.

[desrosj]

@subrataemfluence good catch!

Can we also add function and parameter descriptions, which are also missing? The return value should have a description as well.

Marking as good-first-bug and assigning as to mark as claimed.

Related: #42505.

[subrataemfluence]

Sure! I would love to :) Thank you for assigning me the task.

[subrataemfluence]

@desrosj Please let me know if the patch looks ok!

[audrasjb]

This ticket is triaged in milestone 5.0.3. The patch looks good, small enough and self contained so it would be ok to land in the next minor. Adding the commit keyword to see if it can land in 5.0.3.

[desrosj]

Moving this to 5.1.

One more nit. I think Required. is implied when there is no default value specified for a parameter, so I don't think we need that.

I looked through core and only found a handful of @param tags with Required.. They seem to fall into a few scenarios:

• The parameter has a default value specified.
• It is a required parameter within an array and others are optional (Although the handbook seems to favor marking the optional parameters instead).
• Parameters that are required, but may have scenarios that call for falsey/empty values to be passed.
• Functions where there are a mix of optional and required parameters.

Clarifying appropriate use of Required. in the ​documentation handbook may be helpful.

[subrataemfluence]

Thank you @desrosj. I am reading the handbook.

[subrataemfluence]

@desrosj, None of $plugin_page and $parent_page arguments has a default value set.

Now since get_admin_page_parent function argument has a default value (empty string) set for $parent, I think adding a default value (empty string) for $parent_page might worth. By doing this we can keep this argument optional.

As far as $plugin_page argument is concerned, it has no default value set, so to my understanding, Required should be used for this. In other words, again, to me understanding, argument doesn't have a default value specified should probably be declared as Required.

I would suggest the following modification in Doc block and argument list:

<?php
/**
 *
 * @global array $admin_page_hooks
 * @param string $plugin_page. Name of the page generated by plugin. Required.
 * @param string $parent_page. Name of the parent page of $plugin_page.
 */

function get_plugin_page_hookname( $plugin_page, $parent_page = '' ) {
   ...
   $parent = get_admin_page_parent( $parent_page );
   ...
}

Please let me know if it makes sense!

[SergeyBiryukov]

In 45085:

Docs: Improve documentation for get_plugin_page_hook() and get_plugin_page_hookname().

Props subrataemfluence, desrosj.
Fixes #44224.