WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 5 weeks ago

#44253 closed defect (bug) (fixed)

@return missing in doc comment of get_admin_page_parent function

Reported by: subrataemfluence Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Administration Keywords: dev-feedback
Focuses: docs Cc:
PR Number:

Description

@return is missing in the Doc comment of get_admin_page_parent function.

Also I have a question. At the end of the function body we are returning hard coded empty string i.e. return '';

Why don't we return the $parent_file instead? Since we are already returning the value from rest of the if-else blocks and foreach loop the last final statement either will return an empty string or a value assigned to $parent_file variable.

<?php
if( empty( $parent_file ) ) {
   $parent_file = '';
}

return $parent_file;

Attachments (3)

44253.diff (705 bytes) - added by subrataemfluence 20 months ago.
44253-2.diff (1.3 KB) - added by subrataemfluence 20 months ago.
Revised proposed patch.
44253-3.diff (1.3 KB) - added by subrataemfluence 20 months ago.

Download all attachments as: .zip

Change History (9)

#1 @subrataemfluence
20 months ago

Since get_admin_page_title function is also missing the @return in Doc comment I am uploading a revised patch which combines both the functions.

@subrataemfluence
20 months ago

Revised proposed patch.

#2 @kennithnichol
20 months ago

Given PHPs type coercion, this really could just return $parent_file; without any check for empty, the caller should be responsible for checking for empty().

Returning $parent_file could have unintended side-effects. The check for empty on $parent_file effects a global variable and does not necessarily have any correlation to the return statement.

Last edited 20 months ago by kennithnichol (previous) (diff)

#3 @subrataemfluence
20 months ago

@kennithnichol I understand the concern! Thank you for explaining and correcting me.

#4 @pento
12 months ago

  • Version trunk deleted

#5 @SergeyBiryukov
5 weeks ago

  • Component changed from Plugins to Administration
  • Focuses docs added; coding-standards removed
  • Milestone changed from Awaiting Review to 5.4
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#6 @SergeyBiryukov
5 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 47006:

Docs: Improve documentation for admin menu functions:

  • get_admin_page_parent()
  • get_admin_page_title()
  • get_plugin_page_hook()
  • get_plugin_page_hookname()
  • user_can_access_admin_page()

Add missing descriptions and @since tags.

Props subrataemfluence, kennithnichol, stevenlinx, SergeyBiryukov.
Fixes #44253, #49067.

Note: See TracTickets for help on using tickets.