Making WordPress.org

Opened 3 years ago

Closed 7 months ago

#6433 closed defect (bug) (duplicate)

Issue with numbered variables names in devhub’s docblock parser

Reported by: audrasjb's profile audrasjb Owned by:
Milestone: Priority: normal
Component: Developer Hub Keywords: needs-patch
Cc:

Description

While reviewing this PR, I found that numbered variable names in docblocks are not well interpreted by the docblock parser.

For example, take the parse_db_host() function:

Here is the docblock:

	/**
	 * Parses the DB_HOST setting to interpret it for mysqli_real_connect().
	 *
	 * mysqli_real_connect() doesn't support the host param including a port or socket
	 * like mysql_connect() does. This duplicates how mysql_connect() detects a port
	 * and/or socket file.
	 *
	 * @since 4.9.0
	 *
	 * @param string $host The DB_HOST setting to parse.
	 * @return array|false {
	 *     Array containing the host, the port, the socket and
	 *     whether it is an IPv6 address, in that order.
	 *     False if the host couldn't be parsed.
	 *
	 *     @type string      $0 Host name.
	 *     @type string|null $1 Port.
	 *     @type string|null $2 Socket.
	 *     @type bool        $3 Whether it is an IPv6 address.
	 * }
	 */
	public function parse_db_host( $host ) {

Here is the result in DevHub. The first parameter name, $0, is not rendered.

Same goes with other docblocks, like for example wp_maybe_grant_site_health_caps().

Change History (3)

#1 @dd32
3 years ago

This sounds like a stray empty() when isset() would be appropriate - The arguments are stored as plain-text and processed on display, not in a structured format, so it seems likely to have been missed.

I wonder if this was caught during https://make.wordpress.org/meta/2022/07/01/exploration-improving-devhub/ / https://github.com/wordpress/wporg-developer (If someone wants to try fixing this, you'll find a locally runable devhub in that repo, with an importer which you should be able to edit)

This ticket was mentioned in Slack in #meta by dd32. View the logs.


2 years ago

#3 @coffee2code
7 months ago

  • Resolution set to duplicate
  • Status changed from new to closed

Thanks for the report and additional examples! This is a duplicate of #6371, which I'm about to fix.

Note: See TracTickets for help on using tickets.