WordPress.org

Make WordPress Core

Opened 4 weeks ago

Last modified 4 weeks ago

#44253 new defect (bug)

@return missing in doc comment of get_admin_page_parent function

Reported by: subrataemfluence Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: trunk
Component: Plugins Keywords: dev-feedback
Focuses: coding-standards Cc:

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 4 weeks ago.
44253-2.diff (1.3 KB) - added by subrataemfluence 4 weeks ago.
Revised proposed patch.
44253-3.diff (1.3 KB) - added by subrataemfluence 4 weeks ago.

Download all attachments as: .zip

Change History (6)

#1 @subrataemfluence
4 weeks 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
4 weeks ago

Revised proposed patch.

#2 @kennithnichol
4 weeks 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 4 weeks ago by kennithnichol (previous) (diff)

#3 @subrataemfluence
4 weeks ago

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

Note: See TracTickets for help on using tickets.