DocBlocks for filters inside a conditional expression are not imported

[keesiemeijer]

DocBlocks outside a conditional are not imported if there are other filters (with DocBlocks) within the conditional.

Example 1

In this example DocBlock 2 is not imported for the filter in_conditional_expression

/**
 * DocBlock 1
 */
function test_docblocks() {
	/**
	 * DocBlock 2
	 */
	if ( apply_filters( 'in_conditional_expression', true ) ) {
		/**
		 * DocBlock 3
		 */
		$value = apply_filters( 'in_conditional', true );
	}
}

Example 2

In this example the (wrong) DocBlock 2 is imported for the filter 'in_conditional`.

/**
 * DocBlock 1
 */
function test_docblocks() {
	/**
	 * DocBlock 2
	 */
	if ( apply_filters( 'in_conditional_expression', true ) ) {

		$value = apply_filters( 'in_conditional', true );
	}
}

The wp_save_post_revision_check_for_changes filter doesn't get its DocBlock imported because of this bug.

See this filter in trac

[JDGrimes]

I've created in #186, which addresses the first example (2 documented filters, first doc not picked up). The second example (first filter documented, second not, but docs imported for second one) is really more difficult to handle, and I'm not sure that it is worth the work. If it is really needed, I suppose that there may be some way to do it, but it is not straightforward, as far as I can see. I know that this may seem odd, but it works this way because the second filter is actually parsed first, then the first one.

[keesiemeijer]

I've tested your patch on the wp-includes folder to see if there would be any differences between the current way docblocks are imported and with the patch. The only differences found are for the filters wp_save_post_revision_check_for_changes and header_video_settings. The first now gets imported correctly.

The filter header_video_settings doesn't have a dockblock and isn't imported currently. With your patch it's is imported with the wrong dockblock as you said above.

As all hooks should be documented this shouldn't be a problem. I've already created a ticket for this filter https://core.trac.wordpress.org/attachment/ticket/39130/39130.12.patch

[keesiemeijer]

I've now tested your patch on trunk and it correctly adds the DocBlocks for these hooks:

• custom_menu_order in /wp-admin/includes/menu.php
• wp_save_post_revision_check_for_changes in /wp-includes/revision.php

These undocumented hooks get a wrong DocBlock assigned:

• load-page-new.php in /wp-admin/admin.php (4 undocumented actions below it)
• header_video_settings in /wp-includes/theme.php

All other DocBlocks for hooks (and functions, classes, methods) get imported the same as without the patch. The issue with the undocumented hooks should be fixed by adding the 6 missing DocBlocks to the hooks in WP itself.

[keesiemeijer]

DocBlocks for elseif statements are not picked up (with or without #186). The method $node->getDocComment() doesn't return a DocBlock for Stmt_ElseIf type nodes, while it does for Stmt_If nodes.

In this example DocBlock 2 will never be imported for filter in_conditional_expression because it's in a elseif conditional expression.

/**
 * DocBlock 1
 */
function test_docblocks() {

	if ( 1 == 1 ) {

		/**
		 * DocBlock 2
		 */
	} elseif ( apply_filters( 'in_conditional_expression', true ) ) {

	}
}