From 572593500700ee304ce7bb704ce5e3e3d0746e47 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Fri, 30 Dec 2016 16:45:37 -0500 Subject: [PATCH 1/2] Pick up docs for filters in condition expressions PHPParser loops over the body of a conditional statement (the part between the curly brackets), before looping over each node in the expression. Because we were only saving the last docblock that wasn't associated with a documentable element, if there was also a documented filter within the condition body, the docblock for that filter would be picked up, and the docblock for the first filter in the condition expression would be discarded. So by the time we got to the node for the filter in the condition expression, there was no docblock saved to associate with it. This is now fixed by keeping a stack of stray docblocks for non-documentable elements, so that instead of the docblock for the first filter being discarded it is just pushed down the stack. The docblock for the second filter within the condition block will be pushed on top of it, and then popped off once it has been used. So when we come around to actually looping over the node for the filter in the conditional expression, its docblock will be at the tip of the stack once again, and can be associated with the filter as expected. See #185 --- lib/class-file-reflector.php | 16 ++++++++++------ tests/phpunit/tests/export/docblocks.inc | 10 ++++++++++ tests/phpunit/tests/export/docblocks.php | 10 ++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/class-file-reflector.php b/lib/class-file-reflector.php index 5e1b59f..0292b16 100644 --- a/lib/class-file-reflector.php +++ b/lib/class-file-reflector.php @@ -38,11 +38,11 @@ class File_Reflector extends FileReflector { protected $location = array(); /** - * Last DocBlock associated with a non-documentable element. + * Last DocBlock(s) associated with a non-documentable element(s). * - * @var \PHPParser_Comment_Doc + * @var \PHPParser_Comment_Doc[] */ - protected $last_doc = null; + protected $last_doc = array(); /** * Add hooks to the queue and update the node stack when we enter a node. @@ -83,8 +83,8 @@ public function enterNode( \PHPParser_Node $node ) { if ( $this->isFilter( $node ) ) { if ( $this->last_doc && ! $node->getDocComment() ) { - $node->setAttribute( 'comments', array( $this->last_doc ) ); - $this->last_doc = null; + $last_doc = array_pop( $this->last_doc ); + $node->setAttribute( 'comments', array( $last_doc ) ); } $hook = new Hook_Reflector( $node, $this->context ); @@ -127,7 +127,11 @@ public function enterNode( \PHPParser_Node $node ) { // associated with a named element, and so aren't really from a non- // documentable element after all. if ( ! $this->isNodeDocumentable( $node ) && 'Name' !== $node->getType() && ( $docblock = $node->getDocComment() ) ) { - $this->last_doc = $docblock; + // The same docblock can be associated with multiple non-documentable + // elements, so we have to take care not to save the same docblock twice. + if ( ! $this->last_doc || $docblock->getText() !== end( $this->last_doc )->getText() ) { + $this->last_doc[] = $docblock; + } } } diff --git a/tests/phpunit/tests/export/docblocks.inc b/tests/phpunit/tests/export/docblocks.inc index 05686f5..eac8bc9 100644 --- a/tests/phpunit/tests/export/docblocks.inc +++ b/tests/phpunit/tests/export/docblocks.inc @@ -88,3 +88,13 @@ do_action( 'undocumented_hook' ); * A reference array action. */ do_action_ref_array( 'test_ref_array_action', array( &$var ) ); + +/** + * A filter in a conditional expression. + */ +if ( apply_filters( 'in_conditional_expression', true ) ) { + /** + * A filter in a conditional. + */ + $value = apply_filters( 'in_conditional', true ); +} diff --git a/tests/phpunit/tests/export/docblocks.php b/tests/phpunit/tests/export/docblocks.php index 0d49b15..cc4b0f8 100644 --- a/tests/phpunit/tests/export/docblocks.php +++ b/tests/phpunit/tests/export/docblocks.php @@ -59,6 +59,16 @@ public function test_hook_docblocks() { 'test_ref_array_filter' , array( 'description' => 'A reference array filter.' ) ); + + $this->assertHookHasDocs( + 'in_conditional_expression' + , array( 'description' => 'A filter in a conditional expression.' ) + ); + + $this->assertHookHasDocs( + 'in_conditional' + , array( 'description' => 'A filter in a conditional.' ) + ); } /** From 0d752c863345bd76438af522e5161c4eba4a0de0 Mon Sep 17 00:00:00 2001 From: JDGrimes Date: Fri, 30 Dec 2016 16:46:30 -0500 Subject: [PATCH 2/2] Some fixes for the test docs Discovered while working on #185. --- tests/phpunit/includes/export-testcase.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/phpunit/includes/export-testcase.php b/tests/phpunit/includes/export-testcase.php index b3fe845..70e5eb6 100644 --- a/tests/phpunit/includes/export-testcase.php +++ b/tests/phpunit/includes/export-testcase.php @@ -14,7 +14,7 @@ class Export_UnitTestCase extends \WP_UnitTestCase { /** * The exported data. * - * @var string + * @var array */ protected $export_data; @@ -424,8 +424,8 @@ protected function assertPropertyHasDocs( $class, $property, $docs ) { /** * Assert that a hook has a docblock. * - * @param array $hook The hook name. - * @param array $docs The expected data for the hook's docblock. + * @param string $hook The hook name. + * @param array $docs The expected data for the hook's docblock. */ protected function assertHookHasDocs( $hook, $docs ) { @@ -436,9 +436,9 @@ protected function assertHookHasDocs( $hook, $docs ) { /** * Find the exported data for an entity. * - * @param array $data The data to search in. - * @param string $type The type of entity. - * @param string $entity_name The name of the function. + * @param array $data The data to search in. + * @param string $type The type of entity. + * @param string $entity The name of the entity. * * @return array|false The data for the entity, or false if it couldn't be found. */