Skip to content

Commit

Permalink
Merge pull request moodlehq#119 from andrewnicols/php74SpaceNamespace
Browse files Browse the repository at this point in the history 
Split on whitespace when normalising for php 7.4
  • Loading branch information
@stronk7
stronk7 authored Aug 10, 2023
2 parents 6018946 + e3d0d91 commit bbb13f6
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 57 deletions.
48 changes: 12 additions & 36 deletions file.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,42 +394,28 @@ public function &get_functions() {
$splat = false;

if (PHP_VERSION_ID < 80000) {
$maxindex = array_key_last($argtokens);
// In PHP 7.4 and earlier, the namespace was parsed separately, for example:
// \core\course would be come '\', 'core', '\', 'course'.
// From PHP 8.0 this becomes '\core\course'.
// To address this we modify the tokens to match the PHP 8.0 format.
// This is a bit of a hack, but it works.
// Note: argtokens contains arrays of [token index, string content, line number].
for ($j = 0; $j < count($argtokens); $j++) {
if ($argtokens[$j][0] === T_NS_SEPARATOR && count($argtokens) > $j + 1) {
// If the token is a literal backslash, then
// append future tokens until we find a non-namespace token.
for ($j = 0; $j < $maxindex; $j++) {
if ($argtokens[$j][0] === T_NS_SEPARATOR || $argtokens[$j][0] === T_STRING) {
$argtokens[$j][0] = T_STRING;
$initialtoken = $j;
for ($namespacetoken = $j + 1; $namespacetoken < count($argtokens); $namespacetoken++) {
switch ($argtokens[$namespacetoken][1]) {
case '|':
case '=':
for ($namespacesearch = $j + 1; $namespacesearch < $maxindex; $namespacesearch++) {
switch ($argtokens[$namespacesearch][0]) {
case T_STRING:
case T_NS_SEPARATOR:
break;
default:
break 2;
}
$argtokens[$initialtoken][1] .= $argtokens[$namespacetoken][1];
unset($argtokens[$namespacetoken]);
$j = $namespacetoken;
}
} else if (count($argtokens) <= $j && $argtokens[$j + 1][0] === T_NS_SEPARATOR) {
// If the next token is a literal backslash, then
// append future tokens until we find a non-namespace token.
$argtokens[$j][0] = T_STRING;
$initialtoken = $j;
for ($namespacetoken = $j + 1; $namespacetoken < count($argtokens); $namespacetoken++) {
switch ($argtokens[$namespacetoken][1]) {
case '|':
case '=':
break 2;
}
$argtokens[$initialtoken][1] .= $argtokens[$namespacetoken][1];
unset($argtokens[$namespacetoken]);
$j = $namespacetoken;
$argtokens[$initialtoken][1] .= $argtokens[$namespacesearch][1];
unset($argtokens[$namespacesearch]);
$j = $namespacesearch;
}
}
}
Expand Down Expand Up @@ -475,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
60 changes: 39 additions & 21 deletions rules/phpdocs_basic.php
View file
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') {
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
16 changes: 16 additions & 0 deletions tests/fixtures/phpdoc_tags_general.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,28 @@ public function correct_return_type(): string {
/**
* Namespaced types.
*
* @param \stdClass $data
* @param \core\user $user
* @return \core\user
*/
public function namespaced_parameter_type(
\stdClass $data,
\core\user $user
): \core\user {
return $user;
}

/**
* Namespaced types.
*
* @param null|\stdClass $data
* @param null|\core\test\something|\core\some\other_thing $moredata
* @return \stdClass
*/
public function builtin(
?\stdClass $data,
?\core\test\something|\core\some\other_thing $moredata
): \stdClass {
return $user;
}
}
89 changes: 89 additions & 0 deletions tests/phpdocs_basic_test.php
View file
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 <[email protected]>
* @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 bbb13f6

Please sign in to comment.