diff --git a/file.php b/file.php index a34fbfc..bd1162e 100644 --- a/file.php +++ b/file.php @@ -461,16 +461,6 @@ public function &get_functions() { $type = implode('|', $possibletypes); - // PHP 8 treats namespaces as single token. So we are going to undo this here - // and continue returning only the final part of the namespace. Someday we'll - // move to use full namespaces here, but not for now (we are doing the same, - // in other parts of the code, when processing phpdoc blocks). - if (strpos((string)$type, '\\') !== false) { - // Namespaced typehint, potentially sub-namespaced. - // We need to strip namespacing as this area just isn't that smart. - $type = substr($type, strrpos($type, '\\') + 1); - } - $function->arguments[] = array($type, $variable); } $function->boundaries = $this->find_object_boundaries($function); diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index a1a5fe6..3034714 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -431,22 +431,11 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { // Must be at least type and parameter name. $match = false; } else { - $expectedtype = (string)$function->arguments[$i][0]; + $expectedtype = local_moodlecheck_normalise_function_type((string) $function->arguments[$i][0]); $expectedparam = (string)$function->arguments[$i][1]; - $documentedtype = $documentedarguments[$i][0]; + $documentedtype = local_moodlecheck_normalise_function_type((string) $documentedarguments[$i][0]); $documentedparam = $documentedarguments[$i][1]; - if (strpos($expectedtype, '|' ) !== false) { - $types = explode('|', $expectedtype); - sort($types); - $expectedtype = implode('|', $types); - } - if (strpos($documentedtype, '|' ) !== false) { - $types = explode('|', $documentedtype); - sort($types); - $documentedtype = implode('|', $types); - } - $typematch = $expectedtype === $documentedtype; $parammatch = $expectedparam === $documentedparam; if ($typematch && $parammatch) { @@ -455,18 +444,11 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { // Documented types can be a collection (| separated). foreach (explode('|', $documentedtype) as $documentedtype) { - // Ignore null. They cannot match any type in function. if (trim($documentedtype) === 'null') { continue; } - if (strpos($documentedtype, '\\') !== false) { - // Namespaced typehint, potentially sub-namespaced. - // We need to strip namespacing as this area just isn't that smart. - $documentedtype = substr($documentedtype, strrpos($documentedtype, '\\') + 1); - } - if (strlen($expectedtype) && $expectedtype !== $documentedtype) { // It could be a type hinted array. if ($expectedtype !== 'array' || substr($documentedtype, -2) !== '[]') { @@ -496,6 +478,42 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { return $errors; } +/** + * Normalise function type to be able to compare it. + * + * @param string $typelist + * @return string + */ +function local_moodlecheck_normalise_function_type(string $typelist): string { + // Normalise a nullable type to `null|type` as these are just shorthands. + $typelist = str_replace( + '?', + 'null|', + $typelist, + ); + + // PHP 8 treats namespaces as single token. So we are going to undo this here + // and continue returning only the final part of the namespace. Someday we'll + // move to use full namespaces here, but not for now (we are doing the same, + // in other parts of the code, when processing phpdoc blocks). + $types = explode('|', $typelist); + + // Namespaced typehint, potentially sub-namespaced. + // We need to strip namespacing as this area just isn't that smart. + $types = array_map( + function($type) { + if (strpos((string)$type, '\\') !== false) { + $type = substr($type, strrpos($type, '\\') + 1); + } + return $type; + }, + $types, + ); + sort($types); + + return implode('|', $types); +} + /** * Checks that all variables have proper \var token in phpdoc block * diff --git a/tests/phpdocs_basic_test.php b/tests/phpdocs_basic_test.php new file mode 100644 index 0000000..8f04e90 --- /dev/null +++ b/tests/phpdocs_basic_test.php @@ -0,0 +1,89 @@ +. + +namespace local_moodlecheck; + +/** + * Contains unit tests for covering "moodle" PHPDoc rules. + * + * @package local_moodlecheck + * @category test + * @copyright 2023 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class phpdocs_basic_test extends \advanced_testcase { + + public static function setUpBeforeClass(): void { + global $CFG; + require_once($CFG->dirroot . '/local/moodlecheck/locallib.php'); + require_once($CFG->dirroot . '/local/moodlecheck/rules/phpdocs_basic.php'); + } + + /** + * Test that normalisation of the method and docblock params works as expected. + * + * @dataProvider local_moodlecheck_normalise_function_type_provider + * @param string $inputtype The input type. + * @param string $expectedtype The expected type. + * @covers ::local_moodlecheck_normalise_function_type + */ + public function test_local_moodlecheck_normalise_function_type(string $inputtype, string $expectedtype): void { + $this->assertEquals( + $expectedtype, + local_moodlecheck_normalise_function_type($inputtype), + ); + } + + public static function local_moodlecheck_normalise_function_type_provider(): array { + return [ + 'Simple case' => [ + 'stdClass', 'stdClass', + ], + + 'Fully-qualified stdClass' => [ + '\stdClass', 'stdClass', + ], + + 'Fully-qualified namespaced item' => [ + \core_course\local\some\type_of_item::class, + 'type_of_item', + ], + + 'Unioned simple case' => [ + 'stdClass|object', 'object|stdClass', + ], + + 'Unioned fully-qualfied case' => [ + '\stdClass|\object', 'object|stdClass', + ], + + 'Unioned fully-qualfied namespaced item' => [ + '\stdClass|\core_course\local\some\type_of_item', + 'stdClass|type_of_item', + ], + + 'Nullable fully-qualified type' => [ + '?\core-course\local\some\type_of_item', + 'null|type_of_item', + ], + + 'Nullable fully-qualified type z-a' => [ + '?\core-course\local\some\alpha_item', + 'alpha_item|null', + ], + ]; + } +}