Skip to content

Commit

Permalink
Normalise type comparison for function and docblock params
Browse files Browse the repository at this point in the history
This includes:
- normalizing nullable to a union
- removing fqdn for each union value (not the value as a whole)
  • Loading branch information
andrewnicols committed Aug 10, 2023
1 parent d31ebb2 commit cc985cd
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 30 deletions.
10 changes: 0 additions & 10 deletions file.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
58 changes: 38 additions & 20 deletions rules/phpdocs_basic.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) !== '[]') {
Expand Down Expand Up @@ -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
*
Expand Down
89 changes: 89 additions & 0 deletions tests/phpdocs_basic_test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

namespace local_moodlecheck;

/**
* Contains unit tests for covering "moodle" PHPDoc rules.
*
* @package local_moodlecheck
* @category test
* @copyright 2023 Andrew Lyons <andrew@nicols.co.uk>
* @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',
],
];
}
}

0 comments on commit cc985cd

Please sign in to comment.