diff --git a/phpBB/develop/event_exporter.php b/phpBB/develop/event_exporter.php index 644063df2b..c17e714764 100644 --- a/phpBB/develop/event_exporter.php +++ b/phpBB/develop/event_exporter.php @@ -117,31 +117,10 @@ class event_exporter $event_line = $i; $event_name = $this->get_trigger_event_name($file, $lines[$event_line]); - // Find $vars array + // Find variables of the event $arguments = $this->get_vars_from_array($file, $event_name, $lines, $event_line); - - // Validate $vars array with @var - $find_vars_line = 3; - $doc_vars = array(); - while (strpos(trim($lines[$event_line - $find_vars_line]), '*') === 0) - { - $var_line = trim($lines[$event_line - $find_vars_line]); - $var_line = preg_replace('!\s+!', ' ', $var_line); - if (strpos($var_line, '* @var ') === 0) - { - $doc_line = explode(' ', $var_line); - if (isset($doc_line[3])) - { - $doc_vars[] = $doc_line[3]; - } - } - $find_vars_line++; - } - - if (sizeof($arguments) !== sizeof($doc_vars) && array_intersect($arguments, $doc_vars)) - { - throw new LogicException('$vars array does not match the list of @var tags for event "' . $event_name . '" in file "' . $file . '"'); - } + $doc_vars = $this->get_vars_from_docblock($file, $event_name, $lines, $event_line); + $this->validate_vars_docblock_array($file, $event_name, $arguments, $doc_vars); } else { @@ -277,9 +256,73 @@ class event_exporter } } + sort($vars_array); return $vars_array; } + /** + * Find the $vars array + * + * @param string $file + * @param string $event_name + * @param array $lines + * @param int $event_line Index of the event call in $lines + * @return array List of variables + */ + public function get_vars_from_docblock($file, $event_name, $lines, $event_line) + { + $doc_vars = array(); + $current_doc_line = 1; + $found_comment_end = false; + while (ltrim($lines[$event_line - $current_doc_line], "\t") !== '/**') + { + if (ltrim($lines[$event_line - $current_doc_line], "\t") === '*/') + { + $found_comment_end = true; + } + + if ($found_comment_end) + { + $var_line = trim($lines[$event_line - $current_doc_line]); + $var_line = preg_replace('!\s+!', ' ', $var_line); + if (strpos($var_line, '* @var ') === 0) + { + $doc_line = explode(' ', $var_line, 5); + if (sizeof($doc_line) !== 5) + { + throw new LogicException('Found invalid line "' . $lines[$event_line - $current_doc_line] + . '" for event "' . $event_name . '" in file "' . $file . '"', 1); + } + $doc_vars[] = $doc_line[3]; + } + } + + $current_doc_line++; + if ($current_doc_line > $event_line) + { + // Reached the start of the file + throw new LogicException('Can not find end of docblock for event "' . $event_name . '" in file "' . $file . '"', 2); + } + } + + if (empty($doc_vars)) + { + // Reached the start of the file + throw new LogicException('Can not find @var lines for event "' . $event_name . '" in file "' . $file . '"', 3); + } + + foreach ($doc_vars as $var) + { + if (!preg_match('#^([a-zA-Z_][a-zA-Z0-9_]*)$#', $var)) + { + throw new LogicException('Found invalid @var "' . $var . '" in docblock for event "' . $event_name . '" in file "' . $file . '"', 4); + } + } + + sort($doc_vars); + return $doc_vars; + } + /** * Find the "@since" Information line * @@ -443,6 +486,27 @@ class event_exporter return $event; } + /** + * Validates that two arrays contain the same strings + * + * @param string $file + * @param string $event_name + * @param array $vars_array Variables found in the array line + * @param array $vars_docblock Variables found in the doc block + * @return null + */ + public function validate_vars_docblock_array($file, $event_name, $vars_array, $vars_docblock) + { + $vars_array = array_unique($vars_array); + $vars_docblock = array_unique($vars_docblock); + $sizeof_vars_array = sizeof($vars_array); + + if ($sizeof_vars_array !== sizeof($vars_docblock) || $sizeof_vars_array !== sizeof(array_intersect($vars_array, $vars_docblock))) + { + throw new LogicException('$vars array does not match the list of @var tags for event "' . $event_name . '" in file "' . $file . '"'); + } + } + /** * Returns a list of files in that directory * diff --git a/tests/event/exporter_test.php b/tests/event/exporter_test.php index ee9c9267b2..06a66abb04 100644 --- a/tests/event/exporter_test.php +++ b/tests/event/exporter_test.php @@ -48,17 +48,21 @@ class phpbb_event_exporter_test extends phpbb_test_case ), ), array( - 'legacy_alpha1_version.test', + 'trigger.test', array( - 'legacy_alpha1_version.dispatch' => array( - 'event' => 'legacy_alpha1_version.dispatch', - 'file' => 'legacy_alpha1_version.test', - 'arguments' => array(), - 'since' => '3.1.0-a1', - 'description' => 'Description', + 'core.trigger' => array( + 'event' => 'core.trigger', + 'file' => 'trigger.test', + 'arguments' => array('attachments', 'cp_row', 'current_row_number', 'end', 'post_row', 'row', 'start', 'user_poster_data'), + 'since' => '3.1.0-a3', + 'description' => 'Event after the post data has been assigned to the template', ), ), ), + array( + 'none.test', + array(), + ), ); } @@ -145,6 +149,289 @@ class phpbb_event_exporter_test extends phpbb_test_case $this->exporter->validate_event('', $event_name, $event); } + static public function validate_vars_docblock_array_data() + { + return array( + array(array('abc', 'def'), array('abc', 'def')), + ); + } + + /** + * @dataProvider validate_vars_docblock_array_data + */ + public function test_validate_vars_docblock_array($vars_array, $vars_docblock) + { + $this->assertNull($this->exporter->validate_vars_docblock_array('', '', $vars_array, $vars_docblock)); + } + + static public function validate_vars_docblock_array_throws_data() + { + return array( + array(array('abc', 'def'), array()), + array(array('abc', 'def'), array('abc')), + array(array('abc', 'defg'), array('abc', 'def')), + array(array('abc'), array('abc', 'def')), + array(array(), array('abc', 'def')), + ); + } + + /** + * @dataProvider validate_vars_docblock_array_throws_data + * @expectedException LogicException + */ + public function test_validate_vars_docblock_array_throws($vars_array, $vars_docblock) + { + $this->exporter->validate_vars_docblock_array('', '', $vars_array, $vars_docblock); + } + + static public function get_dispatch_name_data() + { + return array( + array("\$phpbb_dispatcher->dispatch('dispatch.one2');", 'dispatch.one2'), + array("\t\$phpbb_dispatcher->dispatch('dispatch.one2.thr_ee4');", 'dispatch.one2.thr_ee4'), + array("\$this->dispatcher->dispatch('dispatch.one2');", 'dispatch.one2'), + array("\$phpbb_dispatcher->dispatch('dis_patch.one');", 'dis_patch.one'), + ); + } + + /** + * @dataProvider get_dispatch_name_data + */ + public function test_get_dispatch_name($event_line, $expected) + { + $this->assertEquals($expected, $this->exporter->get_dispatch_name('', $event_line)); + } + + static public function get_dispatch_name_throws_data() + { + return array( + array("\$phpbb_dispatcher->dispatch();"), + array("\$phpbb_dispatcher->dispatch('');"), + array("\$phpbb_dispatcher->dispatch('dispatch.2one');"), + array("\$phpbb_dispatcher->dispatch('dispatch');"), + ); + } + + /** + * @dataProvider get_dispatch_name_throws_data + * @expectedException LogicException + */ + public function test_get_dispatch_name_throws($event_line) + { + $this->exporter->get_dispatch_name('', $event_line); + } + + static public function get_trigger_event_name_data() + { + return array( + array("extract(\$phpbb_dispatcher->trigger_event('dispatch.one2', compact(\$vars)));", 'dispatch.one2'), + array("\textract(\$phpbb_dispatcher->trigger_event('dispatch.one2.thr_ee4', compact(\$vars)));", 'dispatch.one2.thr_ee4'), + array("extract(\$this->dispatcher->trigger_event('dispatch.one2', compact(\$vars)));", 'dispatch.one2'), + array("extract(\$phpbb_dispatcher->trigger_event('dis_patch.one', compact(\$vars)));", 'dis_patch.one'), + ); + } + + /** + * @dataProvider get_trigger_event_name_data + */ + public function test_get_trigger_event_name($event_line, $expected) + { + $this->assertEquals($expected, $this->exporter->get_trigger_event_name('', $event_line)); + } + + static public function get_trigger_event_name_throws_data() + { + return array( + array("extract(\$phpbb_dispatcher->trigger_event());"), + array("extract(\$phpbb_dispatcher->trigger_event(''));"), + array("extract(\$phpbb_dispatcher->trigger_event('dispatch.2one'));"), + array("extract(\$phpbb_dispatcher->trigger_event('dispatch'));"), + array("extract(\$phpbb_dispatcher->trigger_event('dispatch.one', \$vars));"), + array("extract(\$phpbb_dispatcher->trigger_event('dispatch.one', compact(\$var)));"), + array("extract(\$phpbb_dispatcher->trigger_event('dispatch.one', compact(\$array)));"), + array("\$phpbb_dispatcher->trigger_event('dis_patch.one', compact(\$vars));", 'dis_patch.one'), + ); + } + + /** + * @dataProvider get_trigger_event_name_throws_data + * @expectedException LogicException + */ + public function test_get_trigger_event_name_throws($event_line) + { + $this->exporter->get_trigger_event_name('', $event_line); + } + + static public function get_vars_from_array_data() + { + return array( + array( + array( + '/**', + '*/', + '$vars = array(\'bertie\');', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 3, + array('bertie'), + ), + array( + array( + "\t/**", + "\t*/", + "\t\$vars = array('_Strange123', 'phpBB3_Test');", + "\t\$this->dispatcher->dispatch('test');", + ), + 3, + array('_Strange123', 'phpBB3_Test'), + ), + ); + } + + /** + * @dataProvider get_vars_from_array_data + */ + public function test_get_vars_from_array($lines, $event_line, $expected) + { + $this->assertEquals($expected, $this->exporter->get_vars_from_array('', '', $lines, $event_line)); + } + + static public function get_vars_from_array_throws_data() + { + return array( + array( + array( + '/**', + '*/', + '$vars = $bertie;', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 3, + 1, + ), + array( + array( + '/**', + '*/', + '$vars = array();', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 3, + 1, + ), + array( + array( + '/**', + '*/', + '$vars = array(\'\');', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 3, + 2, + ), + array( + array( + '/**', + '*/', + '$vars = array(\'$bertie\');', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 3, + 3, + ), + ); + } + + /** + * @dataProvider get_vars_from_array_throws_data + * @expectedException LogicException + */ + public function test_get_vars_from_array_throws($lines, $event_line, $exception_code) + { + $this->setExpectedException('LogicException', '', $exception_code); + $this->exporter->get_vars_from_array('', '', $lines, $event_line); + } + + static public function get_vars_from_docblock_data() + { + return array( + array( + array( + '/**', + '* @var int name1 Description', + '* @var array name2 Description test', + '*/', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 4, + array('name1', 'name2'), + ), + ); + } + + /** + * @dataProvider get_vars_from_docblock_data + */ + public function test_get_vars_from_docblock($lines, $event_line, $expected) + { + $this->assertEquals($expected, $this->exporter->get_vars_from_docblock('', '', $lines, $event_line)); + } + + static public function get_vars_from_docblock_throws_data() + { + return array( + array( + array( + '$vars = array();', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 1, + 2, + ), + array( + array( + '/**', + '* @var int name1', + '* @var array name2 Description test', + '*/', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 4, + 1, + ), + array( + array( + '/**', + '*/', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 2, + 3, + ), + array( + array( + '/**', + '* @var int name1 Description', + '* @var array $name2 Description', + '*/', + '$phpbb_dispatcher->dispatch(\'test\');', + ), + 4, + 4, + ), + ); + } + + /** + * @dataProvider get_vars_from_docblock_throws_data + * @expectedException LogicException + */ + public function test_get_vars_from_docblock_throws($lines, $event_line, $exception_code) + { + $this->setExpectedException('LogicException', '', $exception_code); + $this->exporter->get_vars_from_docblock('', '', $lines, $event_line); + } + static public function find_since_data() { return array( diff --git a/tests/event/fixtures/legacy_alpha1_version.test b/tests/event/fixtures/legacy_alpha1_version.test deleted file mode 100644 index 064af4daf8..0000000000 --- a/tests/event/fixtures/legacy_alpha1_version.test +++ /dev/null @@ -1,9 +0,0 @@ -dispatch('legacy_alpha1_version.dispatch'); diff --git a/tests/event/fixtures/missing_var.test b/tests/event/fixtures/missing_var.test new file mode 100644 index 0000000000..9df6e5b386 --- /dev/null +++ b/tests/event/fixtures/missing_var.test @@ -0,0 +1,18 @@ +trigger_event('core.trigger', compact($vars))); diff --git a/tests/event/fixtures/none.test b/tests/event/fixtures/none.test new file mode 100644 index 0000000000..6e2267024b --- /dev/null +++ b/tests/event/fixtures/none.test @@ -0,0 +1,6 @@ +trigger_event('core.trigger', compact($vars)));