From 165f012fa1e039d8673d529caaae0bf025a2b09a Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Fri, 25 Aug 2023 22:00:12 +1200 Subject: [PATCH 01/26] Parse types for validity, and smarter checking --- classes/type_parser.php | 594 +++++++++++++++++++++++++++++++++++ file.php | 124 +++----- rules/phpdocs_basic.php | 68 +--- tests/phpdocs_basic_test.php | 21 +- 4 files changed, 654 insertions(+), 153 deletions(-) create mode 100644 classes/type_parser.php diff --git a/classes/type_parser.php b/classes/type_parser.php new file mode 100644 index 0000000..2dd905b --- /dev/null +++ b/classes/type_parser.php @@ -0,0 +1,594 @@ +. + +namespace local_moodlecheck; + +/** + * Type parser + * + * Checks that PHPDoc types are well formed, and returns a simplified version if so, or null otherwise. + * + * @package local_moodlecheck + * @copyright 2023 Te Pūkenga – New Zealand Institute of Skills and Technology + * @author James Calder + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class type_parser { + + /** @var string the type to be parsed */ + protected $type; + + /** @var non-negative-int the position of the next token */ + protected $nextpos; + + /** @var ?non-empty-string the next token */ + protected $nexttoken; + + /** @var non-negative-int the position after the next token */ + protected $nextnextpos; + + /** + * Parse the whole type + * + * @param string $type the type to parse + * @return array{?non-empty-string, string} the simplified type, and remaining text + */ + public function parse_type(string $type): array { + + // Initialise variables. + $this->type = strtolower($type); + $this->nextnextpos = 0; + $this->prefetch_next_token(); + + // Try to parse. + try { + $result = $this->parse_dnf_type(); + if ($this->nextpos < strlen($type) && !ctype_space($type[$this->nextpos])) { + throw new \Error("Error parsing type, no space at end"); + } + return [$result, substr($type, $this->nextpos)]; + } catch (\Error $e) { + return [null, $type]; + } + + } + + /** + * Compare types + * + * @param ?string $widetypes the type that should be wider + * @param ?string $narrowtypes the type that should be narrower + * @return bool whether $narrowtypes has the same or narrower scope as $widetypes + */ + public static function compare_types(?string $widetypes, ?string $narrowtypes): bool { + if ($narrowtypes == null || $narrowtypes == '') { + return false; + } else if ($widetypes == null || $widetypes == '' || $widetypes == 'mixed' + || $narrowtypes == 'never') { + return true; + } + + $wideintersections = explode('|', $widetypes); + $narrowintersections = explode('|', $narrowtypes); + + // We have to match all documented intersections. + $haveallintersections = true; + foreach ($narrowintersections as $narrowintersection) { + $narrowsingles = explode('&', $narrowintersection); + + // If the expected types are super types, that should match. + $narrowadditions = []; + foreach ($narrowsingles as $narrowsingle) { + if ($narrowsingle == 'int') { + $supertypes = ['array-key', 'float', 'scalar']; + } else if ($narrowsingle == 'string') { + $supertypes = ['array-key', 'scaler']; + } else if ($narrowsingle == 'callable-string') { + $supertypes = ['callable', 'string', 'array-key', 'scalar']; + } else if (in_array($narrowsingle, ['array-key', 'bool', 'float'])) { + $supertypes = ['scalar']; + } else if ($narrowsingle == 'array') { + $supertypes = ['iterable']; + } else if ($narrowsingle == 'Traversable') { + $supertypes = ['iterable', 'object']; + } else if (in_array($narrowsingle, ['self', 'parent', 'static']) + || $narrowsingle[0] >= 'A' && $narrowsingle[0] <= 'Z' || $narrowsingle[0] == '_') { + $supertypes = ['object']; + } else { + $supertypes = []; + } + $narrowadditions = array_merge($narrowadditions, $supertypes); + } + $narrowsingles = array_merge($narrowsingles, $narrowadditions); + sort($narrowsingles); + $narrowsingles = array_unique($narrowsingles); + + // We need to look in each expected intersection. + $havethisintersection = false; + foreach ($wideintersections as $wideintersection) { + $widesingles = explode('&', $wideintersection); + + // And find all parts of one of them. + $haveallsingles = true; + foreach ($widesingles as $widesingle) { + + if (!in_array($widesingle, $narrowsingles)) { + $haveallsingles = false; + break; + } + + } + if ($haveallsingles) { + $havethisintersection = true; + break; + } + } + if (!$havethisintersection) { + $haveallintersections = false; + break; + } + } + return $haveallintersections; + } + + /** + * Prefetch next token + */ + protected function prefetch_next_token(): void { + + $startpos = $this->nextnextpos; + + // Ignore whitespace. + while ($startpos < strlen($this->type) && ctype_space($this->type[$startpos])) { + $startpos++; + } + + $firstchar = ($startpos < strlen($this->type)) ? $this->type[$startpos] : null; + + // Deal with different types of tokens. + if ($firstchar == null) { + // No more tokens. + $endpos = $startpos; + } else if (ctype_digit($firstchar) || $firstchar == '-') { + // Number token. + $nextchar = $firstchar; + $havepoint = false; + $endpos = $startpos; + do { + $havepoint = $havepoint || $nextchar == '.'; + $endpos = $endpos + 1; + $nextchar = ($endpos < strlen($this->type)) ? $this->type[$endpos] : null; + } while ($nextchar != null && (ctype_digit($nextchar) || $nextchar == '.' && !$havepoint)); + } else if (ctype_alpha($firstchar) || $firstchar == '_' || $firstchar == '$' || $firstchar == '\\') { + // Identifier token. + $endpos = $startpos; + do { + $endpos = $endpos + 1; + $nextchar = ($endpos < strlen($this->type)) ? $this->type[$endpos] : null; + } while ($nextchar != null && (ctype_alnum($nextchar) || $nextchar == '_' + || $firstchar != '$' && ($nextchar == '-' || $nextchar == '\\'))); + } else if ($firstchar == '"' || $firstchar == "'") { + // String token. + $endpos = $startpos; + $nextchar = $firstchar; + do { + if ($nextchar == null) { + throw new \Error('Error parsing type, unterminated string'); + } + $endpos = $endpos + 1; + $lastchar = $nextchar; + $nextchar = ($endpos < strlen($this->type)) ? $this->type[$endpos] : null; + } while ($lastchar != $firstchar || $endpos == $startpos + 1); + } else if (strlen($this->type) >= $startpos + 3 && substr($this->type, $startpos, 3) == '...') { + // Splat. + $endpos = $startpos + 3; + } else { + // Other symbol token. + $endpos = $startpos + 1; + } + + // Store token. + $this->nextpos = $this->nextnextpos; + $this->nexttoken = ($endpos > $startpos) ? substr($this->type, $startpos, $endpos - $startpos) : null; + $this->nextnextpos = $endpos; + } + + /** + * Fetch the next token + * + * @param ?string $expect the expected text + * @return non-empty-string + */ + protected function parse_token(?string $expect = null): string { + + $nexttoken = $this->nexttoken; + + // Check we have the expected token. + if ($expect != null && $nexttoken != $expect) { + throw new \Error("Error parsing type, expected {$expect}, saw {$nexttoken}"); + } else if ($nexttoken == null) { + throw new \Error("Error parsing type, unexpected end"); + } + + // Prefetch next token. + $this->prefetch_next_token(); + + // Return consumed token. + return $nexttoken; + } + + /** + * Parse a list of types seperated by | and/or &, or a single nullable type + * + * @return non-empty-string the simplified type + */ + protected function parse_dnf_type(): string { + $uniontypes = []; + + if ($this->nexttoken == '?') { + // Parse single nullable type. + $this->parse_token('?'); + array_push($uniontypes, 'void'); + array_push($uniontypes, $this->parse_single_type()); + } else { + // Parse union list. + do { + // Parse intersection list. + $havebracket = $this->nexttoken == '('; + if ($havebracket) { + $this->parse_token('('); + } + $intersectiontypes = []; + do { + array_push($intersectiontypes, $this->parse_single_type()); + // We have to figure out whether a & is for intersection or pass by reference. + // Dirty hack. TODO: Do something better. + $haveintersection = $this->nexttoken == '&' + && ($havebracket || !ctype_space($this->type[$this->nextpos]) + || $this->nextnextpos < strlen($this->type) && ctype_space($this->type[$this->nextnextpos])); + if ($haveintersection) { + $this->parse_token('&'); + } + } while ($haveintersection); + if ($havebracket) { + $this->parse_token(')'); + } + // Tidy and store intersection list. TODO: Normalise. + if (in_array('callable', $intersectiontypes) && in_array('string', $intersectiontypes)) { + $intersectiontypes[] = 'callable-string'; + } + sort($intersectiontypes); + $intersectiontypes = array_unique($intersectiontypes); + $neverpos = array_search('never', $intersectiontypes); + if ($neverpos !== false) { + $intersectiontypes = ['never']; + } + $mixedpos = array_search('mixed', $intersectiontypes); + if ($mixedpos !== false && count($intersectiontypes) > 1) { + unset($intersectiontypes[$mixedpos]); + } + array_push($uniontypes, implode('&', $intersectiontypes)); + // Check for more union items. + $haveunion = $this->nexttoken == '|'; + if ($haveunion) { + $this->parse_token('|'); + } + } while ($haveunion); + } + + // Tidy and return union list. TODO: Normalise. + sort($uniontypes); + $uniontypes = array_unique($uniontypes); + $mixedpos = array_search('mixed', $uniontypes); + if ($mixedpos !== false) { + $uniontypes = ['mixed']; + } + $neverpos = array_search('never', $uniontypes); + if ($neverpos !== false && count($uniontypes) > 1) { + unset($uniontypes[$neverpos]); + } + return implode('|', $uniontypes); + + } + + /** + * Parse a single type, possibly array type + * + * @return non-empty-string the simplified type + */ + protected function parse_single_type(): string { + + // Parse name part. + $nextchar = ($this->nexttoken == null) ? null : $this->nexttoken[0]; + if ($nextchar == '"' || $nextchar == "'" || $nextchar >= '0' && $nextchar <= '9' || $nextchar == '-' + || $nextchar == '$' || $nextchar == '\\' || $nextchar != null && ctype_alpha($nextchar) || $nextchar == '_') { + $name = $this->parse_token(); + } else { + throw new \Error("Error parsing type, expecting name, saw {$this->nexttoken}"); + } + + // Parse details part. + if ($name == 'bool' || $name == 'boolean' || $name == 'true' || $name == 'false') { + // Parse bool details. + $name = 'bool'; + } else if ($name == 'int' || $name == 'integer' + || $name == 'positive-int' || $name == 'negative-int' + || $name == 'non-positive-int' || $name == 'non-negative-int' + || $name == 'non-zero-int' + || $name == 'int-mask' || $name == 'int-mask-of' + || ($name[0] >= '0' && $name[0] <= '9' || $name[0] == '-') && strpos($name, '.') === false) { + // Parse int details. + if ($name == 'int' && $this->nexttoken == '<') { + // Parse integer range. + $this->parse_token('<'); + $nexttoken = $this->nexttoken; + if (!($nexttoken != null && ($nexttoken[0] >= '0' && $nexttoken[0] <= '9' || $nexttoken[0] == '-') + || $nexttoken == 'min')) { + throw new \Error("Error parsing type, expected int min, saw {$nexttoken}"); + }; + $this->parse_token(); + $this->parse_token(','); + $nexttoken = $this->nexttoken; + if (!($nexttoken != null && ($nexttoken[0] >= '0' && $nexttoken[0] <= '9' || $nexttoken[0] == '-') + || $nexttoken == 'max')) { + throw new \Error("Error parsing type, expected int max, saw {$nexttoken}"); + }; + $this->parse_token(); + $this->parse_token('>'); + } else if ($name == 'int-mask' || $name == 'int-mask-of') { + // Parse integer mask. + $this->parse_token('<'); + $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; + do { + if (!($nextchar != null && ctype_digit($nextchar) && strpos($this->nexttoken, '.') === false)) { + throw new \Error("Error parsing type, expected int mask, saw {$this->nexttoken}"); + } + $this->parse_token(); + $haveseperator = ($name == 'int-mask') && ($this->nexttoken == ',') + || ($name == 'int-mask-of') && ($this->nexttoken == '|'); + if ($haveseperator) { + $this->parse_token(); + } + } while ($haveseperator); + $this->parse_token('>'); + } + $name = 'int'; + } else if ($name == 'float' || $name == 'double' + || ($name[0] >= '0' && $name[0] <= '9' || $name[0] == '-') && strpos($name, '.') !== false) { + // Parse float details. + $name = 'float'; + } else if ($name == 'string' || $name == 'class-string' + || $name == 'numeric-string' || $name == 'non-empty-string' + || $name == 'non-falsy-string' || $name == 'truthy-string' + || $name == 'literal-string' + || $name[0] == '"' || $name[0] == "'") { + // Parse string details. + if ($name == 'class-string' && $this->nexttoken == '<') { + $this->parse_token('<'); + $classname = $this->parse_single_type(); + if (!($classname[0] >= 'A' && $classname[0] <= 'Z' || $classname[0] == '_')) { + throw new \Error('Error parsing type, class string type isn\'t class name'); + } + $this->parse_token('>'); + } + $name = 'string'; + } else if ($name == 'callable-string') { + // Parse callable-string details. + $name = 'callable-string'; + } else if ($name == 'array' || $name == 'non-empty-array' + || $name == 'list' || $name == 'non-empty-list') { + // Parse array details. + if ($this->nexttoken == '<') { + // Typed array. + $this->parse_token('<'); + $firsttype = $this->parse_dnf_type(); + if ($this->nexttoken == ',') { + if ($name == 'list' || $name == 'non-empty-list') { + throw new \Error('Error parsing type, lists cannot have keys specified'); + } + $key = $firsttype; + if (!in_array($key, [null, 'int', 'string', 'callable-string', 'array-key'])) { + throw new \Error('Error parsing type, invalid array key'); + } + $this->parse_token(','); + $value = $this->parse_dnf_type(); + } else { + $key = null; + $value = $firsttype; + } + $this->parse_token('>'); + } else if ($this->nexttoken == '{') { + // Array shape. + if ($name == 'list' || $name == 'non-empty-list') { + throw new \Error('Error parsing type, lists cannot have shapes'); + } + $this->parse_token('{'); + do { + $key = null; + $savednextpos = $this->nextpos; + $savednexttoken = $this->nexttoken; + $savednextnextpos = $this->nextnextpos; + try { + $key = $this->parse_token(); + if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == "'" || $key[0] == '"' + || (ctype_digit($key) || $key[0] == '-') && strpos($key, '.') === false)) { + throw new \Error('Error parsing type, invalid array key'); + } + if ($this->nexttoken == '?') { + $this->parse_token('?'); + } + $this->parse_token(':'); + } catch (\Error $e) { + $this->nextpos = $savednextpos; + $this->nexttoken = $savednexttoken; + $this->nextnextpos = $savednextnextpos; + } + $this->parse_dnf_type(); + $havecomma = $this->nexttoken == ','; + if ($havecomma) { + $this->parse_token(','); + } + } while ($havecomma); + $this->parse_token('}'); + } + $name = 'array'; + } else if ($name == 'object') { + // Parse object details. + if ($this->nexttoken == '{') { + // Object shape. + $this->parse_token('{'); + do { + $key = $this->parse_token(); + if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == "'" || $key[0] == '"')) { + throw new \Error('Error parsing type, invalid array key'); + } + if ($this->nexttoken == '?') { + $this->parse_token('?'); + } + $this->parse_token(':'); + $this->parse_dnf_type(); + $havecomma = $this->nexttoken == ','; + if ($havecomma) { + $this->parse_token(','); + } + } while ($havecomma); + $this->parse_token('}'); + } + $name = 'object'; + } else if ($name == 'resource') { + // Parse resource details. + $name = 'resource'; + } else if ($name == 'never' || $name == 'never-return' || $name == 'never-returns' || $name == 'no-return') { + // Parse never details. + $name = 'never'; + } else if ($name == 'void' || $name == 'null') { + // Parse void details. + $name = 'void'; + } else if ($name == 'self') { + // Parse self details. + $name = 'self'; + } else if ($name == 'parent') { + // Parse parent details. + $name = 'parent'; + } else if ($name == 'static' || $name == '$this') { + // Parse static details. + $name = 'static'; + } else if ($name == 'callable') { + // Parse callable details. + if ($this->nexttoken == '(') { + $this->parse_token('('); + $splat = false; + while ($this->nexttoken != ')') { + $this->parse_dnf_type(); + if ($this->nexttoken == '=') { + $this->parse_token('='); + } + if ($this->nexttoken == '...') { + $this->parse_token('...'); + $splat = true; + } + $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; + if ($nextchar == '&' || $nextchar == '$') { + if ($this->nexttoken == '&') { + if ($splat) { + throw new \Error("Error parsing type, can't pass splat by reference"); + } + $this->parse_token('&'); + } + $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; + if ($nextchar == '$') { + $this->parse_token(); + } else { + throw new \Error("Error parsing type, expected var name, saw {$this->nexttoken}"); + } + } + if ($this->nexttoken != ')') { + if ($splat) { + throw new \Error("Error parsing type, expected end of param list, saw {$this->nexttoken}"); + } + $this->parse_token(','); + } + }; + $this->parse_token(')'); + $this->parse_token(':'); + if ($this->nexttoken == '(') { + $this->parse_token('('); + $this->parse_dnf_type(); + $this->parse_token(')'); + } else { + if ($this->nexttoken == '?') { + $this->parse_token('?'); + } + $this->parse_single_type(); + } + } + $name = 'callable'; + } else if ($name == 'mixed') { + // Parse mixed details. + $name = 'mixed'; + } else if ($name == 'iterable') { + // Parse iterable details (Traversable|array). + if ($this->nexttoken == '<') { + $this->parse_token('<'); + $this->parse_dnf_type(); + $this->parse_token('>'); + } + $name = 'iterable'; + } else if ($name == 'array-key') { + // Parse array-key details (int|string). + $name = 'array-key'; + } else if ($name == 'scalar') { + // Parse scalar details (bool|int|float|string). + $name = 'scalar'; + } else if ($name == 'key-of') { + // Parse key-of details. + $this->parse_token('<'); + $this->parse_single_type(); + $this->parse_token('>'); + $name = 'array-key'; + } else if ($name == 'value-of') { + // Parse value-of details. + $this->parse_token('<'); + $this->parse_single_type(); + $this->parse_token('>'); + $name = 'mixed'; + } else { + // Check valid class name. + if (strpos($name, '$') !== false || strpos($name, '-') !== false || strpos($name, '\\\\') !== false) { + throw new \Error('Error parsing type, invalid class name'); + } + $lastseperatorpos = strrpos($name, '\\'); + if ($lastseperatorpos !== false) { + $name = substr($name, $lastseperatorpos + 1); + } + if ($name == '') { + throw new \Error('Error parsing type, class name has trailing slash'); + } + $name = ucfirst($name); + } + // TODO: Conditional return types. + + // Parse array suffix. + $arrayed = ($this->nexttoken == '['); + if ($arrayed) { + $this->parse_token('['); + $this->parse_token(']'); + } + + return $arrayed ? 'array' : $name; + } + +} diff --git a/file.php b/file.php index bd1162e..5eed699 100644 --- a/file.php +++ b/file.php @@ -344,6 +344,7 @@ public function &get_classes() { */ public function &get_functions() { if ($this->functions === null) { + $typeparser = new \local_moodlecheck\type_parser(); $this->functions = array(); $tokens = &$this->get_tokens(); for ($tid = 0; $tid < $this->tokenscount; $tid++) { @@ -389,77 +390,46 @@ public function &get_functions() { if (empty($argtokens)) { continue; } - $possibletypes = []; - $variable = null; - $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 < $maxindex; $j++) { - if ($argtokens[$j][0] === T_NS_SEPARATOR || $argtokens[$j][0] === T_STRING) { - $argtokens[$j][0] = T_STRING; - $initialtoken = $j; - 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[$namespacesearch][1]; - unset($argtokens[$namespacesearch]); - $j = $namespacesearch; - } - } - } + + $j = 0; + + // Skip argument visibility. + while ($j < count($argtokens) + && in_array($argtokens[$j][0], [T_WHITESPACE, T_PUBLIC, T_PROTECTED, T_PRIVATE])) { + $j++; } - $argtokens = array_values($argtokens); - - for ($j = 0; $j < count($argtokens); $j++) { - switch ($argtokens[$j][0]) { - // Skip any whitespace, or argument visibility. - case T_WHITESPACE: - case T_PUBLIC: - case T_PROTECTED: - case T_PRIVATE: - continue 2; - case T_VARIABLE: - // The variale name, adding in the vardiadic if required. - $variable = ($splat) ? '...' . $argtokens[$j][1] : $argtokens[$j][1]; - continue 2; - case T_ELLIPSIS: - // For example ...$example - // Variadic function. - $splat = true; - continue 2; - } - switch ($argtokens[$j][1]) { - case '|': - // Union types. - case '&': - // Return by reference. - continue 2; - case '?': - // Nullable type. - $possibletypes[] = 'null'; - continue 2; - case '=': - // Default value. - $j = count($argtokens); - continue 2; - } - $possibletypes[] = $argtokens[$j][1]; + // Get type. + $typeandremainder = ''; + for (; $j < count($argtokens); $j++) { + $typeandremainder .= $argtokens[$j][1]; } + list($type, $remainder) = $typeparser->parse_type($typeandremainder); - $type = implode('|', $possibletypes); + // Get variable. + $variable = ''; + $j = 0; + while ($j < strlen($remainder) && ctype_space($remainder[$j])) { + $j++; + } + if (substr($remainder, $j, 3) == '...') { + $variable .= '...'; + $j += 3; + } + while ($j < strlen($remainder) && ctype_space($remainder[$j])) { + $j++; + } + if (substr($remainder, $j, 1) == '&') { + $j += 1; + } + while ($j < strlen($remainder) && ctype_space($remainder[$j])) { + $j++; + } + while ($j < strlen($remainder) + && ($remainder[$j] == '$' || ctype_alnum($remainder[$j]) || $remainder[$j] == '_')) { + $variable .= $remainder[$j]; + $j++; + } $function->arguments[] = array($type, $variable); } @@ -1389,27 +1359,15 @@ public function get_shortdescription() { * @return array */ public function get_params($tag = 'param', $splitlimit = 3) { + $typeparser = new \local_moodlecheck\type_parser(); $params = array(); foreach ($this->get_tags($tag) as $token) { - $params[] = preg_split('/\s+/', trim($token), $splitlimit); // AKA 'type $name multi-word description'. + list($type, $remainder) = $typeparser->parse_type($token); + $tokenarray = array_merge([$type], preg_split('/\s+/', trim($remainder), $splitlimit - 1)); + $params[] = $tokenarray; // AKA 'type $name multi-word description'. } - foreach ($params as $key => $param) { - if (strpos($param[0], '?') !== false) { - $param[0] = str_replace('?', 'null|', $param[0]); - } - $types = explode('|', $param[0]); - $types = array_map(function($type): string { - // Normalise array types such as `string[]` to `array`. - if (substr($type, -2) == '[]') { - return 'array'; - } - return $type; - }, $types); - sort($types); - $params[$key][0] = implode('|', $types); - } return $params; } diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index 661caed..eb15e02 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -427,39 +427,17 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { $documentedarguments = $function->phpdocs->get_params(); $match = (count($documentedarguments) == count($function->arguments)); for ($i = 0; $match && $i < count($documentedarguments); $i++) { - if (count($documentedarguments[$i]) < 2) { + if (count($documentedarguments[$i]) < 2 || $documentedarguments[$i][0] == null) { // Must be at least type and parameter name. $match = false; } else { - $expectedtype = local_moodlecheck_normalise_function_type((string) $function->arguments[$i][0]); + $expectedtype = $function->arguments[$i][0]; $expectedparam = (string)$function->arguments[$i][1]; - $documentedtype = local_moodlecheck_normalise_function_type((string) $documentedarguments[$i][0]); + $documentedtype = $documentedarguments[$i][0]; $documentedparam = $documentedarguments[$i][1]; - $typematch = $expectedtype === $documentedtype; - $parammatch = $expectedparam === $documentedparam; - if ($typematch && $parammatch) { - continue; - } - - // 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 (strlen($expectedtype) && $expectedtype !== $documentedtype) { - // It could be a type hinted array. - if ($expectedtype !== 'array' || substr($documentedtype, -2) !== '[]') { - $match = false; - } - } else if ($documentedtype === 'type') { - $match = false; - } else if ($expectedparam !== $documentedparam) { - $match = false; - } - } + $match = ($expectedparam === $documentedparam) + && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype); } } $documentedreturns = $function->phpdocs->get_params('return'); @@ -482,36 +460,12 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { * Normalise function type to be able to compare it. * * @param string $typelist - * @return string + * @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); +function local_moodlecheck_normalise_function_type(string $typelist): ?string { + $typeparser = new \local_moodlecheck\type_parser(); + list($type, $remainder) = $typeparser->parse_type($typelist); + return $type; } /** @@ -525,7 +479,7 @@ function local_moodlecheck_variableshasvar(local_moodlecheck_file $file) { foreach ($file->get_variables() as $variable) { if ($variable->phpdocs !== false) { $documentedvars = $variable->phpdocs->get_params('var', 2); - if (!count($documentedvars) || $documentedvars[0][0] == 'type') { + if (!count($documentedvars) || $documentedvars[0][0] == 'type' || $documentedvars[0][0] == null) { $errors[] = array( 'line' => $variable->phpdocs->get_line_number($file, '@var'), 'variable' => $variable->fullname); diff --git a/tests/phpdocs_basic_test.php b/tests/phpdocs_basic_test.php index 7d98d4b..7fae63d 100644 --- a/tests/phpdocs_basic_test.php +++ b/tests/phpdocs_basic_test.php @@ -50,39 +50,34 @@ public function test_local_moodlecheck_normalise_function_type(string $inputtype public static function local_moodlecheck_normalise_function_type_provider(): array { return [ 'Simple case' => [ - 'stdClass', 'stdClass', + 'stdClass', 'Stdclass', ], 'Fully-qualified stdClass' => [ - '\stdClass', 'stdClass', + '\stdClass', 'Stdclass', ], 'Fully-qualified namespaced item' => [ \core_course\local\some\type_of_item::class, - 'type_of_item', + 'Type_of_item', ], 'Unioned simple case' => [ - 'stdClass|object', 'object|stdClass', + 'stdClass|object', 'Stdclass|object', ], 'Unioned fully-qualfied case' => [ - '\stdClass|\object', 'object|stdClass', + '\stdClass|\object', 'Object|Stdclass', ], 'Unioned fully-qualfied namespaced item' => [ '\stdClass|\core_course\local\some\type_of_item', - 'stdClass|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', + '?\core_course\local\some\type_of_item', + 'Type_of_item|void', ], ]; } From 5e33b89b67712a5db5d312b5c7677812877ecbeb Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Sat, 26 Aug 2023 17:53:42 +1200 Subject: [PATCH 02/26] Fix mistakes and omission --- classes/type_parser.php | 141 +++++++++++++++++++++++++++++----------- file.php | 48 ++++---------- rules/phpdocs_basic.php | 6 +- 3 files changed, 121 insertions(+), 74 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 2dd905b..261baa7 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -43,27 +43,63 @@ class type_parser { /** * Parse the whole type * - * @param string $type the type to parse - * @return array{?non-empty-string, string} the simplified type, and remaining text + * @param string $intype the type to parse + * @param bool $getvar whether to get variable name + * @return array{?non-empty-string, ?non-empty-string, string} the simplified type, variable, and remaining text */ - public function parse_type(string $type): array { + public function parse_type_and_var(string $intype, bool $getvar = true): array { // Initialise variables. - $this->type = strtolower($type); + $this->type = strtolower($intype); $this->nextnextpos = 0; $this->prefetch_next_token(); - // Try to parse. + // Try to parse type. + $savednextpos = $this->nextpos; + $savednexttoken = $this->nexttoken; + $savednextnextpos = $this->nextnextpos; try { - $result = $this->parse_dnf_type(); - if ($this->nextpos < strlen($type) && !ctype_space($type[$this->nextpos])) { - throw new \Error("Error parsing type, no space at end"); + $outtype = $this->parse_dnf_type(); + if ($this->nextpos < strlen($intype) && !ctype_space($intype[$this->nextpos]) && $this->nexttoken != '...') { + throw new \Error("Error parsing type, no space at end of type"); } - return [$result, substr($type, $this->nextpos)]; } catch (\Error $e) { - return [null, $type]; + $this->nextpos = $savednextpos; + $this->nexttoken = $savednexttoken; + $this->nextnextpos = $savednextnextpos; + $outtype = null; } + // Try to parse variable. + if ($getvar) { + $savednextpos = $this->nextpos; + $savednexttoken = $this->nexttoken; + $savednextnextpos = $this->nextnextpos; + try { + if ($this->nexttoken == '&') { + $this->parse_token('&'); + } + if ($this->nexttoken == '...') { + $this->parse_token('...'); + } + if (!($this->nexttoken != null && $this->nexttoken[0] == '$')) { + throw new \Error("Error parsing type, expected variable, saw {$this->nexttoken}"); + } + $variable = $this->parse_token(); + if ($this->nextpos < strlen($intype) && !ctype_space($intype[$this->nextpos]) && $this->nexttoken != '=') { + throw new \Error("Error parsing type, no space at end of variable"); + } + } catch (\Error $e) { + $this->nextpos = $savednextpos; + $this->nexttoken = $savednexttoken; + $this->nextnextpos = $savednextnextpos; + $variable = null; + } + } else { + $variable = null; + } + + return [$outtype, $variable, trim(substr($intype, $this->nextpos))]; } /** @@ -92,24 +128,7 @@ public static function compare_types(?string $widetypes, ?string $narrowtypes): // If the expected types are super types, that should match. $narrowadditions = []; foreach ($narrowsingles as $narrowsingle) { - if ($narrowsingle == 'int') { - $supertypes = ['array-key', 'float', 'scalar']; - } else if ($narrowsingle == 'string') { - $supertypes = ['array-key', 'scaler']; - } else if ($narrowsingle == 'callable-string') { - $supertypes = ['callable', 'string', 'array-key', 'scalar']; - } else if (in_array($narrowsingle, ['array-key', 'bool', 'float'])) { - $supertypes = ['scalar']; - } else if ($narrowsingle == 'array') { - $supertypes = ['iterable']; - } else if ($narrowsingle == 'Traversable') { - $supertypes = ['iterable', 'object']; - } else if (in_array($narrowsingle, ['self', 'parent', 'static']) - || $narrowsingle[0] >= 'A' && $narrowsingle[0] <= 'Z' || $narrowsingle[0] == '_') { - $supertypes = ['object']; - } else { - $supertypes = []; - } + $supertypes = static::super_types($narrowsingle); $narrowadditions = array_merge($narrowadditions, $supertypes); } $narrowsingles = array_merge($narrowsingles, $narrowadditions); @@ -144,6 +163,34 @@ public static function compare_types(?string $widetypes, ?string $narrowtypes): return $haveallintersections; } + /** + * Get super types + * + * @param string $basetype + * @return non-empty-string[] super types + */ + protected static function super_types(string $basetype): array { + if ($basetype == 'int') { + $supertypes = ['array-key', 'float', 'scalar']; + } else if ($basetype == 'string') { + $supertypes = ['array-key', 'scaler']; + } else if ($basetype == 'callable-string') { + $supertypes = ['callable', 'string', 'array-key', 'scalar']; + } else if (in_array($basetype, ['array-key', 'bool', 'float'])) { + $supertypes = ['scalar']; + } else if ($basetype == 'array') { + $supertypes = ['iterable']; + } else if ($basetype == 'Traversable') { + $supertypes = ['iterable', 'object']; + } else if (in_array($basetype, ['self', 'parent', 'static']) + || $basetype[0] >= 'A' && $basetype[0] <= 'Z' || $basetype[0] == '_') { + $supertypes = ['object']; + } else { + $supertypes = []; + } + return $supertypes; + } + /** * Prefetch next token */ @@ -270,6 +317,15 @@ protected function parse_dnf_type(): string { if (in_array('callable', $intersectiontypes) && in_array('string', $intersectiontypes)) { $intersectiontypes[] = 'callable-string'; } + foreach ($intersectiontypes as $intersectiontype) { + $supertypes = static::super_types($intersectiontype); + foreach ($supertypes as $supertype) { + $superpos = array_search($supertype, $intersectiontypes); + if ($superpos !== false) { + unset($intersectiontypes[$superpos]); + } + } + } sort($intersectiontypes); $intersectiontypes = array_unique($intersectiontypes); $neverpos = array_search('never', $intersectiontypes); @@ -290,6 +346,20 @@ protected function parse_dnf_type(): string { } // Tidy and return union list. TODO: Normalise. + if ((in_array('int', $uniontypes) || in_array('float', $uniontypes)) && in_array('string', $uniontypes)) { + $uniontypes[] = 'array-key'; + } + if (in_array('array-key', $uniontypes) && in_array('bool', $uniontypes) && in_array('float', $uniontypes)) { + $uniontypes[] = 'scalar'; + } + if (in_array('Traversable', $uniontypes) && in_array('array', $uniontypes)) { + $uniontypes[] = 'iterable'; + } + if (in_array('scalar', $uniontypes) && (in_array('array', $uniontypes) || in_array('iterable', $uniontypes)) + && in_array('object', $uniontypes) && in_array('resource', $uniontypes) && in_array('callable', $uniontypes) + && in_array('void', $uniontypes)) { + $uniontypes = ['mixed']; + } sort($uniontypes); $uniontypes = array_unique($uniontypes); $mixedpos = array_search('mixed', $uniontypes); @@ -493,21 +563,18 @@ protected function parse_single_type(): string { $splat = false; while ($this->nexttoken != ')') { $this->parse_dnf_type(); - if ($this->nexttoken == '=') { - $this->parse_token('='); + if ($this->nexttoken == '&') { + $this->parse_token('&'); } if ($this->nexttoken == '...') { $this->parse_token('...'); $splat = true; } + if ($this->nexttoken == '=') { + $this->parse_token('='); + } $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; - if ($nextchar == '&' || $nextchar == '$') { - if ($this->nexttoken == '&') { - if ($splat) { - throw new \Error("Error parsing type, can't pass splat by reference"); - } - $this->parse_token('&'); - } + if ($nextchar == '$') { $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; if ($nextchar == '$') { $this->parse_token(); diff --git a/file.php b/file.php index 5eed699..3bcd15b 100644 --- a/file.php +++ b/file.php @@ -399,37 +399,12 @@ public function &get_functions() { $j++; } - // Get type. - $typeandremainder = ''; + // Get type and variable. + $text = ''; for (; $j < count($argtokens); $j++) { - $typeandremainder .= $argtokens[$j][1]; - } - list($type, $remainder) = $typeparser->parse_type($typeandremainder); - - // Get variable. - $variable = ''; - $j = 0; - while ($j < strlen($remainder) && ctype_space($remainder[$j])) { - $j++; - } - if (substr($remainder, $j, 3) == '...') { - $variable .= '...'; - $j += 3; - } - while ($j < strlen($remainder) && ctype_space($remainder[$j])) { - $j++; - } - if (substr($remainder, $j, 1) == '&') { - $j += 1; - } - while ($j < strlen($remainder) && ctype_space($remainder[$j])) { - $j++; - } - while ($j < strlen($remainder) - && ($remainder[$j] == '$' || ctype_alnum($remainder[$j]) || $remainder[$j] == '_')) { - $variable .= $remainder[$j]; - $j++; + $text .= $argtokens[$j][1]; } + list($type, $variable, $default) = $typeparser->parse_type_and_var($text); $function->arguments[] = array($type, $variable); } @@ -1355,17 +1330,22 @@ public function get_shortdescription() { * Each element is array(typename, variablename, variabledescription) * * @param string $tag tag name to look for. Usually param but may be var for variables - * @param int $splitlimit maximum number of chunks to return + * @param bool $getvar whether to get variable name * @return array */ - public function get_params($tag = 'param', $splitlimit = 3) { + public function get_params($tag = 'param', $getvar = true) { $typeparser = new \local_moodlecheck\type_parser(); $params = array(); foreach ($this->get_tags($tag) as $token) { - list($type, $remainder) = $typeparser->parse_type($token); - $tokenarray = array_merge([$type], preg_split('/\s+/', trim($remainder), $splitlimit - 1)); - $params[] = $tokenarray; // AKA 'type $name multi-word description'. + list($type, $variable, $description) = $typeparser->parse_type_and_var($token, $getvar); + $param = []; + $param[] = $type; + if ($getvar) { + $param[] = $variable; + } + $param[] = $description; + $params[] = $param; } return $params; diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index eb15e02..80fa5ea 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -440,7 +440,7 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype); } } - $documentedreturns = $function->phpdocs->get_params('return'); + $documentedreturns = $function->phpdocs->get_params('return', false); for ($i = 0; $match && $i < count($documentedreturns); $i++) { if (empty($documentedreturns[$i][0]) || $documentedreturns[$i][0] == 'type') { $match = false; @@ -464,7 +464,7 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { */ function local_moodlecheck_normalise_function_type(string $typelist): ?string { $typeparser = new \local_moodlecheck\type_parser(); - list($type, $remainder) = $typeparser->parse_type($typelist); + list($type, $variable, $remainder) = $typeparser->parse_type_and_var($typelist, false); return $type; } @@ -478,7 +478,7 @@ function local_moodlecheck_variableshasvar(local_moodlecheck_file $file) { $errors = array(); foreach ($file->get_variables() as $variable) { if ($variable->phpdocs !== false) { - $documentedvars = $variable->phpdocs->get_params('var', 2); + $documentedvars = $variable->phpdocs->get_params('var', false); if (!count($documentedvars) || $documentedvars[0][0] == 'type' || $documentedvars[0][0] == null) { $errors[] = array( 'line' => $variable->phpdocs->get_line_number($file, '@var'), From 727da5f55cf38402126b7a13f69b3c88099e0d9f Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Sat, 26 Aug 2023 19:31:50 +1200 Subject: [PATCH 03/26] Minor tweak --- classes/type_parser.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 261baa7..50a4ba1 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -60,7 +60,8 @@ public function parse_type_and_var(string $intype, bool $getvar = true): array { $savednextnextpos = $this->nextnextpos; try { $outtype = $this->parse_dnf_type(); - if ($this->nextpos < strlen($intype) && !ctype_space($intype[$this->nextpos]) && $this->nexttoken != '...') { + if ($this->nextpos < strlen($intype) && !ctype_space($intype[$this->nextpos]) + && !($this->nexttoken == '&' || $this->nexttoken == '...')) { throw new \Error("Error parsing type, no space at end of type"); } } catch (\Error $e) { @@ -303,9 +304,9 @@ protected function parse_dnf_type(): string { array_push($intersectiontypes, $this->parse_single_type()); // We have to figure out whether a & is for intersection or pass by reference. // Dirty hack. TODO: Do something better. - $haveintersection = $this->nexttoken == '&' - && ($havebracket || !ctype_space($this->type[$this->nextpos]) - || $this->nextnextpos < strlen($this->type) && ctype_space($this->type[$this->nextnextpos])); + $haveintersection = $this->nexttoken == '&' && ($this->nextnextpos < strlen($this->type)) + && ($havebracket || !(ctype_space($this->type[$this->nextpos]) + xor ctype_space($this->type[$this->nextnextpos]))); if ($haveintersection) { $this->parse_token('&'); } From 254e3b95401b477f1d9cc320ff2dd96c9ec619b4 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Sat, 26 Aug 2023 21:56:28 +1200 Subject: [PATCH 04/26] Tweak the tweak --- classes/type_parser.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 50a4ba1..c18bb5d 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -304,9 +304,11 @@ protected function parse_dnf_type(): string { array_push($intersectiontypes, $this->parse_single_type()); // We have to figure out whether a & is for intersection or pass by reference. // Dirty hack. TODO: Do something better. - $haveintersection = $this->nexttoken == '&' && ($this->nextnextpos < strlen($this->type)) - && ($havebracket || !(ctype_space($this->type[$this->nextpos]) - xor ctype_space($this->type[$this->nextnextpos]))); + $haveintersection = $this->nexttoken == '&' && ($havebracket + || !($this->nextnextpos >= strlen($this->type) + || in_array($this->type[$this->nextnextpos], ['.', '=', '$', ',', ')'])) + && !(!ctype_space($this->type[$this->nextpos]) + && ctype_space($this->type[$this->nextnextpos]))); if ($haveintersection) { $this->parse_token('&'); } From 8c34e5a2291ffcaa666f50cfd154d3e239eac032 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Sun, 27 Aug 2023 15:49:50 +1200 Subject: [PATCH 05/26] Proper fix, and support a bit more stuff --- classes/type_parser.php | 73 +++++++++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index c18bb5d..a09e548 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -243,6 +243,9 @@ protected function prefetch_next_token(): void { } else if (strlen($this->type) >= $startpos + 3 && substr($this->type, $startpos, 3) == '...') { // Splat. $endpos = $startpos + 3; + } else if (strlen($this->type) >= $startpos + 2 && substr($this->type, $startpos, 2) == '::') { + // Scope resolution operator. + $endpos = $startpos + 2; } else { // Other symbol token. $endpos = $startpos + 1; @@ -303,12 +306,14 @@ protected function parse_dnf_type(): string { do { array_push($intersectiontypes, $this->parse_single_type()); // We have to figure out whether a & is for intersection or pass by reference. - // Dirty hack. TODO: Do something better. - $haveintersection = $this->nexttoken == '&' && ($havebracket - || !($this->nextnextpos >= strlen($this->type) - || in_array($this->type[$this->nextnextpos], ['.', '=', '$', ',', ')'])) - && !(!ctype_space($this->type[$this->nextpos]) - && ctype_space($this->type[$this->nextnextpos]))); + // Dirty hack. + $nextnextpos = $this->nextnextpos; + while ($nextnextpos < strlen($this->type) && ctype_space($this->type[$nextnextpos])) { + $nextnextpos++; + } + $nextnextchar = ($nextnextpos < strlen($this->type)) ? $this->type[$nextnextpos] : null; + $haveintersection = $this->nexttoken == '&' + && ($havebracket || !in_array($nextnextchar, ['.', '=', '$', ',', ')', null])); if ($haveintersection) { $this->parse_token('&'); } @@ -316,7 +321,7 @@ protected function parse_dnf_type(): string { if ($havebracket) { $this->parse_token(')'); } - // Tidy and store intersection list. TODO: Normalise. + // Tidy and store intersection list. if (in_array('callable', $intersectiontypes) && in_array('string', $intersectiontypes)) { $intersectiontypes[] = 'callable-string'; } @@ -339,6 +344,7 @@ protected function parse_dnf_type(): string { if ($mixedpos !== false && count($intersectiontypes) > 1) { unset($intersectiontypes[$mixedpos]); } + // TODO: Check for conflicting types. array_push($uniontypes, implode('&', $intersectiontypes)); // Check for more union items. $haveunion = $this->nexttoken == '|'; @@ -348,7 +354,7 @@ protected function parse_dnf_type(): string { } while ($haveunion); } - // Tidy and return union list. TODO: Normalise. + // Tidy and return union list. if ((in_array('int', $uniontypes) || in_array('float', $uniontypes)) && in_array('string', $uniontypes)) { $uniontypes[] = 'array-key'; } @@ -373,6 +379,7 @@ protected function parse_dnf_type(): string { if ($neverpos !== false && count($uniontypes) > 1) { unset($uniontypes[$neverpos]); } + // TODO: Check for redundant types. return implode('|', $uniontypes); } @@ -425,17 +432,22 @@ protected function parse_single_type(): string { // Parse integer mask. $this->parse_token('<'); $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; - do { - if (!($nextchar != null && ctype_digit($nextchar) && strpos($this->nexttoken, '.') === false)) { - throw new \Error("Error parsing type, expected int mask, saw {$this->nexttoken}"); - } - $this->parse_token(); - $haveseperator = ($name == 'int-mask') && ($this->nexttoken == ',') - || ($name == 'int-mask-of') && ($this->nexttoken == '|'); - if ($haveseperator) { + if (ctype_digit($nextchar) || $name == 'int-mask') { + do { + if (!($nextchar != null && ctype_digit($nextchar) && strpos($this->nexttoken, '.') === false)) { + throw new \Error("Error parsing type, expected int mask, saw {$this->nexttoken}"); + } $this->parse_token(); - } - } while ($haveseperator); + $haveseperator = ($name == 'int-mask') && ($this->nexttoken == ',') + || ($name == 'int-mask-of') && ($this->nexttoken == '|'); + if ($haveseperator) { + $this->parse_token(); + } + $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; + } while ($haveseperator); + } else { + $this->parse_single_type(); + } $this->parse_token('>'); } $name = 'int'; @@ -626,13 +638,13 @@ protected function parse_single_type(): string { } else if ($name == 'key-of') { // Parse key-of details. $this->parse_token('<'); - $this->parse_single_type(); + $this->parse_dnf_type(); $this->parse_token('>'); $name = 'array-key'; } else if ($name == 'value-of') { // Parse value-of details. $this->parse_token('<'); - $this->parse_single_type(); + $this->parse_dnf_type(); $this->parse_token('>'); $name = 'mixed'; } else { @@ -649,16 +661,27 @@ protected function parse_single_type(): string { } $name = ucfirst($name); } - // TODO: Conditional return types. - // Parse array suffix. - $arrayed = ($this->nexttoken == '['); - if ($arrayed) { + if ($this->nexttoken == '::' && ($name == 'object' || in_array('object', static::super_types($name)))) { + // Parse class constant. + $this->parse_token('::'); + $nextchar = ($this->nexttoken == null) ? null : $this->nexttoken[0]; + $haveconstantname = $nextchar != null && (ctype_alpha($nextchar) || $nextchar == '_'); + if ($haveconstantname) { + $this->parse_token(); + } + if ($this->nexttoken == '*' || !$haveconstantname) { + $this->parse_token('*'); + } + $name = 'mixed'; + } else if ($this->nexttoken == '[') { + // Parse array suffix. $this->parse_token('['); $this->parse_token(']'); + $name = 'array'; } - return $arrayed ? 'array' : $name; + return $name; } } From 207c0223befc51a27dc76d194aa9cf0c549181f9 Mon Sep 17 00:00:00 2001 From: james-cnz <5689414+james-cnz@users.noreply.github.com> Date: Mon, 28 Aug 2023 13:35:05 +1200 Subject: [PATCH 06/26] Fix false positives and tidying --- classes/type_parser.php | 296 +++++++++++++++++++--------------------- 1 file changed, 138 insertions(+), 158 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index a09e548..46ae5c0 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -28,8 +28,11 @@ */ class type_parser { - /** @var string the type to be parsed */ - protected $type; + /** @var string the text to be parsed */ + protected $text; + + /** @var bool when we encounter an unknown type, should we go wide or narrow */ + protected $gowide; /** @var non-negative-int the position of the next token */ protected $nextpos; @@ -41,41 +44,36 @@ class type_parser { protected $nextnextpos; /** - * Parse the whole type + * Parse a type and possibly variable name * - * @param string $intype the type to parse + * @param string $text the text to parse * @param bool $getvar whether to get variable name * @return array{?non-empty-string, ?non-empty-string, string} the simplified type, variable, and remaining text */ - public function parse_type_and_var(string $intype, bool $getvar = true): array { + public function parse_type_and_var(string $text, bool $getvar = true): array { // Initialise variables. - $this->type = strtolower($intype); + $this->text = strtolower($text); + $this->gowide = false; $this->nextnextpos = 0; $this->prefetch_next_token(); // Try to parse type. - $savednextpos = $this->nextpos; - $savednexttoken = $this->nexttoken; - $savednextnextpos = $this->nextnextpos; + $savedstate = [$this->nextpos, $this->nexttoken, $this->nextnextpos]; try { - $outtype = $this->parse_dnf_type(); - if ($this->nextpos < strlen($intype) && !ctype_space($intype[$this->nextpos]) - && !($this->nexttoken == '&' || $this->nexttoken == '...')) { - throw new \Error("Error parsing type, no space at end of type"); + $type = $this->parse_dnf_type(); + if ($this->nextpos < strlen($text) + && !(ctype_space($text[$this->nextpos]) || $this->nexttoken == '&' || $this->nexttoken == '...')) { + throw new \Error("Error parsing type, no space at end of type."); } } catch (\Error $e) { - $this->nextpos = $savednextpos; - $this->nexttoken = $savednexttoken; - $this->nextnextpos = $savednextnextpos; - $outtype = null; + list($this->nextpos, $this->nexttoken, $this->nextnextpos) = $savedstate; + $type = null; } // Try to parse variable. if ($getvar) { - $savednextpos = $this->nextpos; - $savednexttoken = $this->nexttoken; - $savednextnextpos = $this->nextnextpos; + $savedstate = [$this->nextpos, $this->nexttoken, $this->nextnextpos]; try { if ($this->nexttoken == '&') { $this->parse_token('&'); @@ -84,49 +82,47 @@ public function parse_type_and_var(string $intype, bool $getvar = true): array { $this->parse_token('...'); } if (!($this->nexttoken != null && $this->nexttoken[0] == '$')) { - throw new \Error("Error parsing type, expected variable, saw {$this->nexttoken}"); + throw new \Error("Error parsing type, expected variable, saw \"{$this->nexttoken}\"."); } $variable = $this->parse_token(); - if ($this->nextpos < strlen($intype) && !ctype_space($intype[$this->nextpos]) && $this->nexttoken != '=') { - throw new \Error("Error parsing type, no space at end of variable"); + if ($this->nextpos < strlen($text) && !(ctype_space($text[$this->nextpos]) || $this->nexttoken == '=')) { + throw new \Error("Error parsing type, no space at end of variable."); } } catch (\Error $e) { - $this->nextpos = $savednextpos; - $this->nexttoken = $savednexttoken; - $this->nextnextpos = $savednextnextpos; + list($this->nextpos, $this->nexttoken, $this->nextnextpos) = $savedstate; $variable = null; } } else { $variable = null; } - return [$outtype, $variable, trim(substr($intype, $this->nextpos))]; + return [$type, $variable, trim(substr($text, $this->nextpos))]; } /** * Compare types * - * @param ?string $widetypes the type that should be wider - * @param ?string $narrowtypes the type that should be narrower - * @return bool whether $narrowtypes has the same or narrower scope as $widetypes + * @param ?string $widetype the type that should be wider + * @param ?string $narrowtype the type that should be narrower + * @return bool whether $narrowtype has the same or narrower scope as $widetype */ - public static function compare_types(?string $widetypes, ?string $narrowtypes): bool { - if ($narrowtypes == null || $narrowtypes == '') { + public static function compare_types(?string $widetype, ?string $narrowtype): bool { + if ($narrowtype == null || $narrowtype == '') { return false; - } else if ($widetypes == null || $widetypes == '' || $widetypes == 'mixed' - || $narrowtypes == 'never') { + } else if ($widetype == null || $widetype == '' || $widetype == 'mixed' + || $narrowtype == 'never') { return true; } - $wideintersections = explode('|', $widetypes); - $narrowintersections = explode('|', $narrowtypes); + $wideintersections = explode('|', $widetype); + $narrowintersections = explode('|', $narrowtype); - // We have to match all documented intersections. + // We have to match all narrow intersections. $haveallintersections = true; foreach ($narrowintersections as $narrowintersection) { $narrowsingles = explode('&', $narrowintersection); - // If the expected types are super types, that should match. + // If the wide types are super types, that should match. $narrowadditions = []; foreach ($narrowsingles as $narrowsingle) { $supertypes = static::super_types($narrowsingle); @@ -136,7 +132,7 @@ public static function compare_types(?string $widetypes, ?string $narrowtypes): sort($narrowsingles); $narrowsingles = array_unique($narrowsingles); - // We need to look in each expected intersection. + // We need to look in each wide intersection. $havethisintersection = false; foreach ($wideintersections as $wideintersection) { $widesingles = explode('&', $wideintersection); @@ -200,11 +196,11 @@ protected function prefetch_next_token(): void { $startpos = $this->nextnextpos; // Ignore whitespace. - while ($startpos < strlen($this->type) && ctype_space($this->type[$startpos])) { + while ($startpos < strlen($this->text) && ctype_space($this->text[$startpos])) { $startpos++; } - $firstchar = ($startpos < strlen($this->type)) ? $this->type[$startpos] : null; + $firstchar = ($startpos < strlen($this->text)) ? $this->text[$startpos] : null; // Deal with different types of tokens. if ($firstchar == null) { @@ -218,32 +214,32 @@ protected function prefetch_next_token(): void { do { $havepoint = $havepoint || $nextchar == '.'; $endpos = $endpos + 1; - $nextchar = ($endpos < strlen($this->type)) ? $this->type[$endpos] : null; + $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; } while ($nextchar != null && (ctype_digit($nextchar) || $nextchar == '.' && !$havepoint)); } else if (ctype_alpha($firstchar) || $firstchar == '_' || $firstchar == '$' || $firstchar == '\\') { // Identifier token. $endpos = $startpos; do { $endpos = $endpos + 1; - $nextchar = ($endpos < strlen($this->type)) ? $this->type[$endpos] : null; + $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; } while ($nextchar != null && (ctype_alnum($nextchar) || $nextchar == '_' || $firstchar != '$' && ($nextchar == '-' || $nextchar == '\\'))); - } else if ($firstchar == '"' || $firstchar == "'") { + } else if ($firstchar == '"' || $firstchar == '\'') { // String token. - $endpos = $startpos; - $nextchar = $firstchar; - do { + $endpos = $startpos + 1; + $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; + while ($nextchar != $firstchar) { if ($nextchar == null) { - throw new \Error('Error parsing type, unterminated string'); + throw new \Error("Error parsing type, unterminated string."); } $endpos = $endpos + 1; - $lastchar = $nextchar; - $nextchar = ($endpos < strlen($this->type)) ? $this->type[$endpos] : null; - } while ($lastchar != $firstchar || $endpos == $startpos + 1); - } else if (strlen($this->type) >= $startpos + 3 && substr($this->type, $startpos, 3) == '...') { + $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; + } + $endpos++; + } else if (strlen($this->text) >= $startpos + 3 && substr($this->text, $startpos, 3) == '...') { // Splat. $endpos = $startpos + 3; - } else if (strlen($this->type) >= $startpos + 2 && substr($this->type, $startpos, 2) == '::') { + } else if (strlen($this->text) >= $startpos + 2 && substr($this->text, $startpos, 2) == '::') { // Scope resolution operator. $endpos = $startpos + 2; } else { @@ -253,7 +249,7 @@ protected function prefetch_next_token(): void { // Store token. $this->nextpos = $this->nextnextpos; - $this->nexttoken = ($endpos > $startpos) ? substr($this->type, $startpos, $endpos - $startpos) : null; + $this->nexttoken = ($endpos > $startpos) ? substr($this->text, $startpos, $endpos - $startpos) : null; $this->nextnextpos = $endpos; } @@ -269,9 +265,9 @@ protected function parse_token(?string $expect = null): string { // Check we have the expected token. if ($expect != null && $nexttoken != $expect) { - throw new \Error("Error parsing type, expected {$expect}, saw {$nexttoken}"); + throw new \Error("Error parsing type, expected \"{$expect}\", saw \"{$nexttoken}\"."); } else if ($nexttoken == null) { - throw new \Error("Error parsing type, unexpected end"); + throw new \Error("Error parsing type, unexpected end."); } // Prefetch next token. @@ -308,10 +304,10 @@ protected function parse_dnf_type(): string { // We have to figure out whether a & is for intersection or pass by reference. // Dirty hack. $nextnextpos = $this->nextnextpos; - while ($nextnextpos < strlen($this->type) && ctype_space($this->type[$nextnextpos])) { + while ($nextnextpos < strlen($this->text) && ctype_space($this->text[$nextnextpos])) { $nextnextpos++; } - $nextnextchar = ($nextnextpos < strlen($this->type)) ? $this->type[$nextnextpos] : null; + $nextnextchar = ($nextnextpos < strlen($this->text)) ? $this->text[$nextnextpos] : null; $haveintersection = $this->nexttoken == '&' && ($havebracket || !in_array($nextnextchar, ['.', '=', '$', ',', ')', null])); if ($haveintersection) { @@ -391,55 +387,53 @@ protected function parse_dnf_type(): string { */ protected function parse_single_type(): string { - // Parse name part. + // Parse base part. $nextchar = ($this->nexttoken == null) ? null : $this->nexttoken[0]; - if ($nextchar == '"' || $nextchar == "'" || $nextchar >= '0' && $nextchar <= '9' || $nextchar == '-' + if ($nextchar == '"' || $nextchar == '\'' || $nextchar >= '0' && $nextchar <= '9' || $nextchar == '-' || $nextchar == '$' || $nextchar == '\\' || $nextchar != null && ctype_alpha($nextchar) || $nextchar == '_') { - $name = $this->parse_token(); + $type = $this->parse_token(); } else { - throw new \Error("Error parsing type, expecting name, saw {$this->nexttoken}"); + throw new \Error("Error parsing type, expecting type, saw \"{$this->nexttoken}\"."); } // Parse details part. - if ($name == 'bool' || $name == 'boolean' || $name == 'true' || $name == 'false') { + if (in_array($type, ['bool', 'boolean', 'true', 'false'])) { // Parse bool details. - $name = 'bool'; - } else if ($name == 'int' || $name == 'integer' - || $name == 'positive-int' || $name == 'negative-int' - || $name == 'non-positive-int' || $name == 'non-negative-int' - || $name == 'non-zero-int' - || $name == 'int-mask' || $name == 'int-mask-of' - || ($name[0] >= '0' && $name[0] <= '9' || $name[0] == '-') && strpos($name, '.') === false) { + $type = 'bool'; + } else if (in_array($type, ['int', 'integer', + 'positive-int', 'negative-int', 'non-positive-int', 'non-negative-int', 'non-zero-int', + 'int-mask', 'int-mask-of']) + || ($type[0] >= '0' && $type[0] <= '9' || $type[0] == '-') && strpos($type, '.') === false) { // Parse int details. - if ($name == 'int' && $this->nexttoken == '<') { + if ($type == 'int' && $this->nexttoken == '<') { // Parse integer range. $this->parse_token('<'); $nexttoken = $this->nexttoken; if (!($nexttoken != null && ($nexttoken[0] >= '0' && $nexttoken[0] <= '9' || $nexttoken[0] == '-') || $nexttoken == 'min')) { - throw new \Error("Error parsing type, expected int min, saw {$nexttoken}"); + throw new \Error("Error parsing type, expected int min, saw \"{$nexttoken}\"."); }; $this->parse_token(); $this->parse_token(','); $nexttoken = $this->nexttoken; if (!($nexttoken != null && ($nexttoken[0] >= '0' && $nexttoken[0] <= '9' || $nexttoken[0] == '-') || $nexttoken == 'max')) { - throw new \Error("Error parsing type, expected int max, saw {$nexttoken}"); + throw new \Error("Error parsing type, expected int max, saw \"{$nexttoken}\"."); }; $this->parse_token(); $this->parse_token('>'); - } else if ($name == 'int-mask' || $name == 'int-mask-of') { + } else if (in_array($type, ['int-mask', 'int-mask-of'])) { // Parse integer mask. $this->parse_token('<'); $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; - if (ctype_digit($nextchar) || $name == 'int-mask') { + if (ctype_digit($nextchar) || $type == 'int-mask') { do { if (!($nextchar != null && ctype_digit($nextchar) && strpos($this->nexttoken, '.') === false)) { - throw new \Error("Error parsing type, expected int mask, saw {$this->nexttoken}"); + throw new \Error("Error parsing type, expected int mask, saw \"{$this->nexttoken}\"."); } $this->parse_token(); - $haveseperator = ($name == 'int-mask') && ($this->nexttoken == ',') - || ($name == 'int-mask-of') && ($this->nexttoken == '|'); + $haveseperator = ($type == 'int-mask') && ($this->nexttoken == ',') + || ($type == 'int-mask-of') && ($this->nexttoken == '|'); if ($haveseperator) { $this->parse_token(); } @@ -450,44 +444,38 @@ protected function parse_single_type(): string { } $this->parse_token('>'); } - $name = 'int'; - } else if ($name == 'float' || $name == 'double' - || ($name[0] >= '0' && $name[0] <= '9' || $name[0] == '-') && strpos($name, '.') !== false) { + $type = 'int'; + } else if (in_array($type, ['float', 'double']) + || ($type[0] >= '0' && $type[0] <= '9' || $type[0] == '-') && strpos($type, '.') !== false) { // Parse float details. - $name = 'float'; - } else if ($name == 'string' || $name == 'class-string' - || $name == 'numeric-string' || $name == 'non-empty-string' - || $name == 'non-falsy-string' || $name == 'truthy-string' - || $name == 'literal-string' - || $name[0] == '"' || $name[0] == "'") { + $type = 'float'; + } else if (in_array($type, ['string', 'class-string', 'numeric-string', 'literal-string', + 'non-empty-string', 'non-falsy-string', 'truthy-string']) + || $type[0] == '"' || $type[0] == '\'') { // Parse string details. - if ($name == 'class-string' && $this->nexttoken == '<') { + if ($type == 'class-string' && $this->nexttoken == '<') { $this->parse_token('<'); $classname = $this->parse_single_type(); if (!($classname[0] >= 'A' && $classname[0] <= 'Z' || $classname[0] == '_')) { - throw new \Error('Error parsing type, class string type isn\'t class name'); + throw new \Error("Error parsing type, class string type isn't class name."); } $this->parse_token('>'); } - $name = 'string'; - } else if ($name == 'callable-string') { + $type = 'string'; + } else if ($type == 'callable-string') { // Parse callable-string details. - $name = 'callable-string'; - } else if ($name == 'array' || $name == 'non-empty-array' - || $name == 'list' || $name == 'non-empty-list') { + $type = 'callable-string'; + } else if (in_array($type, ['array', 'non-empty-array', 'list', 'non-empty-list'])) { // Parse array details. if ($this->nexttoken == '<') { // Typed array. $this->parse_token('<'); $firsttype = $this->parse_dnf_type(); if ($this->nexttoken == ',') { - if ($name == 'list' || $name == 'non-empty-list') { - throw new \Error('Error parsing type, lists cannot have keys specified'); + if (in_array($type, ['list', 'non-empty-list'])) { + throw new \Error("Error parsing type, lists cannot have keys specified."); } $key = $firsttype; - if (!in_array($key, [null, 'int', 'string', 'callable-string', 'array-key'])) { - throw new \Error('Error parsing type, invalid array key'); - } $this->parse_token(','); $value = $this->parse_dnf_type(); } else { @@ -497,29 +485,25 @@ protected function parse_single_type(): string { $this->parse_token('>'); } else if ($this->nexttoken == '{') { // Array shape. - if ($name == 'list' || $name == 'non-empty-list') { - throw new \Error('Error parsing type, lists cannot have shapes'); + if (in_array($type, ['list', 'non-empty-list'])) { + throw new \Error("Error parsing type, lists cannot have shapes."); } $this->parse_token('{'); do { $key = null; - $savednextpos = $this->nextpos; - $savednexttoken = $this->nexttoken; - $savednextnextpos = $this->nextnextpos; + $savedstate = [$this->nextpos, $this->nexttoken, $this->nextnextpos]; try { $key = $this->parse_token(); - if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == "'" || $key[0] == '"' + if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == '\'' || $key[0] == '"' || (ctype_digit($key) || $key[0] == '-') && strpos($key, '.') === false)) { - throw new \Error('Error parsing type, invalid array key'); + throw new \Error("Error parsing type, invalid array key."); } if ($this->nexttoken == '?') { $this->parse_token('?'); } $this->parse_token(':'); } catch (\Error $e) { - $this->nextpos = $savednextpos; - $this->nexttoken = $savednexttoken; - $this->nextnextpos = $savednextnextpos; + list($this->nextpos, $this->nexttoken, $this->nextnextpos) = $savedstate; } $this->parse_dnf_type(); $havecomma = $this->nexttoken == ','; @@ -529,16 +513,16 @@ protected function parse_single_type(): string { } while ($havecomma); $this->parse_token('}'); } - $name = 'array'; - } else if ($name == 'object') { + $type = 'array'; + } else if ($type == 'object') { // Parse object details. if ($this->nexttoken == '{') { // Object shape. $this->parse_token('{'); do { $key = $this->parse_token(); - if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == "'" || $key[0] == '"')) { - throw new \Error('Error parsing type, invalid array key'); + if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == '\'' || $key[0] == '"')) { + throw new \Error("Error parsing type, invalid array key."); } if ($this->nexttoken == '?') { $this->parse_token('?'); @@ -552,26 +536,26 @@ protected function parse_single_type(): string { } while ($havecomma); $this->parse_token('}'); } - $name = 'object'; - } else if ($name == 'resource') { + $type = 'object'; + } else if ($type == 'resource') { // Parse resource details. - $name = 'resource'; - } else if ($name == 'never' || $name == 'never-return' || $name == 'never-returns' || $name == 'no-return') { + $type = 'resource'; + } else if ($type == 'never' || $type == 'never-return' || $type == 'never-returns' || $type == 'no-return') { // Parse never details. - $name = 'never'; - } else if ($name == 'void' || $name == 'null') { + $type = 'never'; + } else if (in_array($type, ['void', 'null'])) { // Parse void details. - $name = 'void'; - } else if ($name == 'self') { + $type = 'void'; + } else if ($type == 'self') { // Parse self details. - $name = 'self'; - } else if ($name == 'parent') { + $type = 'self'; + } else if ($type == 'parent') { // Parse parent details. - $name = 'parent'; - } else if ($name == 'static' || $name == '$this') { + $type = 'parent'; + } else if (in_array($type, ['static', '$this'])) { // Parse static details. - $name = 'static'; - } else if ($name == 'callable') { + $type = 'static'; + } else if ($type == 'callable') { // Parse callable details. if ($this->nexttoken == '(') { $this->parse_token('('); @@ -590,16 +574,11 @@ protected function parse_single_type(): string { } $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; if ($nextchar == '$') { - $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; - if ($nextchar == '$') { - $this->parse_token(); - } else { - throw new \Error("Error parsing type, expected var name, saw {$this->nexttoken}"); - } + $this->parse_token(); } if ($this->nexttoken != ')') { if ($splat) { - throw new \Error("Error parsing type, expected end of param list, saw {$this->nexttoken}"); + throw new \Error("Error parsing type, expected end of param list, saw \"{$this->nexttoken}\"."); } $this->parse_token(','); } @@ -617,52 +596,53 @@ protected function parse_single_type(): string { $this->parse_single_type(); } } - $name = 'callable'; - } else if ($name == 'mixed') { + $type = 'callable'; + } else if ($type == 'mixed') { // Parse mixed details. - $name = 'mixed'; - } else if ($name == 'iterable') { + $type = 'mixed'; + } else if ($type == 'iterable') { // Parse iterable details (Traversable|array). if ($this->nexttoken == '<') { $this->parse_token('<'); $this->parse_dnf_type(); $this->parse_token('>'); } - $name = 'iterable'; - } else if ($name == 'array-key') { + $type = 'iterable'; + } else if ($type == 'array-key') { // Parse array-key details (int|string). - $name = 'array-key'; - } else if ($name == 'scalar') { + $type = 'array-key'; + } else if ($type == 'scalar') { // Parse scalar details (bool|int|float|string). - $name = 'scalar'; - } else if ($name == 'key-of') { + $type = 'scalar'; + } else if ($type == 'key-of') { // Parse key-of details. $this->parse_token('<'); $this->parse_dnf_type(); $this->parse_token('>'); - $name = 'array-key'; - } else if ($name == 'value-of') { + $type = $this->gowide ? 'array-key' : 'never'; + } else if ($type == 'value-of') { // Parse value-of details. $this->parse_token('<'); $this->parse_dnf_type(); $this->parse_token('>'); - $name = 'mixed'; + $type = $this->gowide ? 'mixed' : 'never'; } else { // Check valid class name. - if (strpos($name, '$') !== false || strpos($name, '-') !== false || strpos($name, '\\\\') !== false) { - throw new \Error('Error parsing type, invalid class name'); + if (strpos($type, '$') !== false || strpos($type, '-') !== false || strpos($type, '\\\\') !== false) { + throw new \Error("Error parsing type, invalid class name."); } - $lastseperatorpos = strrpos($name, '\\'); + $lastseperatorpos = strrpos($type, '\\'); if ($lastseperatorpos !== false) { - $name = substr($name, $lastseperatorpos + 1); + $type = substr($type, $lastseperatorpos + 1); } - if ($name == '') { - throw new \Error('Error parsing type, class name has trailing slash'); + if ($type == '') { + throw new \Error("Error parsing type, class name has trailing slash."); } - $name = ucfirst($name); + $type = ucfirst($type); } - if ($this->nexttoken == '::' && ($name == 'object' || in_array('object', static::super_types($name)))) { + // Parse suffix. + if ($this->nexttoken == '::' && ($type == 'object' || in_array('object', static::super_types($type)))) { // Parse class constant. $this->parse_token('::'); $nextchar = ($this->nexttoken == null) ? null : $this->nexttoken[0]; @@ -673,15 +653,15 @@ protected function parse_single_type(): string { if ($this->nexttoken == '*' || !$haveconstantname) { $this->parse_token('*'); } - $name = 'mixed'; + $type = $this->gowide ? 'mixed' : 'never'; } else if ($this->nexttoken == '[') { // Parse array suffix. $this->parse_token('['); $this->parse_token(']'); - $name = 'array'; + $type = 'array'; } - return $name; + return $type; } } From 2d8cb4c8e3a35cbb803623d17509b7cf0b71c1cb Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Mon, 28 Aug 2023 19:18:18 +1200 Subject: [PATCH 07/26] Fix false positives --- classes/type_parser.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/classes/type_parser.php b/classes/type_parser.php index 46ae5c0..dfe0a9f 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -88,6 +88,12 @@ public function parse_type_and_var(string $text, bool $getvar = true): array { if ($this->nextpos < strlen($text) && !(ctype_space($text[$this->nextpos]) || $this->nexttoken == '=')) { throw new \Error("Error parsing type, no space at end of variable."); } + if ($this->nexttoken == '=') { + $this->parse_token('='); + if ($this->nexttoken == 'null' && $type != null) { + $type = $type . '|void'; + } + } } catch (\Error $e) { list($this->nextpos, $this->nexttoken, $this->nextnextpos) = $savedstate; $variable = null; From dffbada854dce058fce58917d76efa84a66a9c0e Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Tue, 29 Aug 2023 09:02:31 +1200 Subject: [PATCH 08/26] Fix missed error --- file.php | 4 ++-- rules/phpdocs_basic.php | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/file.php b/file.php index 3bcd15b..5aa221c 100644 --- a/file.php +++ b/file.php @@ -399,14 +399,14 @@ public function &get_functions() { $j++; } - // Get type and variable. + // Get type, variable name, and default value. $text = ''; for (; $j < count($argtokens); $j++) { $text .= $argtokens[$j][1]; } list($type, $variable, $default) = $typeparser->parse_type_and_var($text); - $function->arguments[] = array($type, $variable); + $function->arguments[] = array($type, $variable, $default); } $function->boundaries = $this->find_object_boundaries($function); $this->functions[] = $function; diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index 80fa5ea..2373486 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -433,11 +433,15 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { } else { $expectedtype = $function->arguments[$i][0]; $expectedparam = (string)$function->arguments[$i][1]; + $expecteddefault = $function->arguments[$i][2]; $documentedtype = $documentedarguments[$i][0]; $documentedparam = $documentedarguments[$i][1]; $match = ($expectedparam === $documentedparam) - && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype); + && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype) + && !($expecteddefault == 'null' + && strpos("|{$documentedtype}|", '|void|') === false + && $documentedtype != 'mixed'); } } $documentedreturns = $function->phpdocs->get_params('return', false); From 31c03f3168b6b5ebeffb0bf5b632c3c2d4d39c2f Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Tue, 29 Aug 2023 12:02:29 +1200 Subject: [PATCH 09/26] Revert "Fix missed error" This reverts commit dffbada854dce058fce58917d76efa84a66a9c0e. --- file.php | 4 ++-- rules/phpdocs_basic.php | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/file.php b/file.php index 5aa221c..3bcd15b 100644 --- a/file.php +++ b/file.php @@ -399,14 +399,14 @@ public function &get_functions() { $j++; } - // Get type, variable name, and default value. + // Get type and variable. $text = ''; for (; $j < count($argtokens); $j++) { $text .= $argtokens[$j][1]; } list($type, $variable, $default) = $typeparser->parse_type_and_var($text); - $function->arguments[] = array($type, $variable, $default); + $function->arguments[] = array($type, $variable); } $function->boundaries = $this->find_object_boundaries($function); $this->functions[] = $function; diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index 2373486..80fa5ea 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -433,15 +433,11 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { } else { $expectedtype = $function->arguments[$i][0]; $expectedparam = (string)$function->arguments[$i][1]; - $expecteddefault = $function->arguments[$i][2]; $documentedtype = $documentedarguments[$i][0]; $documentedparam = $documentedarguments[$i][1]; $match = ($expectedparam === $documentedparam) - && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype) - && !($expecteddefault == 'null' - && strpos("|{$documentedtype}|", '|void|') === false - && $documentedtype != 'mixed'); + && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype); } } $documentedreturns = $function->phpdocs->get_params('return', false); From d4b082daa744bd9f3e307d188f14751882281f69 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Tue, 29 Aug 2023 22:53:11 +1200 Subject: [PATCH 10/26] More false positives --- classes/type_parser.php | 44 +++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index dfe0a9f..73505b4 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -62,10 +62,6 @@ public function parse_type_and_var(string $text, bool $getvar = true): array { $savedstate = [$this->nextpos, $this->nexttoken, $this->nextnextpos]; try { $type = $this->parse_dnf_type(); - if ($this->nextpos < strlen($text) - && !(ctype_space($text[$this->nextpos]) || $this->nexttoken == '&' || $this->nexttoken == '...')) { - throw new \Error("Error parsing type, no space at end of type."); - } } catch (\Error $e) { list($this->nextpos, $this->nexttoken, $this->nextnextpos) = $savedstate; $type = null; @@ -108,14 +104,14 @@ public function parse_type_and_var(string $text, bool $getvar = true): array { /** * Compare types * - * @param ?string $widetype the type that should be wider - * @param ?string $narrowtype the type that should be narrower + * @param ?string $widetype the type that should be wider, e.g. PHP type + * @param ?string $narrowtype the type that should be narrower, e.g. PHPDoc type * @return bool whether $narrowtype has the same or narrower scope as $widetype */ public static function compare_types(?string $widetype, ?string $narrowtype): bool { - if ($narrowtype == null || $narrowtype == '') { + if ($narrowtype === null || $narrowtype == '') { return false; - } else if ($widetype == null || $widetype == '' || $widetype == 'mixed' + } else if ($widetype === null || $widetype == '' || $widetype == 'mixed' || $narrowtype == 'never') { return true; } @@ -432,7 +428,7 @@ protected function parse_single_type(): string { // Parse integer mask. $this->parse_token('<'); $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; - if (ctype_digit($nextchar) || $type == 'int-mask') { + if ($nextchar != null && ctype_digit($nextchar) || $type == 'int-mask') { do { if (!($nextchar != null && ctype_digit($nextchar) && strpos($this->nexttoken, '.') === false)) { throw new \Error("Error parsing type, expected int mask, saw \"{$this->nexttoken}\"."); @@ -645,8 +641,22 @@ protected function parse_single_type(): string { throw new \Error("Error parsing type, class name has trailing slash."); } $type = ucfirst($type); - } + if ($this->nexttoken == '<') { + // Collection. + $this->parse_token('<'); + $firsttype = $this->parse_dnf_type(); + if ($this->nexttoken == ',') { + $key = $firsttype; + $this->parse_token(','); + $value = $this->parse_dnf_type(); + } else { + $key = null; + $value = $firsttype; + } + $this->parse_token('>'); + } + } // Parse suffix. if ($this->nexttoken == '::' && ($type == 'object' || in_array('object', static::super_types($type)))) { // Parse class constant. @@ -660,11 +670,15 @@ protected function parse_single_type(): string { $this->parse_token('*'); } $type = $this->gowide ? 'mixed' : 'never'; - } else if ($this->nexttoken == '[') { - // Parse array suffix. - $this->parse_token('['); - $this->parse_token(']'); - $type = 'array'; + } else { + while ($this->nexttoken == '[' + && ($this->text[$this->nextpos] == '[' + || $this->nextnextpos < strlen($this->text) && $this->text[$this->nextnextpos] == ']')) { + // Parse array suffix. + $this->parse_token('['); + $this->parse_token(']'); + $type = 'array'; + } } return $type; From 40e21aa2a8f35f351228768593445cb532021202 Mon Sep 17 00:00:00 2001 From: james-cnz <5689414+james-cnz@users.noreply.github.com> Date: Wed, 30 Aug 2023 13:05:41 +1200 Subject: [PATCH 11/26] Fix CI error --- tests/fixtures/phpdoc_tags_general.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/phpdoc_tags_general.php b/tests/fixtures/phpdoc_tags_general.php index 32021b9..79ae5c4 100644 --- a/tests/fixtures/phpdoc_tags_general.php +++ b/tests/fixtures/phpdoc_tags_general.php @@ -254,7 +254,7 @@ public function namespaced_parameter_type( */ public function builtin( ?\stdClass $data, - ?\core\test\something|\core\some\other_thing $moredata + \core\test\something|\core\some\other_thing|null $moredata ): \stdClass { return $user; } From 679e57b1aa163160693d8cd010c33f7ae7d6d913 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Wed, 6 Sep 2023 11:38:27 +1200 Subject: [PATCH 12/26] Fixes based on analysis, and CI checks --- classes/type_parser.php | 748 +++++++++++++----------- file.php | 18 +- rules/phpdocs_basic.php | 19 +- tests/fixtures/phpdoc_types_invalid.php | 96 +++ tests/fixtures/phpdoc_types_valid.php | 398 +++++++++++++ tests/moodlecheck_rules_test.php | 44 ++ tests/phpdocs_basic_test.php | 12 + 7 files changed, 985 insertions(+), 350 deletions(-) create mode 100644 tests/fixtures/phpdoc_types_invalid.php create mode 100644 tests/fixtures/phpdoc_types_valid.php diff --git a/classes/type_parser.php b/classes/type_parser.php index 73505b4..e33022f 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -20,99 +20,125 @@ * Type parser * * Checks that PHPDoc types are well formed, and returns a simplified version if so, or null otherwise. + * Global constants and the Collection|Type[] construct aren't supported, + * because this would require looking up type and global definitions. * * @package local_moodlecheck * @copyright 2023 Te Pūkenga – New Zealand Institute of Skills and Technology * @author James Calder - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later (or CC BY-SA v4 or later) */ class type_parser { /** @var string the text to be parsed */ protected $text; + /** @var string the text to be parsed, with case retained */ + protected $textwithcase; + /** @var bool when we encounter an unknown type, should we go wide or narrow */ protected $gowide; - /** @var non-negative-int the position of the next token */ - protected $nextpos; + /** @var object{startpos: non-negative-int, endpos: non-negative-int, text: ?non-empty-string}[] next tokens */ + protected $nexts; /** @var ?non-empty-string the next token */ - protected $nexttoken; - - /** @var non-negative-int the position after the next token */ - protected $nextnextpos; + protected $next; /** * Parse a type and possibly variable name * * @param string $text the text to parse - * @param bool $getvar whether to get variable name - * @return array{?non-empty-string, ?non-empty-string, string} the simplified type, variable, and remaining text + * @param 0|1|2|3 $getwhat what to get 0=type only 1=also var 2=also modifiers (& ...) 3=also default + * @param bool $gowide if we can't determine the type, should we assume wide (for native type) or narrow (for PHPDoc)? + * @return array{?non-empty-string, ?non-empty-string, string, bool} the simplified type, variable, remaining text, + * and whether the type is explicitly nullable */ - public function parse_type_and_var(string $text, bool $getvar = true): array { + public function parse_type_and_var(string $text, int $getwhat, bool $gowide): array { // Initialise variables. $this->text = strtolower($text); - $this->gowide = false; - $this->nextnextpos = 0; - $this->prefetch_next_token(); + $this->textwithcase = $text; + $this->gowide = $gowide; + $this->nexts = []; + $this->next = $this->next(); // Try to parse type. - $savedstate = [$this->nextpos, $this->nexttoken, $this->nextnextpos]; + $savednexts = $this->nexts; try { - $type = $this->parse_dnf_type(); - } catch (\Error $e) { - list($this->nextpos, $this->nexttoken, $this->nextnextpos) = $savedstate; + $type = $this->parse_any_type(); + $explicitnullable = strpos("|{$type}|", "|void|") !== false; // For code smell check. + if (!($this->next == null || $getwhat >= 1 + || ctype_space(substr($this->text, $this->nexts[0]->startpos - 1, 1)) + || in_array($this->next, [',', ';', ':', '.']))) { + // Code smell check. + throw new \Exception("Warning parsing type, no space after type."); + } + } catch (\Exception $e) { + $this->nexts = $savednexts; + $this->next = $this->next(); $type = null; + $explicitnullable = false; } // Try to parse variable. - if ($getvar) { - $savedstate = [$this->nextpos, $this->nexttoken, $this->nextnextpos]; + if ($getwhat >= 1) { + $savednexts = $this->nexts; try { - if ($this->nexttoken == '&') { - $this->parse_token('&'); - } - if ($this->nexttoken == '...') { - $this->parse_token('...'); + $variable = ''; + if ($getwhat >= 2) { + if ($this->next == '&') { + // Not adding this for code smell check, + // because the checker previously disallowed pass by reference & in PHPDocs, + // so adding this would be a nusiance for people who changed their PHPDocs + // to conform to the previous rules. + $this->parse_token('&'); + } + if ($this->next == '...') { + // Add to variable name for code smell check. + $variable .= $this->parse_token('...'); + } } - if (!($this->nexttoken != null && $this->nexttoken[0] == '$')) { - throw new \Error("Error parsing type, expected variable, saw \"{$this->nexttoken}\"."); + if (!($this->next != null && $this->next[0] == '$')) { + throw new \Exception("Error parsing type, expected variable, saw \"{$this->next}\"."); } - $variable = $this->parse_token(); - if ($this->nextpos < strlen($text) && !(ctype_space($text[$this->nextpos]) || $this->nexttoken == '=')) { - throw new \Error("Error parsing type, no space at end of variable."); + $variable .= $this->next(0, true); + assert($variable != ''); + $this->parse_token(); + if (!($this->next == null || $getwhat >= 3 && $this->next == '=' + || ctype_space(substr($this->text, $this->nexts[0]->startpos - 1, 1)) + || in_array($this->next, [',', ';', ':', '.']))) { + // Code smell check. + throw new \Exception("Warning parsing type, no space after variable name."); } - if ($this->nexttoken == '=') { - $this->parse_token('='); - if ($this->nexttoken == 'null' && $type != null) { + if ($getwhat >= 3) { + if ($this->next == '=' && $this->next(1) == 'null' && $type != null) { $type = $type . '|void'; } } - } catch (\Error $e) { - list($this->nextpos, $this->nexttoken, $this->nextnextpos) = $savedstate; + } catch (\Exception $e) { + $this->nexts = $savednexts; + $this->next = $this->next(); $variable = null; } } else { $variable = null; } - return [$type, $variable, trim(substr($text, $this->nextpos))]; + return [$type, $variable, trim(substr($text, $this->nexts[0]->startpos)), $explicitnullable]; } /** * Compare types * - * @param ?string $widetype the type that should be wider, e.g. PHP type - * @param ?string $narrowtype the type that should be narrower, e.g. PHPDoc type + * @param ?non-empty-string $widetype the type that should be wider, e.g. PHP type + * @param ?non-empty-string $narrowtype the type that should be narrower, e.g. PHPDoc type * @return bool whether $narrowtype has the same or narrower scope as $widetype */ public static function compare_types(?string $widetype, ?string $narrowtype): bool { - if ($narrowtype === null || $narrowtype == '') { + if ($narrowtype == null) { return false; - } else if ($widetype === null || $widetype == '' || $widetype == 'mixed' - || $narrowtype == 'never') { + } else if ($widetype == null || $widetype == 'mixed' || $narrowtype == 'never') { return true; } @@ -127,6 +153,7 @@ public static function compare_types(?string $widetype, ?string $narrowtype): bo // If the wide types are super types, that should match. $narrowadditions = []; foreach ($narrowsingles as $narrowsingle) { + assert($narrowsingle != ''); $supertypes = static::super_types($narrowsingle); $narrowadditions = array_merge($narrowadditions, $supertypes); } @@ -165,13 +192,11 @@ public static function compare_types(?string $widetype, ?string $narrowtype): bo /** * Get super types * - * @param string $basetype + * @param non-empty-string $basetype * @return non-empty-string[] super types */ protected static function super_types(string $basetype): array { - if ($basetype == 'int') { - $supertypes = ['array-key', 'float', 'scalar']; - } else if ($basetype == 'string') { + if (in_array($basetype, ['int', 'string'])) { $supertypes = ['array-key', 'scaler']; } else if ($basetype == 'callable-string') { $supertypes = ['callable', 'string', 'array-key', 'scalar']; @@ -181,8 +206,10 @@ protected static function super_types(string $basetype): array { $supertypes = ['iterable']; } else if ($basetype == 'Traversable') { $supertypes = ['iterable', 'object']; + } else if ($basetype == 'Closure') { + $supertypes = ['callable', 'object']; } else if (in_array($basetype, ['self', 'parent', 'static']) - || $basetype[0] >= 'A' && $basetype[0] <= 'Z' || $basetype[0] == '_') { + || ctype_upper($basetype[0]) || $basetype[0] == '_') { $supertypes = ['object']; } else { $supertypes = []; @@ -192,168 +219,203 @@ protected static function super_types(string $basetype): array { /** * Prefetch next token + * + * @param non-negative-int $lookahead + * @param bool $getcase + * @return ?non-empty-string */ - protected function prefetch_next_token(): void { + protected function next(int $lookahead = 0, bool $getcase = false): ?string { - $startpos = $this->nextnextpos; + // Fetch any more tokens we need. + while (count($this->nexts) < $lookahead + 1) { - // Ignore whitespace. - while ($startpos < strlen($this->text) && ctype_space($this->text[$startpos])) { - $startpos++; - } + $startpos = $this->nexts ? end($this->nexts)->endpos : 0; - $firstchar = ($startpos < strlen($this->text)) ? $this->text[$startpos] : null; - - // Deal with different types of tokens. - if ($firstchar == null) { - // No more tokens. - $endpos = $startpos; - } else if (ctype_digit($firstchar) || $firstchar == '-') { - // Number token. - $nextchar = $firstchar; - $havepoint = false; - $endpos = $startpos; - do { - $havepoint = $havepoint || $nextchar == '.'; - $endpos = $endpos + 1; - $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; - } while ($nextchar != null && (ctype_digit($nextchar) || $nextchar == '.' && !$havepoint)); - } else if (ctype_alpha($firstchar) || $firstchar == '_' || $firstchar == '$' || $firstchar == '\\') { - // Identifier token. - $endpos = $startpos; - do { - $endpos = $endpos + 1; + // Ignore whitespace. + while ($startpos < strlen($this->text) && ctype_space($this->text[$startpos])) { + $startpos++; + } + + $firstchar = ($startpos < strlen($this->text)) ? $this->text[$startpos] : null; + + // Deal with different types of tokens. + if ($firstchar == null) { + // No more tokens. + $endpos = $startpos; + } else if (ctype_alpha($firstchar) || $firstchar == '_' || $firstchar == '$' || $firstchar == '\\') { + // Identifier token. + $endpos = $startpos; + do { + $endpos = $endpos + 1; + $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; + } while ($nextchar != null && (ctype_alnum($nextchar) || $nextchar == '_' + || $firstchar != '$' && ($nextchar == '-' || $nextchar == '\\'))); + } else if (ctype_digit($firstchar) || $firstchar == '-' + || $firstchar == '.' && strlen($this->text) >= $startpos + 2 && ctype_digit($this->text[$startpos + 1])) { + // Number token. + $nextchar = $firstchar; + $havepoint = false; + $endpos = $startpos; + do { + $havepoint = $havepoint || $nextchar == '.'; + $endpos = $endpos + 1; + $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; + } while ($nextchar != null && (ctype_digit($nextchar) || $nextchar == '.' && !$havepoint)); + } else if ($firstchar == '"' || $firstchar == '\'') { + // String token. + $endpos = $startpos + 1; $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; - } while ($nextchar != null && (ctype_alnum($nextchar) || $nextchar == '_' - || $firstchar != '$' && ($nextchar == '-' || $nextchar == '\\'))); - } else if ($firstchar == '"' || $firstchar == '\'') { - // String token. - $endpos = $startpos + 1; - $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; - while ($nextchar != $firstchar) { - if ($nextchar == null) { - throw new \Error("Error parsing type, unterminated string."); + while ($nextchar != $firstchar && $nextchar != null) { // There may be apostropes in comments. + $endpos = $endpos + 1; + $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; } - $endpos = $endpos + 1; - $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; + if ($nextchar != null) { + $endpos++; + } + } else if (strlen($this->text) >= $startpos + 3 && substr($this->text, $startpos, 3) == '...') { + // Splat. + $endpos = $startpos + 3; + } else if (strlen($this->text) >= $startpos + 2 && substr($this->text, $startpos, 2) == '::') { + // Scope resolution operator. + $endpos = $startpos + 2; + } else { + // Other symbol token. + $endpos = $startpos + 1; } - $endpos++; - } else if (strlen($this->text) >= $startpos + 3 && substr($this->text, $startpos, 3) == '...') { - // Splat. - $endpos = $startpos + 3; - } else if (strlen($this->text) >= $startpos + 2 && substr($this->text, $startpos, 2) == '::') { - // Scope resolution operator. - $endpos = $startpos + 2; - } else { - // Other symbol token. - $endpos = $startpos + 1; + + // Store token. + $next = substr($this->text, $startpos, $endpos - $startpos); + assert ($next !== false); + if (strlen($next) > 0 && in_array($next, ['"', '\'']) + && (strlen($next) < 2 || !in_array(substr($next, -1), ['"', '\'']))) { + // If we have an unterminated string, we've reached the end of usable tokens. + $next = ''; + } + $this->nexts[] = (object)['startpos' => $startpos, 'endpos' => $endpos, + 'text' => ($next !== '') ? $next : null]; } - // Store token. - $this->nextpos = $this->nextnextpos; - $this->nexttoken = ($endpos > $startpos) ? substr($this->text, $startpos, $endpos - $startpos) : null; - $this->nextnextpos = $endpos; + // Return the needed token. + if ($getcase) { + $next = substr($this->textwithcase, $this->nexts[$lookahead]->startpos, + $this->nexts[$lookahead]->endpos - $this->nexts[$lookahead]->startpos); + assert($next !== false); + return ($next !== '') ? $next : null; + } else { + return $this->nexts[$lookahead]->text; + } } /** * Fetch the next token * - * @param ?string $expect the expected text + * @param ?non-empty-string $expect the expected text * @return non-empty-string */ protected function parse_token(?string $expect = null): string { - $nexttoken = $this->nexttoken; + $next = $this->next; // Check we have the expected token. - if ($expect != null && $nexttoken != $expect) { - throw new \Error("Error parsing type, expected \"{$expect}\", saw \"{$nexttoken}\"."); - } else if ($nexttoken == null) { - throw new \Error("Error parsing type, unexpected end."); + if ($expect != null && $next != $expect) { + throw new \Exception("Error parsing type, expected \"{$expect}\", saw \"{$next}\"."); + } else if ($next == null) { + throw new \Exception("Error parsing type, unexpected end."); } // Prefetch next token. - $this->prefetch_next_token(); + $this->next(1); // Return consumed token. - return $nexttoken; + array_shift($this->nexts); + $this->next = $this->next(); + return $next; } /** - * Parse a list of types seperated by | and/or &, or a single nullable type + * Parse a list of types seperated by | and/or &, single nullable type, or conditional return type * + * @param bool $inbrackets are we immediately inside brackets? * @return non-empty-string the simplified type */ - protected function parse_dnf_type(): string { - $uniontypes = []; + protected function parse_any_type(bool $inbrackets = false): string { - if ($this->nexttoken == '?') { - // Parse single nullable type. + if ($inbrackets && $this->next !== null && $this->next[0] == '$' && $this->next(1) == 'is') { + // Conditional return type. + $this->parse_token(); + $this->parse_token('is'); + $this->parse_any_type(); + $this->parse_token('?'); + $firsttype = $this->parse_any_type(); + $this->parse_token(':'); + $secondtype = $this->parse_any_type(); + $uniontypes = array_merge(explode('|', $firsttype), explode('|', $secondtype)); + } else if ($this->next == '?') { + // Single nullable type. $this->parse_token('?'); - array_push($uniontypes, 'void'); - array_push($uniontypes, $this->parse_single_type()); + $uniontypes = explode('|', $this->parse_single_type()); + $uniontypes[] = 'void'; } else { - // Parse union list. + // Union list. + $uniontypes = []; do { - // Parse intersection list. - $havebracket = $this->nexttoken == '('; - if ($havebracket) { - $this->parse_token('('); - } + // Intersection list. + $unioninstead = null; $intersectiontypes = []; do { - array_push($intersectiontypes, $this->parse_single_type()); - // We have to figure out whether a & is for intersection or pass by reference. - // Dirty hack. - $nextnextpos = $this->nextnextpos; - while ($nextnextpos < strlen($this->text) && ctype_space($this->text[$nextnextpos])) { - $nextnextpos++; + $singletype = $this->parse_single_type(); + if (strpos($singletype, '|') !== false) { + $intersectiontypes[] = $this->gowide ? 'mixed' : 'never'; + $unioninstead = $singletype; + } else { + $intersectiontypes = array_merge($intersectiontypes, explode('&', $singletype)); } - $nextnextchar = ($nextnextpos < strlen($this->text)) ? $this->text[$nextnextpos] : null; - $haveintersection = $this->nexttoken == '&' - && ($havebracket || !in_array($nextnextchar, ['.', '=', '$', ',', ')', null])); - if ($haveintersection) { + // We have to figure out whether a & is for intersection or pass by reference. + $nextnext = $this->next(1); + $havemoreintersections = $this->next == '&' + && !(in_array($nextnext, ['...', '=', ',', ')', null]) + || $nextnext != null && $nextnext[0] == '$'); + if ($havemoreintersections) { $this->parse_token('&'); } - } while ($haveintersection); - if ($havebracket) { - $this->parse_token(')'); - } - // Tidy and store intersection list. - if (in_array('callable', $intersectiontypes) && in_array('string', $intersectiontypes)) { - $intersectiontypes[] = 'callable-string'; - } - foreach ($intersectiontypes as $intersectiontype) { - $supertypes = static::super_types($intersectiontype); - foreach ($supertypes as $supertype) { - $superpos = array_search($supertype, $intersectiontypes); - if ($superpos !== false) { - unset($intersectiontypes[$superpos]); + } while ($havemoreintersections); + if (count($intersectiontypes) <= 1 && $unioninstead !== null) { + $uniontypes = array_merge($uniontypes, explode('|', $unioninstead)); + } else { + // Tidy and store intersection list. + foreach ($intersectiontypes as $intersectiontype) { + assert ($intersectiontype != ''); + $supertypes = static::super_types($intersectiontype); + foreach ($supertypes as $supertype) { + $superpos = array_search($supertype, $intersectiontypes); + if ($superpos !== false) { + unset($intersectiontypes[$superpos]); + } } } + sort($intersectiontypes); + $intersectiontypes = array_unique($intersectiontypes); + $neverpos = array_search('never', $intersectiontypes); + if ($neverpos !== false) { + $intersectiontypes = ['never']; + } + $mixedpos = array_search('mixed', $intersectiontypes); + if ($mixedpos !== false && count($intersectiontypes) > 1) { + unset($intersectiontypes[$mixedpos]); + } + // TODO: Check for conflicting types, and reduce to never? + array_push($uniontypes, implode('&', $intersectiontypes)); } - sort($intersectiontypes); - $intersectiontypes = array_unique($intersectiontypes); - $neverpos = array_search('never', $intersectiontypes); - if ($neverpos !== false) { - $intersectiontypes = ['never']; - } - $mixedpos = array_search('mixed', $intersectiontypes); - if ($mixedpos !== false && count($intersectiontypes) > 1) { - unset($intersectiontypes[$mixedpos]); - } - // TODO: Check for conflicting types. - array_push($uniontypes, implode('&', $intersectiontypes)); // Check for more union items. - $haveunion = $this->nexttoken == '|'; - if ($haveunion) { + $havemoreunions = $this->next == '|'; + if ($havemoreunions) { $this->parse_token('|'); } - } while ($haveunion); + } while ($havemoreunions); } // Tidy and return union list. - if ((in_array('int', $uniontypes) || in_array('float', $uniontypes)) && in_array('string', $uniontypes)) { + if (in_array('int', $uniontypes) && in_array('string', $uniontypes)) { $uniontypes[] = 'array-key'; } if (in_array('array-key', $uniontypes) && in_array('bool', $uniontypes) && in_array('float', $uniontypes)) { @@ -362,11 +424,6 @@ protected function parse_dnf_type(): string { if (in_array('Traversable', $uniontypes) && in_array('array', $uniontypes)) { $uniontypes[] = 'iterable'; } - if (in_array('scalar', $uniontypes) && (in_array('array', $uniontypes) || in_array('iterable', $uniontypes)) - && in_array('object', $uniontypes) && in_array('resource', $uniontypes) && in_array('callable', $uniontypes) - && in_array('void', $uniontypes)) { - $uniontypes = ['mixed']; - } sort($uniontypes); $uniontypes = array_unique($uniontypes); $mixedpos = array_search('mixed', $uniontypes); @@ -377,8 +434,10 @@ protected function parse_dnf_type(): string { if ($neverpos !== false && count($uniontypes) > 1) { unset($uniontypes[$neverpos]); } - // TODO: Check for redundant types. - return implode('|', $uniontypes); + // TODO: Check for and remove redundant sub types. + $type = implode('|', $uniontypes); + assert($type != ''); + return $type; } @@ -388,127 +447,137 @@ protected function parse_dnf_type(): string { * @return non-empty-string the simplified type */ protected function parse_single_type(): string { - - // Parse base part. - $nextchar = ($this->nexttoken == null) ? null : $this->nexttoken[0]; - if ($nextchar == '"' || $nextchar == '\'' || $nextchar >= '0' && $nextchar <= '9' || $nextchar == '-' - || $nextchar == '$' || $nextchar == '\\' || $nextchar != null && ctype_alpha($nextchar) || $nextchar == '_') { - $type = $this->parse_token(); + if ($this->next == '(') { + $this->parse_token('('); + $type = $this->parse_any_type(true); + $this->parse_token(')'); } else { - throw new \Error("Error parsing type, expecting type, saw \"{$this->nexttoken}\"."); + $type = $this->parse_basic_type(); + } + while ($this->next == '[' && $this->next(1) == ']') { + // Array suffix. + $this->parse_token('['); + $this->parse_token(']'); + $type = 'array'; } + return $type; + } - // Parse details part. - if (in_array($type, ['bool', 'boolean', 'true', 'false'])) { - // Parse bool details. + /** + * Parse a basic type + * + * @return non-empty-string the simplified type + */ + protected function parse_basic_type(): string { + + $next = $this->next; + if ($next == null) { + throw new \Exception("Error parsing type, expected type, saw end"); + } + $nextchar = $next[0]; + + if (in_array($next, ['bool', 'boolean', 'true', 'false'])) { + // Bool. + $this->parse_token(); $type = 'bool'; - } else if (in_array($type, ['int', 'integer', - 'positive-int', 'negative-int', 'non-positive-int', 'non-negative-int', 'non-zero-int', + } else if (in_array($next, ['int', 'integer', 'positive-int', 'negative-int', + 'non-positive-int', 'non-negative-int', 'non-zero-int', 'int-mask', 'int-mask-of']) - || ($type[0] >= '0' && $type[0] <= '9' || $type[0] == '-') && strpos($type, '.') === false) { - // Parse int details. - if ($type == 'int' && $this->nexttoken == '<') { - // Parse integer range. + || ($nextchar >= '0' && $nextchar <= '9' || $nextchar == '-') && strpos($next, '.') === false) { + // Int. + $inttype = $this->parse_token(); + if ($inttype == 'int' && $this->next == '<') { + // Integer range. $this->parse_token('<'); - $nexttoken = $this->nexttoken; - if (!($nexttoken != null && ($nexttoken[0] >= '0' && $nexttoken[0] <= '9' || $nexttoken[0] == '-') - || $nexttoken == 'min')) { - throw new \Error("Error parsing type, expected int min, saw \"{$nexttoken}\"."); - }; - $this->parse_token(); + if ($this->next == 'min') { + $this->parse_token('min'); + } else { + $this->parse_basic_type(); + } $this->parse_token(','); - $nexttoken = $this->nexttoken; - if (!($nexttoken != null && ($nexttoken[0] >= '0' && $nexttoken[0] <= '9' || $nexttoken[0] == '-') - || $nexttoken == 'max')) { - throw new \Error("Error parsing type, expected int max, saw \"{$nexttoken}\"."); - }; - $this->parse_token(); + if ($this->next == 'max') { + $this->parse_token('max'); + } else { + $this->parse_basic_type(); + } $this->parse_token('>'); - } else if (in_array($type, ['int-mask', 'int-mask-of'])) { - // Parse integer mask. + } else if (in_array($inttype, ['int-mask', 'int-mask-of'])) { + // Integer mask. $this->parse_token('<'); - $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; - if ($nextchar != null && ctype_digit($nextchar) || $type == 'int-mask') { - do { - if (!($nextchar != null && ctype_digit($nextchar) && strpos($this->nexttoken, '.') === false)) { - throw new \Error("Error parsing type, expected int mask, saw \"{$this->nexttoken}\"."); - } + do { + $this->parse_basic_type(); + $haveseperator = ($inttype == 'int-mask') && ($this->next == ',') + || ($inttype == 'int-mask-of') && ($this->next == '|'); + if ($haveseperator) { $this->parse_token(); - $haveseperator = ($type == 'int-mask') && ($this->nexttoken == ',') - || ($type == 'int-mask-of') && ($this->nexttoken == '|'); - if ($haveseperator) { - $this->parse_token(); - } - $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; - } while ($haveseperator); - } else { - $this->parse_single_type(); - } + } + } while ($haveseperator); $this->parse_token('>'); } $type = 'int'; - } else if (in_array($type, ['float', 'double']) - || ($type[0] >= '0' && $type[0] <= '9' || $type[0] == '-') && strpos($type, '.') !== false) { - // Parse float details. + } else if (in_array($next, ['float', 'double']) + || ($nextchar >= '0' && $nextchar <= '9' || $nextchar == '-' + || $nextchar == '.' && $next != '...') && strpos($next, '.') !== false) { + // Float. + $this->parse_token(); $type = 'float'; - } else if (in_array($type, ['string', 'class-string', 'numeric-string', 'literal-string', + } else if (in_array($next, ['string', 'class-string', 'numeric-string', 'literal-string', 'non-empty-string', 'non-falsy-string', 'truthy-string']) - || $type[0] == '"' || $type[0] == '\'') { - // Parse string details. - if ($type == 'class-string' && $this->nexttoken == '<') { + || $nextchar == '"' || $nextchar == '\'') { + // String. + $strtype = $this->parse_token(); + if ($strtype == 'class-string' && $this->next == '<') { $this->parse_token('<'); - $classname = $this->parse_single_type(); - if (!($classname[0] >= 'A' && $classname[0] <= 'Z' || $classname[0] == '_')) { - throw new \Error("Error parsing type, class string type isn't class name."); + $stringtype = $this->parse_any_type(); + if (!static::compare_types('object', $stringtype)) { + throw new \Exception("Error parsing type, class-string type isn't class."); } $this->parse_token('>'); } $type = 'string'; - } else if ($type == 'callable-string') { - // Parse callable-string details. + } else if ($next == 'callable-string') { + // Callable-string. + $this->parse_token('callable-string'); $type = 'callable-string'; - } else if (in_array($type, ['array', 'non-empty-array', 'list', 'non-empty-list'])) { - // Parse array details. - if ($this->nexttoken == '<') { + } else if (in_array($next, ['array', 'non-empty-array', 'list', 'non-empty-list'])) { + // Array. + $arraytype = $this->parse_token(); + if ($this->next == '<') { // Typed array. $this->parse_token('<'); - $firsttype = $this->parse_dnf_type(); - if ($this->nexttoken == ',') { - if (in_array($type, ['list', 'non-empty-list'])) { - throw new \Error("Error parsing type, lists cannot have keys specified."); + $firsttype = $this->parse_any_type(); + if ($this->next == ',') { + if (in_array($arraytype, ['list', 'non-empty-list'])) { + throw new \Exception("Error parsing type, lists cannot have keys specified."); } $key = $firsttype; $this->parse_token(','); - $value = $this->parse_dnf_type(); + $value = $this->parse_any_type(); } else { $key = null; $value = $firsttype; } $this->parse_token('>'); - } else if ($this->nexttoken == '{') { + } else if ($this->next == '{') { // Array shape. - if (in_array($type, ['list', 'non-empty-list'])) { - throw new \Error("Error parsing type, lists cannot have shapes."); + if (in_array($arraytype, ['non-empty-array', 'non-empty-list'])) { + throw new \Exception("Error parsing type, non-empty-arrays cannot have shapes."); } $this->parse_token('{'); do { - $key = null; - $savedstate = [$this->nextpos, $this->nexttoken, $this->nextnextpos]; - try { - $key = $this->parse_token(); - if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == '\'' || $key[0] == '"' - || (ctype_digit($key) || $key[0] == '-') && strpos($key, '.') === false)) { - throw new \Error("Error parsing type, invalid array key."); - } - if ($this->nexttoken == '?') { + $next = $this->next; + if ($next != null + && (ctype_alpha($next) || $next[0] == '_' || $next[0] == '\'' || $next[0] == '"' + || (ctype_digit($next) || $next[0] == '-') && strpos($next, '.') === false) + && ($this->next(1) == ':' || $this->next(1) == '?' && $this->next(2) == ':')) { + $this->parse_token(); + if ($this->next == '?') { $this->parse_token('?'); } $this->parse_token(':'); - } catch (\Error $e) { - list($this->nextpos, $this->nexttoken, $this->nextnextpos) = $savedstate; } - $this->parse_dnf_type(); - $havecomma = $this->nexttoken == ','; + $this->parse_any_type(); + $havecomma = $this->next == ','; if ($havecomma) { $this->parse_token(','); } @@ -516,22 +585,23 @@ protected function parse_single_type(): string { $this->parse_token('}'); } $type = 'array'; - } else if ($type == 'object') { - // Parse object details. - if ($this->nexttoken == '{') { + } else if ($next == 'object') { + // Object. + $this->parse_token('object'); + if ($this->next == '{') { // Object shape. $this->parse_token('{'); do { $key = $this->parse_token(); if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == '\'' || $key[0] == '"')) { - throw new \Error("Error parsing type, invalid array key."); + throw new \Exception("Error parsing type, invalid object key."); } - if ($this->nexttoken == '?') { + if ($this->next == '?') { $this->parse_token('?'); } $this->parse_token(':'); - $this->parse_dnf_type(); - $havecomma = $this->nexttoken == ','; + $this->parse_any_type(); + $havecomma = $this->next == ','; if ($havecomma) { $this->parse_token(','); } @@ -539,148 +609,158 @@ protected function parse_single_type(): string { $this->parse_token('}'); } $type = 'object'; - } else if ($type == 'resource') { - // Parse resource details. + } else if ($next == 'resource') { + // Resource. + $this->parse_token('resource'); $type = 'resource'; - } else if ($type == 'never' || $type == 'never-return' || $type == 'never-returns' || $type == 'no-return') { - // Parse never details. + } else if (in_array($next, ['never', 'never-return', 'never-returns', 'no-return'])) { + // Never. + $this->parse_token(); $type = 'never'; - } else if (in_array($type, ['void', 'null'])) { - // Parse void details. + } else if (in_array($next, ['void', 'null'])) { + // Void. + $this->parse_token(); $type = 'void'; - } else if ($type == 'self') { - // Parse self details. + } else if ($next == 'self') { + // Self. + $this->parse_token('self'); $type = 'self'; - } else if ($type == 'parent') { - // Parse parent details. + } else if ($next == 'parent') { + // Parent. + $this->parse_token('parent'); $type = 'parent'; - } else if (in_array($type, ['static', '$this'])) { - // Parse static details. + } else if (in_array($next, ['static', '$this'])) { + // Static. + $this->parse_token(); $type = 'static'; - } else if ($type == 'callable') { - // Parse callable details. - if ($this->nexttoken == '(') { + } else if (in_array($next, ['callable', 'closure', '\closure'])) { + // Callable. + $callabletype = $this->parse_token(); + if ($this->next == '(') { $this->parse_token('('); - $splat = false; - while ($this->nexttoken != ')') { - $this->parse_dnf_type(); - if ($this->nexttoken == '&') { + while ($this->next != ')') { + $this->parse_any_type(); + if ($this->next == '&') { $this->parse_token('&'); } - if ($this->nexttoken == '...') { + if ($this->next == '...') { $this->parse_token('...'); - $splat = true; } - if ($this->nexttoken == '=') { + if ($this->next == '=') { $this->parse_token('='); } - $nextchar = ($this->nexttoken != null) ? $this->nexttoken[0] : null; + $nextchar = ($this->next != null) ? $this->next[0] : null; if ($nextchar == '$') { $this->parse_token(); } - if ($this->nexttoken != ')') { - if ($splat) { - throw new \Error("Error parsing type, expected end of param list, saw \"{$this->nexttoken}\"."); - } + if ($this->next != ')') { $this->parse_token(','); } - }; + } $this->parse_token(')'); $this->parse_token(':'); - if ($this->nexttoken == '(') { - $this->parse_token('('); - $this->parse_dnf_type(); - $this->parse_token(')'); + if ($this->next == '?') { + $this->parse_any_type(); } else { - if ($this->nexttoken == '?') { - $this->parse_token('?'); - } $this->parse_single_type(); } } - $type = 'callable'; - } else if ($type == 'mixed') { - // Parse mixed details. + if ($callabletype == 'callable') { + $type = 'callable'; + } else { + $type = 'Closure'; + } + } else if ($next == 'mixed') { + // Mixed. + $this->parse_token('mixed'); $type = 'mixed'; - } else if ($type == 'iterable') { - // Parse iterable details (Traversable|array). - if ($this->nexttoken == '<') { + } else if ($next == 'iterable') { + // Iterable (Traversable|array). + $this->parse_token('iterable'); + if ($this->next == '<') { $this->parse_token('<'); - $this->parse_dnf_type(); + $firsttype = $this->parse_any_type(); + if ($this->next == ',') { + $key = $firsttype; + $this->parse_token(','); + $value = $this->parse_any_type(); + } else { + $key = null; + $value = $firsttype; + } $this->parse_token('>'); } $type = 'iterable'; - } else if ($type == 'array-key') { - // Parse array-key details (int|string). + } else if ($next == 'array-key') { + // Array-key (int|string). + $this->parse_token('array-key'); $type = 'array-key'; - } else if ($type == 'scalar') { - // Parse scalar details (bool|int|float|string). + } else if ($next == 'scalar') { + // Scalar can be (bool|int|float|string). + $this->parse_token('scalar'); $type = 'scalar'; - } else if ($type == 'key-of') { - // Parse key-of details. + } else if ($next == 'key-of') { + // Key-of. + $this->parse_token('key-of'); $this->parse_token('<'); - $this->parse_dnf_type(); + $this->parse_any_type(); $this->parse_token('>'); $type = $this->gowide ? 'array-key' : 'never'; - } else if ($type == 'value-of') { - // Parse value-of details. + } else if ($next == 'value-of') { + // Value-of. + $this->parse_token('value-of'); $this->parse_token('<'); - $this->parse_dnf_type(); + $this->parse_any_type(); $this->parse_token('>'); $type = $this->gowide ? 'mixed' : 'never'; - } else { - // Check valid class name. - if (strpos($type, '$') !== false || strpos($type, '-') !== false || strpos($type, '\\\\') !== false) { - throw new \Error("Error parsing type, invalid class name."); - } + } else if ((ctype_alpha($next[0]) || $next[0] == '_' || $next[0] == '\\') + && strpos($next, '-') === false && strpos($next, '\\\\') === false) { + // Class name. + $type = $this->parse_token(); $lastseperatorpos = strrpos($type, '\\'); if ($lastseperatorpos !== false) { $type = substr($type, $lastseperatorpos + 1); - } - if ($type == '') { - throw new \Error("Error parsing type, class name has trailing slash."); + if ($type == '') { + throw new \Exception("Error parsing type, class name has trailing slash."); + } } $type = ucfirst($type); - if ($this->nexttoken == '<') { - // Collection. + assert($type != ''); + if ($this->next == '<') { + // Collection / Traversable. $this->parse_token('<'); - $firsttype = $this->parse_dnf_type(); - if ($this->nexttoken == ',') { + $firsttype = $this->parse_any_type(); + if ($this->next == ',') { $key = $firsttype; $this->parse_token(','); - $value = $this->parse_dnf_type(); + $value = $this->parse_any_type(); } else { $key = null; $value = $firsttype; } $this->parse_token('>'); } - + } else { + throw new \Exception("Error parsing type, unrecognised type."); } - // Parse suffix. - if ($this->nexttoken == '::' && ($type == 'object' || in_array('object', static::super_types($type)))) { - // Parse class constant. + + // Suffix. + // We can't embed this in the class name section, because it could apply to relative classes. + if ($this->next == '::' && (in_array('object', static::super_types($type)))) { + // Class constant. $this->parse_token('::'); - $nextchar = ($this->nexttoken == null) ? null : $this->nexttoken[0]; + $nextchar = ($this->next == null) ? null : $this->next[0]; $haveconstantname = $nextchar != null && (ctype_alpha($nextchar) || $nextchar == '_'); if ($haveconstantname) { $this->parse_token(); } - if ($this->nexttoken == '*' || !$haveconstantname) { + if ($this->next == '*' || !$haveconstantname) { $this->parse_token('*'); } $type = $this->gowide ? 'mixed' : 'never'; - } else { - while ($this->nexttoken == '[' - && ($this->text[$this->nextpos] == '[' - || $this->nextnextpos < strlen($this->text) && $this->text[$this->nextnextpos] == ']')) { - // Parse array suffix. - $this->parse_token('['); - $this->parse_token(']'); - $type = 'array'; - } } + assert(strpos($type, '|') === false && strpos($type, '&') === false); return $type; } diff --git a/file.php b/file.php index 3bcd15b..81ef67b 100644 --- a/file.php +++ b/file.php @@ -402,11 +402,13 @@ public function &get_functions() { // Get type and variable. $text = ''; for (; $j < count($argtokens); $j++) { - $text .= $argtokens[$j][1]; + if ($argtokens[$j][0] != T_COMMENT) { + $text .= $argtokens[$j][1]; + } } - list($type, $variable, $default) = $typeparser->parse_type_and_var($text); + list($type, $variable, $default, $nullable) = $typeparser->parse_type_and_var($text, 3, true); - $function->arguments[] = array($type, $variable); + $function->arguments[] = array($type, $variable, $nullable); } $function->boundaries = $this->find_object_boundaries($function); $this->functions[] = $function; @@ -1329,19 +1331,19 @@ public function get_shortdescription() { * * Each element is array(typename, variablename, variabledescription) * - * @param string $tag tag name to look for. Usually param but may be var for variables - * @param bool $getvar whether to get variable name + * @param string $tag tag name to look for. Usually 'param' but may be 'var' for variables + * @param 0|1|2|3 $getwhat what to get 0=type only 1=also var 2=also modifiers (& ...) 3=also default * @return array */ - public function get_params($tag = 'param', $getvar = true) { + public function get_params(string $tag, int $getwhat) { $typeparser = new \local_moodlecheck\type_parser(); $params = array(); foreach ($this->get_tags($tag) as $token) { - list($type, $variable, $description) = $typeparser->parse_type_and_var($token, $getvar); + list($type, $variable, $description) = $typeparser->parse_type_and_var($token, $getwhat, false); $param = []; $param[] = $type; - if ($getvar) { + if ($getwhat >= 1) { $param[] = $variable; } $param[] = $description; diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index 80fa5ea..24b1883 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -424,25 +424,28 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { foreach ($file->get_functions() as $function) { if ($function->phpdocs !== false) { - $documentedarguments = $function->phpdocs->get_params(); + $documentedarguments = $function->phpdocs->get_params('param', 2); $match = (count($documentedarguments) == count($function->arguments)); for ($i = 0; $match && $i < count($documentedarguments); $i++) { - if (count($documentedarguments[$i]) < 2 || $documentedarguments[$i][0] == null) { + if (count($documentedarguments[$i]) < 2 || $documentedarguments[$i][0] == null || $documentedarguments[$i][1] == null) { // Must be at least type and parameter name. $match = false; } else { $expectedtype = $function->arguments[$i][0]; $expectedparam = (string)$function->arguments[$i][1]; + $expectednullable = $function->arguments[$i][2]; $documentedtype = $documentedarguments[$i][0]; $documentedparam = $documentedarguments[$i][1]; $match = ($expectedparam === $documentedparam) - && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype); + && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype) + // Code smell check follows. + && (!$expectednullable || strpos("|{$documentedtype}|", "|void|") !== false); } } - $documentedreturns = $function->phpdocs->get_params('return', false); + $documentedreturns = $function->phpdocs->get_params('return', 0); for ($i = 0; $match && $i < count($documentedreturns); $i++) { - if (empty($documentedreturns[$i][0]) || $documentedreturns[$i][0] == 'type') { + if (empty($documentedreturns[$i][0])) { $match = false; } } @@ -464,7 +467,7 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { */ function local_moodlecheck_normalise_function_type(string $typelist): ?string { $typeparser = new \local_moodlecheck\type_parser(); - list($type, $variable, $remainder) = $typeparser->parse_type_and_var($typelist, false); + list($type, $variable, $remainder) = $typeparser->parse_type_and_var($typelist, 0, true); return $type; } @@ -478,8 +481,8 @@ function local_moodlecheck_variableshasvar(local_moodlecheck_file $file) { $errors = array(); foreach ($file->get_variables() as $variable) { if ($variable->phpdocs !== false) { - $documentedvars = $variable->phpdocs->get_params('var', false); - if (!count($documentedvars) || $documentedvars[0][0] == 'type' || $documentedvars[0][0] == null) { + $documentedvars = $variable->phpdocs->get_params('var', 0); + if (!count($documentedvars) || $documentedvars[0][0] == null) { $errors[] = array( 'line' => $variable->phpdocs->get_line_number($file, '@var'), 'variable' => $variable->fullname); diff --git a/tests/fixtures/phpdoc_types_invalid.php b/tests/fixtures/phpdoc_types_invalid.php new file mode 100644 index 0000000..5f4f7fc --- /dev/null +++ b/tests/fixtures/phpdoc_types_invalid.php @@ -0,0 +1,96 @@ +. + +// phpcs:ignoreFile + +/** + * A collection of invalid types for testing + * + * All these should fail type checking. + * Having just invalid types in here means the number of errors should match the number of type annotations. + * + * @package local_moodlecheck + * @copyright 2023 Te Pūkenga – New Zealand Institute of Skills and Technology + * @author James Calder + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later (or CC BY-SA v4 or later) + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +/** + * A collection of invalid types for testing + * + * @package local_moodlecheck + * @copyright 2023 Te Pūkenga – New Zealand Institute of Skills and Technology + * @author James Calder + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later (or CC BY-SA v4 or later) + */ +class types_invalid { + + /** + * Expecting variable name, saw end + * @param int + */ + public function expecting_var_saw_end(int $x): void { + } + + /** + * Expecting variable name, saw other + * @param int int + */ + public function expecting_var_saw_other(int $x): void { + } + + // Expecting type, saw end. + /** @var */ + public $expectingtypesawend; + + /** @var $varname Expecting type, saw other */ + public $expectingtypesawother; + + // Expecting class for class-string, saw end. + /** @var class-string< */ + public $expectingclassforclassstringsawend; + + /** @var class-string Expecting class for class-string, saw other */ + public $expectingclassforclassstringsawother; + + /** @var list List key */ + public $listkey; + + /** @var non-empty-array{'a': int} Non-empty-array shape */ + public $nonemptyarrayshape; + + /** @var object{0.0: int} Invalid object key */ + public $invalidobjectkey; + + /** + * Class name has trailing slash + * @param types_invalid/ $x + */ + public function class_name_has_trailing_slash(object $x): void { + } + + // Expecting closing bracket, saw end. + /** @var (types_invalid */ + public $expectingclosingbracketsawend; + + /** @var (types_invalid int Expecting closing bracket, saw other*/ + public $expectingclosingbracketsawother; + +} diff --git a/tests/fixtures/phpdoc_types_valid.php b/tests/fixtures/phpdoc_types_valid.php new file mode 100644 index 0000000..93ffb2f --- /dev/null +++ b/tests/fixtures/phpdoc_types_valid.php @@ -0,0 +1,398 @@ +. + +/** + * A collection of valid types for testing + * + * These should pass all relevant code checks. + * Having just valid code in here means it can be checked with another checker, such as PHPStan, + * to verify we are actually checking against correct examples. + * + * @package local_moodlecheck + * @copyright 2023 Te Pūkenga – New Zealand Institute of Skills and Technology + * @author James Calder + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later (or CC BY-SA v4 or later) + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +/** + * A parent class + */ +class types_valid_parent { +} + +/** + * An interface + */ +interface types_valid_interface { +} + +/** + * A collection of valid types for testing + * + * @package local_moodlecheck + * @copyright 2023 Te Pūkenga – New Zealand Institute of Skills and Technology + * @author James Calder + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later (or CC BY-SA v4 or later) + */ +class types_valid extends types_valid_parent { + + /** @var array */ + public const ARRAY_CONST = [ 1 => 'one', 2 => 'two' ]; + /** @var int */ + public const INT_ONE = 1; + /** @var int */ + public const INT_TWO = 2; + /** @var float */ + public const FLOAT_1_0 = 1.0; + /** @var float */ + public const FLOAT_2_0 = 2.0; + /** @var string */ + public const STRING_HELLO = "Hello"; + /** @var string */ + public const STRING_WORLD = "World"; + /** @var bool */ + public const BOOL_FALSE = false; + /** @var bool */ + public const BOOL_TRUE = true; + + + /** + * Basic type equivalence + * @param bool $bool + * @param int $int + * @param float $float + * @param string $string + * @param object $object + * @param self $self + * @param parent $parent + * @param types_valid $specificclass + * @param callable $callable + * @return void + */ + public function basic_type_equivalence( + bool $bool, + int $int, + float $float, + string $string, + object $object, + self $self, + parent $parent, + types_valid $specificclass, + callable $callable + ): void { + } + + /** + * Types not supported natively (as of PHP 7.2) + * @param array $parameterisedarray + * @param resource $resource + * @param static $static + * @param iterable $parameterisediterable + * @param array-key $arraykey + * @param scalar $scalar + * @param mixed $mixed + * @return never + */ + public function non_native_types($parameterisedarray, $resource, $static, $parameterisediterable, + $arraykey, $scalar, $mixed): void { + throw new \Exception(); + } + + /** + * Parameter modifiers + * @param object &$reference + * @param int ...$splat + */ + public function parameter_modifiers( + object &$reference, + int ...$splat): void { + } + + /** + * Boolean types + * @param bool|boolean $bool + * @param true|false $literal + */ + public function boolean_types(bool $bool, bool $literal): void { + } + + /** + * Integer types + * @param int|integer $int + * @param positive-int|negative-int|non-positive-int|non-negative-int|non-zero-int $intrange1 + * @param int<0, 100>|int|int<50, max>|int<-100, max> $intrange2 + * @param 234|-234 $literal + * @param int-mask<1, 2, 4>|int-mask-of<1|2|4> $intmask1 + */ + public function integer_types(int $int, int $intrange1, int $intrange2, + int $literal, int $intmask1): void { + } + + /** + * Integer types complex + * @param int $intrange3 + * @param int-mask|int-mask> $intmask2 + * @param int-mask-of|int-mask-of> $intmask3 + */ + public function integer_types_complex(int $intrange3, int $intmask2, int $intmask3): void { + } + + /** + * Float types + * @param float|double $float + * @param 1.0|-1.0|.1|-.1 $literal + */ + public function float_types(float $float, float $literal): void { + } + + /** + * String types + * @param string $string + * @param class-string|class-string|class-string $classstring1 + * @param callable-string|numeric-string|non-empty-string|non-falsy-string|truthy-string|literal-string $other + * @param 'foo'|'bar' $literal + */ + public function string_types(string $string, string $classstring1, string $other, string $literal): void { + } + + /** + * String types complex + * @param class-string $classstring2 + */ + public function string_types_complex(string $classstring2): void { + } + + /** + * Array types + * @param types_valid[]|array|array $genarray1 + * @param non-empty-array|non-empty-array $genarray2 + * @param list|non-empty-list $list + * @param array{'foo': int, "bar": string}|array{'foo': int, "bar"?: string}|array{int, int} $shapes1 + * @param array{0: int, 1?: int}|array{foo: int, bar: string} $shapes2 + */ + public function array_types(array $genarray1, array $genarray2, array $list, + array $shapes1, array $shapes2): void { + } + + + /** + * Array types complex + * @param array|array<1|2, string>|array $genarray3 + */ + public function array_types_complex(array $genarray3): void { + } + + /** + * Object types + * @param object $object + * @param object{'foo': int, "bar": string}|object{'foo': int, "bar"?: string} $shapes1 + * @param object{foo: int, bar?: string} $shapes2 + * @param types_valid $class + * @param self|parent|static|$this $relative + * @param Traversable|Traversable $traversable1 + * @param \Closure|\Closure(int, int): string $closure + */ + public function object_types(object $object, object $shapes1, object $shapes2, object $class, + object $relative, object $traversable1, object $closure): void { + } + + /** + * Object types complex + * @param Traversable<1|2, types_valid|types_valid_interface>|Traversable $traversable2 + */ + public function object_types_complex(object $traversable2): void { + } + + /** + * Never type + * @return never|never-return|never-returns|no-return + */ + public function never_type(): void { + throw new \Exception(); + } + + /** + * Void type + * @param ?int $explicitnullable + * @param ?int $implicitnullable + * @param null $standalonenull + * @return void + */ + public function void_type( + ?int $explicitnullable, + int $implicitnullable=null, + $standalonenull + ): void { + } + + /** + * User-defined type + * @param types_valid|\types_valid $class + */ + public function user_defined_type(types_valid $class): void { + } + + /** + * Callable types + * @param callable|callable(int, int): string|callable(int, int=): string $callable1 + * @param callable(int $foo, string $bar): void|callable(string &$bar): mixed $callable2 + * @param callable(float ...$floats): (int|null)|callable(float...): (int|null) $callable3 + * @param \Closure|\Closure(int, int): string $closure + * @param callable-string $callablestring + */ + public function callable_types(callable $callable1, callable $callable2, callable $callable3, + callable $closure, callable $callablestring): void { + } + + /** + * Iterable types + * @param array $array + * @param iterable|iterable $iterable1 + * @param Traversable|Traversable $traversable1 + */ + public function iterable_types(iterable $array, iterable $iterable1, iterable $traversable1): void { + } + + /** + * Iterable types complex + * @param iterable<1|2, types_valid>|iterable $iterable2 + * @param Traversable<1|2, types_valid>|Traversable $traversable2 + */ + public function iterable_types_complex(iterable $iterable2, iterable $traversable2): void { + } + + /** + * Key and value of + * @param key-of $keyof1 + * @param value-of $valueof1 + */ + public function key_and_value_of(int $keyof1, string $valueof1): void { + } + + /** + * Key and valud of complex + * @param key-of> $keyof2 + * @param value-of> $valueof2 + */ + public function key_and_value_of_complex(int $keyof2, string $valueof2): void { + } + + /** + * Conditional return types + * @param int $size + * @return ($size is positive-int ? non-empty-array : array) + */ + public function conditional_return(int $size): array { + return ($size > 0) ? array_fill(0, $size, "entry") : []; + } + + /** + * Conditional return types complex 1 + * @param types_valid::INT_*|types_valid::STRING_* $x + * @return ($x is types_valid::INT_* ? types_valid::INT_* : types_valid::STRING_*) + */ + public function conditional_return_complex_1($x) { + return $x; + } + + /** + * Conditional return types complex 2 + * @param 1|2|'Hello'|'World' $x + * @return ($x is 1|2 ? 1|2 : 'Hello'|'World') + */ + public function conditional_return_complex_2($x) { + return $x; + } + + /** + * Constant enumerations + * @param types_valid::BOOL_FALSE|types_valid::BOOL_TRUE|types_valid::BOOL_* $bool + * @param types_valid::INT_ONE $int1 + * @param types_valid::INT_ONE|types_valid::INT_TWO $int2 + * @param self::INT_* $int3 + * @param types_valid::* $mixed + * @param types_valid::FLOAT_1_0|types_valid::FLOAT_2_0 $float + * @param types_valid::STRING_HELLO $string + * @param types_valid::ARRAY_CONST $array + */ + public function constant_enumerations(bool $bool, int $int1, int $int2, int $int3, $mixed, + float $float, string $string, array $array): void { + } + + /** + * Basic structure + * @param ?int $nullable + * @param int|string $union + * @param types_valid&object{additionalproperty: string} $intersection + * @param (int) $brackets + * @param int[] $arraysuffix + + */ + public function basic_structure( + ?int $nullable, + $union, + object $intersection, + int $brackets, + array $arraysuffix + ): void { + } + + /** + * Structure combinations + * @param int|float|string $multipleunion + * @param types_valid&object{additionalproperty: string}&\Traversable $multipleintersection + * @param ((int)) $multiplebracket + * @param int[][] $multiplearray + * @param ?(int) $nullablebracket1 + * @param (?int) $nullablebracket2 + * @param ?int[] $nullablearray + * @param (int|float) $unionbracket1 + * @param int|(float) $unionbracket2 + * @param int|int[] $unionarray + * @param (types_valid&object{additionalproperty: string}) $intersectionbracket1 + * @param types_valid&(object{additionalproperty: string}) $intersectionbracket2 + * @param array&object{additionalproperty: string}[] $intersectionarray + * @param (int)[] $bracketarray1 + * @param (int[]) $bracketarray2 + * @param int|(types_valid&object{additionalproperty: string}) $dnf + * @param array-key&(int|string) $nondnf + */ + public function structure_combos( + $multipleunion, + object $multipleintersection, + int $multiplebracket, + array $multiplearray, + ?int $nullablebracket1, + ?int $nullablebracket2, + ?array $nullablearray, + $unionbracket1, + $unionbracket2, + $unionarray, + object $intersectionbracket1, + object $intersectionbracket2, + array $intersectionarray, + array $bracketarray1, + array $bracketarray2, + $dnf, + $nondnf + ): void { + } + +} diff --git a/tests/moodlecheck_rules_test.php b/tests/moodlecheck_rules_test.php index 697544c..cc306a9 100644 --- a/tests/moodlecheck_rules_test.php +++ b/tests/moodlecheck_rules_test.php @@ -708,4 +708,48 @@ public function test_html_format_errors_and_warnings() { $this->assertStringContainsString('11: Class someclass is not documented (error)', $result); $this->assertStringContainsString('12: Function someclass::somefunc is not documented (warning)', $result); } + + /** + * Verify valid types pass checks + * + * @covers \local_moodlecheck\type_parser + */ + public function test_phpdoc_types_valid() { + global $PAGE; + $output = $PAGE->get_renderer('local_moodlecheck'); + $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_types_valid.php', null); + $result = $output->display_path($path, 'xml'); + + // Convert results to XML Objext. + $xmlresult = new \DOMDocument(); + $xmlresult->loadXML($result); + + // Let's verify we have received a xml with the correct number of elements. + $xpath = new \DOMXpath($xmlresult); + $found = $xpath->query("//file/error"); + // TODO: Change to DOMNodeList::count() when php71 support is gone. + $this->assertSame(1, $found->length); + } + + /** + * Verify invalid types fail checks + * + * @covers \local_moodlecheck\type_parser + */ + public function test_phpdoc_types_invalid() { + global $PAGE; + $output = $PAGE->get_renderer('local_moodlecheck'); + $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_types_invalid.php', null); + $result = $output->display_path($path, 'xml'); + + // Convert results to XML Objext. + $xmlresult = new \DOMDocument(); + $xmlresult->loadXML($result); + + // Let's verify we have received a xml with the correct number of elements. + $xpath = new \DOMXpath($xmlresult); + $found = $xpath->query("//file/error"); + // TODO: Change to DOMNodeList::count() when php71 support is gone. + $this->assertSame(13, $found->length); + } } diff --git a/tests/phpdocs_basic_test.php b/tests/phpdocs_basic_test.php index 7fae63d..55f0f68 100644 --- a/tests/phpdocs_basic_test.php +++ b/tests/phpdocs_basic_test.php @@ -39,6 +39,7 @@ public static function setUpBeforeClass(): void { * @param string $inputtype The input type. * @param string $expectedtype The expected type. * @covers ::local_moodlecheck_normalise_function_type + * @covers \local_moodlecheck\type_parser */ public function test_local_moodlecheck_normalise_function_type(string $inputtype, string $expectedtype): void { $this->assertEquals( @@ -79,6 +80,17 @@ public static function local_moodlecheck_normalise_function_type_provider(): arr '?\core_course\local\some\type_of_item', 'Type_of_item|void', ], + + 'Intersection type' => [ + 'Type2&Type1', + 'Type1&Type2', + ], + + 'DNF type' => [ + 'Type3|(Type2&Type1)', + 'Type1&Type2|Type3', + ], + ]; } } From 33f994c7d273835e166d9d81c56679cf0b7814f7 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Wed, 6 Sep 2023 12:36:01 +1200 Subject: [PATCH 13/26] CI fixes --- rules/phpdocs_basic.php | 3 ++- tests/moodlecheck_rules_test.php | 4 ++-- tests/phpdocs_basic_test.php | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index 24b1883..cb7c793 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -427,7 +427,8 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { $documentedarguments = $function->phpdocs->get_params('param', 2); $match = (count($documentedarguments) == count($function->arguments)); for ($i = 0; $match && $i < count($documentedarguments); $i++) { - if (count($documentedarguments[$i]) < 2 || $documentedarguments[$i][0] == null || $documentedarguments[$i][1] == null) { + if (count($documentedarguments[$i]) < 2 || $documentedarguments[$i][0] == null + || $documentedarguments[$i][1] == null) { // Must be at least type and parameter name. $match = false; } else { diff --git a/tests/moodlecheck_rules_test.php b/tests/moodlecheck_rules_test.php index cc306a9..0030eb0 100644 --- a/tests/moodlecheck_rules_test.php +++ b/tests/moodlecheck_rules_test.php @@ -728,7 +728,7 @@ public function test_phpdoc_types_valid() { $xpath = new \DOMXpath($xmlresult); $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(1, $found->length); + $this->assertSame(2, $found->length); // No idea why this is 2 not 0. } /** @@ -750,6 +750,6 @@ public function test_phpdoc_types_invalid() { $xpath = new \DOMXpath($xmlresult); $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(13, $found->length); + $this->assertSame(14, $found->length); // No idea why this is 14 not 12. } } diff --git a/tests/phpdocs_basic_test.php b/tests/phpdocs_basic_test.php index 55f0f68..d65ebb2 100644 --- a/tests/phpdocs_basic_test.php +++ b/tests/phpdocs_basic_test.php @@ -85,7 +85,7 @@ public static function local_moodlecheck_normalise_function_type_provider(): arr 'Type2&Type1', 'Type1&Type2', ], - + 'DNF type' => [ 'Type3|(Type2&Type1)', 'Type1&Type2|Type3', From 38036e35dbd1ca540e5079f54b5e52dae408e251 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Sat, 9 Sep 2023 15:17:37 +1200 Subject: [PATCH 14/26] Support string literal escape sequences --- classes/type_parser.php | 20 ++++++++++++++------ tests/fixtures/phpdoc_types_invalid.php | 12 ++++++++++++ tests/fixtures/phpdoc_types_valid.php | 4 ++-- tests/moodlecheck_rules_test.php | 4 ++-- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index e33022f..3b8921e 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -230,6 +230,7 @@ protected function next(int $lookahead = 0, bool $getcase = false): ?string { while (count($this->nexts) < $lookahead + 1) { $startpos = $this->nexts ? end($this->nexts)->endpos : 0; + $stringunterminated = false; // Ignore whitespace. while ($startpos < strlen($this->text) && ctype_space($this->text[$startpos])) { @@ -265,12 +266,18 @@ protected function next(int $lookahead = 0, bool $getcase = false): ?string { // String token. $endpos = $startpos + 1; $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; - while ($nextchar != $firstchar && $nextchar != null) { // There may be apostropes in comments. - $endpos = $endpos + 1; + while ($nextchar != $firstchar && $nextchar != null) { // There may be unterminated strings. + if ($nextchar == '\\' && strlen($this->text) >= $endpos + 2) { + $endpos = $endpos + 2; + } else { + $endpos++; + } $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; } if ($nextchar != null) { $endpos++; + } else { + $stringunterminated = true; } } else if (strlen($this->text) >= $startpos + 3 && substr($this->text, $startpos, 3) == '...') { // Splat. @@ -286,8 +293,7 @@ protected function next(int $lookahead = 0, bool $getcase = false): ?string { // Store token. $next = substr($this->text, $startpos, $endpos - $startpos); assert ($next !== false); - if (strlen($next) > 0 && in_array($next, ['"', '\'']) - && (strlen($next) < 2 || !in_array(substr($next, -1), ['"', '\'']))) { + if ($stringunterminated) { // If we have an unterminated string, we've reached the end of usable tokens. $next = ''; } @@ -592,10 +598,12 @@ protected function parse_basic_type(): string { // Object shape. $this->parse_token('{'); do { - $key = $this->parse_token(); - if (!(ctype_alpha($key) || $key[0] == '_' || $key[0] == '\'' || $key[0] == '"')) { + $next = $this->next; + if ($next == null + || !(ctype_alpha($next) || $next[0] == '_' || $next[0] == '\'' || $next[0] == '"')) { throw new \Exception("Error parsing type, invalid object key."); } + $this->parse_token(); if ($this->next == '?') { $this->parse_token('?'); } diff --git a/tests/fixtures/phpdoc_types_invalid.php b/tests/fixtures/phpdoc_types_invalid.php index 5f4f7fc..c5b8ac5 100644 --- a/tests/fixtures/phpdoc_types_invalid.php +++ b/tests/fixtures/phpdoc_types_invalid.php @@ -63,6 +63,18 @@ public function expecting_var_saw_other(int $x): void { /** @var $varname Expecting type, saw other */ public $expectingtypesawother; + // Unterminated string. + /** @var " */ + public $unterminatedstring; + + // Unterminated string with escaped quote. + /** @var "\"*/ + public $unterminatedstringwithescapedquote; + + // String has escape with no following character. + /** @var "\*/ + public $stringhasescapewithnofollowingchar; + // Expecting class for class-string, saw end. /** @var class-string< */ public $expectingclassforclassstringsawend; diff --git a/tests/fixtures/phpdoc_types_valid.php b/tests/fixtures/phpdoc_types_valid.php index 93ffb2f..2068e62 100644 --- a/tests/fixtures/phpdoc_types_valid.php +++ b/tests/fixtures/phpdoc_types_valid.php @@ -175,8 +175,9 @@ public function string_types(string $string, string $classstring1, string $other /** * String types complex * @param class-string $classstring2 + * @param '\'' $stringwithescape */ - public function string_types_complex(string $classstring2): void { + public function string_types_complex(string $classstring2, string $stringwithescape): void { } /** @@ -191,7 +192,6 @@ public function array_types(array $genarray1, array $genarray2, array $list, array $shapes1, array $shapes2): void { } - /** * Array types complex * @param array|array<1|2, string>|array $genarray3 diff --git a/tests/moodlecheck_rules_test.php b/tests/moodlecheck_rules_test.php index 0030eb0..5200385 100644 --- a/tests/moodlecheck_rules_test.php +++ b/tests/moodlecheck_rules_test.php @@ -728,7 +728,7 @@ public function test_phpdoc_types_valid() { $xpath = new \DOMXpath($xmlresult); $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(2, $found->length); // No idea why this is 2 not 0. + $this->assertSame(2, $found->length); // No type errors, 2 complaints about package annotations. } /** @@ -750,6 +750,6 @@ public function test_phpdoc_types_invalid() { $xpath = new \DOMXpath($xmlresult); $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(14, $found->length); // No idea why this is 14 not 12. + $this->assertSame(17, $found->length); // 15 Type errors, 2 complaints about package annotations. } } From 23dd815f6b11485055816d1315607e3fb14ebf45 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Sun, 10 Sep 2023 14:42:19 +1200 Subject: [PATCH 15/26] Support number type --- classes/type_parser.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 3b8921e..3fcb233 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -196,11 +196,15 @@ public static function compare_types(?string $widetype, ?string $narrowtype): bo * @return non-empty-string[] super types */ protected static function super_types(string $basetype): array { - if (in_array($basetype, ['int', 'string'])) { + if ($basetype == 'int') { + $supertypes = ['array-key', 'number', 'scaler']; + } else if ($basetype == 'string') { $supertypes = ['array-key', 'scaler']; } else if ($basetype == 'callable-string') { $supertypes = ['callable', 'string', 'array-key', 'scalar']; - } else if (in_array($basetype, ['array-key', 'bool', 'float'])) { + } else if ($basetype == 'float') { + $supertypes = ['number', 'scalar']; + } else if (in_array($basetype, ['array-key', 'number', 'bool'])) { $supertypes = ['scalar']; } else if ($basetype == 'array') { $supertypes = ['iterable']; @@ -421,10 +425,13 @@ protected function parse_any_type(bool $inbrackets = false): string { } // Tidy and return union list. - if (in_array('int', $uniontypes) && in_array('string', $uniontypes)) { + if ((in_array('int', $uniontypes) || in_array('number', $uniontypes)) && in_array('string', $uniontypes)) { $uniontypes[] = 'array-key'; } - if (in_array('array-key', $uniontypes) && in_array('bool', $uniontypes) && in_array('float', $uniontypes)) { + if ((in_array('int', $uniontypes) || in_array('array-key', $uniontypes)) && in_array('float', $uniontypes)) { + $uniontypes[] = 'number'; + } + if (in_array('bool', $uniontypes) && in_array('number', $uniontypes) && in_array('array-key', $uniontypes)) { $uniontypes[] = 'scalar'; } if (in_array('Traversable', $uniontypes) && in_array('array', $uniontypes)) { @@ -703,6 +710,10 @@ protected function parse_basic_type(): string { // Array-key (int|string). $this->parse_token('array-key'); $type = 'array-key'; + } else if ($next == 'number') { + // Number (int|float). + $this->parse_token('number'); + $type = 'number'; } else if ($next == 'scalar') { // Scalar can be (bool|int|float|string). $this->parse_token('scalar'); From ed76dae296da1c86517e0257eb71d926cfda3dad Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Mon, 11 Sep 2023 17:01:52 +1200 Subject: [PATCH 16/26] Support underscore numeric separator --- classes/type_parser.php | 4 ++-- tests/fixtures/phpdoc_types_valid.php | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 3fcb233..dde1511 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -265,14 +265,14 @@ protected function next(int $lookahead = 0, bool $getcase = false): ?string { $havepoint = $havepoint || $nextchar == '.'; $endpos = $endpos + 1; $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; - } while ($nextchar != null && (ctype_digit($nextchar) || $nextchar == '.' && !$havepoint)); + } while ($nextchar != null && (ctype_digit($nextchar) || $nextchar == '.' && !$havepoint || $nextchar == '_')); } else if ($firstchar == '"' || $firstchar == '\'') { // String token. $endpos = $startpos + 1; $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; while ($nextchar != $firstchar && $nextchar != null) { // There may be unterminated strings. if ($nextchar == '\\' && strlen($this->text) >= $endpos + 2) { - $endpos = $endpos + 2; + $endpos = $endpos + 2; } else { $endpos++; } diff --git a/tests/fixtures/phpdoc_types_valid.php b/tests/fixtures/phpdoc_types_valid.php index 2068e62..a86a9d9 100644 --- a/tests/fixtures/phpdoc_types_valid.php +++ b/tests/fixtures/phpdoc_types_valid.php @@ -138,20 +138,21 @@ public function boolean_types(bool $bool, bool $literal): void { * @param int|integer $int * @param positive-int|negative-int|non-positive-int|non-negative-int|non-zero-int $intrange1 * @param int<0, 100>|int|int<50, max>|int<-100, max> $intrange2 - * @param 234|-234 $literal + * @param 234|-234 $literal1 * @param int-mask<1, 2, 4>|int-mask-of<1|2|4> $intmask1 */ public function integer_types(int $int, int $intrange1, int $intrange2, - int $literal, int $intmask1): void { + int $literal1, int $intmask1): void { } /** * Integer types complex * @param int $intrange3 + * @param 1_000|-1_000 $literal2 * @param int-mask|int-mask> $intmask2 * @param int-mask-of|int-mask-of> $intmask3 */ - public function integer_types_complex(int $intrange3, int $intmask2, int $intmask3): void { + public function integer_types_complex(int $intrange3, int $literal2, int $intmask2, int $intmask3): void { } /** From 66eaf09b65d52d27989ad88b8643e9f7ca171507 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Wed, 20 Sep 2023 16:31:03 +1200 Subject: [PATCH 17/26] Tighter checking --- classes/type_parser.php | 166 ++++++++++++++---------- tests/fixtures/phpdoc_types_invalid.php | 41 ++++-- tests/fixtures/phpdoc_types_valid.php | 27 ++-- tests/moodlecheck_rules_test.php | 2 +- tests/phpdocs_basic_test.php | 15 +++ 5 files changed, 160 insertions(+), 91 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index dde1511..b9bd714 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -255,8 +255,8 @@ protected function next(int $lookahead = 0, bool $getcase = false): ?string { $nextchar = ($endpos < strlen($this->text)) ? $this->text[$endpos] : null; } while ($nextchar != null && (ctype_alnum($nextchar) || $nextchar == '_' || $firstchar != '$' && ($nextchar == '-' || $nextchar == '\\'))); - } else if (ctype_digit($firstchar) || $firstchar == '-' - || $firstchar == '.' && strlen($this->text) >= $startpos + 2 && ctype_digit($this->text[$startpos + 1])) { + } else if (ctype_digit($firstchar) + || $firstchar == '-' && strlen($this->text) >= $startpos + 2 && ctype_digit($this->text[$startpos + 1])) { // Number token. $nextchar = $firstchar; $havepoint = false; @@ -389,31 +389,37 @@ protected function parse_any_type(bool $inbrackets = false): string { $this->parse_token('&'); } } while ($havemoreintersections); - if (count($intersectiontypes) <= 1 && $unioninstead !== null) { + if (count($intersectiontypes) > 1 && $unioninstead !== null) { + throw new \Exception("Error parsing type, non-DNF."); + } else if (count($intersectiontypes) <= 1 && $unioninstead !== null) { $uniontypes = array_merge($uniontypes, explode('|', $unioninstead)); } else { // Tidy and store intersection list. - foreach ($intersectiontypes as $intersectiontype) { - assert ($intersectiontype != ''); - $supertypes = static::super_types($intersectiontype); - foreach ($supertypes as $supertype) { - $superpos = array_search($supertype, $intersectiontypes); - if ($superpos !== false) { - unset($intersectiontypes[$superpos]); + if (count($intersectiontypes) > 1) { + foreach ($intersectiontypes as $intersectiontype) { + assert ($intersectiontype != ''); + $supertypes = static::super_types($intersectiontype); + if (!($intersectiontype == 'object' || in_array('object', $supertypes))) { + throw new \Exception("Error parsing type, intersection can only be used with objects."); + } + foreach ($supertypes as $supertype) { + $superpos = array_search($supertype, $intersectiontypes); + if ($superpos !== false) { + unset($intersectiontypes[$superpos]); + } } } + sort($intersectiontypes); + $intersectiontypes = array_unique($intersectiontypes); + $neverpos = array_search('never', $intersectiontypes); + if ($neverpos !== false) { + $intersectiontypes = ['never']; + } + $mixedpos = array_search('mixed', $intersectiontypes); + if ($mixedpos !== false && count($intersectiontypes) > 1) { + unset($intersectiontypes[$mixedpos]); + } } - sort($intersectiontypes); - $intersectiontypes = array_unique($intersectiontypes); - $neverpos = array_search('never', $intersectiontypes); - if ($neverpos !== false) { - $intersectiontypes = ['never']; - } - $mixedpos = array_search('mixed', $intersectiontypes); - if ($mixedpos !== false && count($intersectiontypes) > 1) { - unset($intersectiontypes[$mixedpos]); - } - // TODO: Check for conflicting types, and reduce to never? array_push($uniontypes, implode('&', $intersectiontypes)); } // Check for more union items. @@ -425,29 +431,39 @@ protected function parse_any_type(bool $inbrackets = false): string { } // Tidy and return union list. - if ((in_array('int', $uniontypes) || in_array('number', $uniontypes)) && in_array('string', $uniontypes)) { - $uniontypes[] = 'array-key'; - } - if ((in_array('int', $uniontypes) || in_array('array-key', $uniontypes)) && in_array('float', $uniontypes)) { - $uniontypes[] = 'number'; - } - if (in_array('bool', $uniontypes) && in_array('number', $uniontypes) && in_array('array-key', $uniontypes)) { - $uniontypes[] = 'scalar'; - } - if (in_array('Traversable', $uniontypes) && in_array('array', $uniontypes)) { - $uniontypes[] = 'iterable'; - } - sort($uniontypes); - $uniontypes = array_unique($uniontypes); - $mixedpos = array_search('mixed', $uniontypes); - if ($mixedpos !== false) { - $uniontypes = ['mixed']; - } - $neverpos = array_search('never', $uniontypes); - if ($neverpos !== false && count($uniontypes) > 1) { - unset($uniontypes[$neverpos]); + if (count($uniontypes) > 1) { + if ((in_array('int', $uniontypes) || in_array('number', $uniontypes)) && in_array('string', $uniontypes)) { + $uniontypes[] = 'array-key'; + } + if ((in_array('int', $uniontypes) || in_array('array-key', $uniontypes)) && in_array('float', $uniontypes)) { + $uniontypes[] = 'number'; + } + if (in_array('bool', $uniontypes) && in_array('number', $uniontypes) && in_array('array-key', $uniontypes)) { + $uniontypes[] = 'scalar'; + } + if (in_array('Traversable', $uniontypes) && in_array('array', $uniontypes)) { + $uniontypes[] = 'iterable'; + } + sort($uniontypes); + $uniontypes = array_unique($uniontypes); + $mixedpos = array_search('mixed', $uniontypes); + if ($mixedpos !== false) { + $uniontypes = ['mixed']; + } + $neverpos = array_search('never', $uniontypes); + if ($neverpos !== false && count($uniontypes) > 1) { + unset($uniontypes[$neverpos]); + } + foreach ($uniontypes as $uniontype) { + assert ($uniontype != ''); + foreach ($uniontypes as $key => $uniontype2) { + assert($uniontype2 != ''); + if ($uniontype2 != $uniontype && static::compare_types($uniontype, $uniontype2)) { + unset($uniontypes[$key]); + } + } + } } - // TODO: Check for and remove redundant sub types. $type = implode('|', $uniontypes); assert($type != ''); return $type; @@ -485,7 +501,7 @@ protected function parse_basic_type(): string { $next = $this->next; if ($next == null) { - throw new \Exception("Error parsing type, expected type, saw end"); + throw new \Exception("Error parsing type, expected type, saw end."); } $nextchar = $next[0]; @@ -494,43 +510,54 @@ protected function parse_basic_type(): string { $this->parse_token(); $type = 'bool'; } else if (in_array($next, ['int', 'integer', 'positive-int', 'negative-int', - 'non-positive-int', 'non-negative-int', 'non-zero-int', + 'non-positive-int', 'non-negative-int', 'int-mask', 'int-mask-of']) - || ($nextchar >= '0' && $nextchar <= '9' || $nextchar == '-') && strpos($next, '.') === false) { + || (ctype_digit($nextchar) || $nextchar == '-') && strpos($next, '.') === false) { // Int. $inttype = $this->parse_token(); if ($inttype == 'int' && $this->next == '<') { // Integer range. $this->parse_token('<'); - if ($this->next == 'min') { - $this->parse_token('min'); - } else { - $this->parse_basic_type(); + $next = $this->next; + if ($next == null + || !($next == 'min' || (ctype_digit($next[0]) || $next[0] == '-') && strpos($next, '.') === false)) { + throw new \Exception("Error parsing type, expected int min, saw \"{$next}\"."); } + $this->parse_token(); $this->parse_token(','); - if ($this->next == 'max') { - $this->parse_token('max'); - } else { - $this->parse_basic_type(); + $next = $this->next; + if ($next == null + || !($next == 'max' || (ctype_digit($next[0]) || $next[0] == '-') && strpos($next, '.') === false)) { + throw new \Exception("Error parsing type, expected int max, saw \"{$next}\"."); } + $this->parse_token(); $this->parse_token('>'); - } else if (in_array($inttype, ['int-mask', 'int-mask-of'])) { + } else if ($inttype == 'int-mask') { // Integer mask. $this->parse_token('<'); do { - $this->parse_basic_type(); - $haveseperator = ($inttype == 'int-mask') && ($this->next == ',') - || ($inttype == 'int-mask-of') && ($this->next == '|'); + $mask = $this->parse_basic_type(); + if (!static::compare_types('int', $mask)) { + throw new \Exception("Error parsing type, invalid int mask."); + } + $haveseperator = $this->next == ','; if ($haveseperator) { - $this->parse_token(); + $this->parse_token(','); } } while ($haveseperator); $this->parse_token('>'); + } else if ($inttype == 'int-mask-of') { + // Integer mask of. + $this->parse_token('<'); + $mask = $this->parse_basic_type(); + if (!static::compare_types('int', $mask)) { + throw new \Exception("Error parsing type, invalid int mask."); + } + $this->parse_token('>'); } $type = 'int'; } else if (in_array($next, ['float', 'double']) - || ($nextchar >= '0' && $nextchar <= '9' || $nextchar == '-' - || $nextchar == '.' && $next != '...') && strpos($next, '.') !== false) { + || (ctype_digit($nextchar) || $nextchar == '-') && strpos($next, '.') !== false) { // Float. $this->parse_token(); $type = 'float'; @@ -564,6 +591,9 @@ protected function parse_basic_type(): string { throw new \Exception("Error parsing type, lists cannot have keys specified."); } $key = $firsttype; + if (!static::compare_types('array-key', $key)) { + throw new \Exception("Error parsing type, invalid array key."); + } $this->parse_token(','); $value = $this->parse_any_type(); } else { @@ -581,7 +611,7 @@ protected function parse_basic_type(): string { $next = $this->next; if ($next != null && (ctype_alpha($next) || $next[0] == '_' || $next[0] == '\'' || $next[0] == '"' - || (ctype_digit($next) || $next[0] == '-') && strpos($next, '.') === false) + || (ctype_digit($next[0]) || $next[0] == '-') && strpos($next, '.') === false) && ($this->next(1) == ':' || $this->next(1) == '?' && $this->next(2) == ':')) { $this->parse_token(); if ($this->next == '?') { @@ -722,14 +752,20 @@ protected function parse_basic_type(): string { // Key-of. $this->parse_token('key-of'); $this->parse_token('<'); - $this->parse_any_type(); + $iterable = $this->parse_any_type(); + if (!(static::compare_types('iterable', $iterable) || static::compare_types('object', $iterable))) { + throw new \Exception("Error parsing type, can't get key of non-iterable."); + } $this->parse_token('>'); - $type = $this->gowide ? 'array-key' : 'never'; + $type = $this->gowide ? 'mixed' : 'never'; } else if ($next == 'value-of') { // Value-of. $this->parse_token('value-of'); $this->parse_token('<'); - $this->parse_any_type(); + $iterable = $this->parse_any_type(); + if (!(static::compare_types('iterable', $iterable) || static::compare_types('object', $iterable))) { + throw new \Exception("Error parsing type, can't get value of non-iterable."); + } $this->parse_token('>'); $type = $this->gowide ? 'mixed' : 'never'; } else if ((ctype_alpha($next[0]) || $next[0] == '_' || $next[0] == '\\') diff --git a/tests/fixtures/phpdoc_types_invalid.php b/tests/fixtures/phpdoc_types_invalid.php index c5b8ac5..4561948 100644 --- a/tests/fixtures/phpdoc_types_invalid.php +++ b/tests/fixtures/phpdoc_types_invalid.php @@ -19,7 +19,7 @@ /** * A collection of invalid types for testing * - * All these should fail type checking. + * Every type annotation should give an error either when checked with PHPStan or Psalm. * Having just invalid types in here means the number of errors should match the number of type annotations. * * @package local_moodlecheck @@ -30,8 +30,6 @@ defined('MOODLE_INTERNAL') || die(); -global $CFG; - /** * A collection of invalid types for testing * @@ -50,7 +48,7 @@ public function expecting_var_saw_end(int $x): void { } /** - * Expecting variable name, saw other + * Expecting variable name, saw other (passes Psalm) * @param int int */ public function expecting_var_saw_other(int $x): void { @@ -63,18 +61,36 @@ public function expecting_var_saw_other(int $x): void { /** @var $varname Expecting type, saw other */ public $expectingtypesawother; - // Unterminated string. + // Unterminated string (passes Psalm). /** @var " */ public $unterminatedstring; - // Unterminated string with escaped quote. + // Unterminated string with escaped quote (passes Psalm). /** @var "\"*/ public $unterminatedstringwithescapedquote; - // String has escape with no following character. + // String has escape with no following character (passes Psalm). /** @var "\*/ public $stringhasescapewithnofollowingchar; + /** @var array-key&(int|string) Non-DNF type (passes PHPStan) */ + public $nondnftype; + + /** @var int&string Invalid intersection */ + public $invalidintersection; + + /** @var int<0.0, 1> Invalid int min */ + public $invalidintmin; + + /** @var int<0, 1.0> Invalid int max */ + public $invalidintmax; + + /** @var int-mask<1.0, 2.0> Invalid int mask 1 */ + public $invalidintmask1; + + /** @var int-mask-of Invalid int mask 2 */ + public $invalidintmask2; + // Expecting class for class-string, saw end. /** @var class-string< */ public $expectingclassforclassstringsawend; @@ -85,12 +101,21 @@ public function expecting_var_saw_other(int $x): void { /** @var list List key */ public $listkey; + /** @var array Invalid array key (passes Psalm) */ + public $invalidarraykey; + /** @var non-empty-array{'a': int} Non-empty-array shape */ public $nonemptyarrayshape; - /** @var object{0.0: int} Invalid object key */ + /** @var object{0.0: int} Invalid object key (passes Psalm) */ public $invalidobjectkey; + /** @var key-of Can't get key of non-iterable */ + public $cantgetkeyofnoniterable; + + /** @var value-of Can't get value of non-iterable */ + public $cantgetvalueofnoniterable; + /** * Class name has trailing slash * @param types_invalid/ $x diff --git a/tests/fixtures/phpdoc_types_valid.php b/tests/fixtures/phpdoc_types_valid.php index a86a9d9..d8b3582 100644 --- a/tests/fixtures/phpdoc_types_valid.php +++ b/tests/fixtures/phpdoc_types_valid.php @@ -17,8 +17,8 @@ /** * A collection of valid types for testing * - * These should pass all relevant code checks. - * Having just valid code in here means it can be checked with another checker, such as PHPStan, + * This file should have no errors when checked with either PHPStan or Psalm. + * Having just valid code in here means it can be easily checked with other checkers, * to verify we are actually checking against correct examples. * * @package local_moodlecheck @@ -29,8 +29,6 @@ defined('MOODLE_INTERNAL') || die(); -global $CFG; - /** * A parent class */ @@ -136,10 +134,10 @@ public function boolean_types(bool $bool, bool $literal): void { /** * Integer types * @param int|integer $int - * @param positive-int|negative-int|non-positive-int|non-negative-int|non-zero-int $intrange1 + * @param positive-int|negative-int|non-positive-int|non-negative-int $intrange1 * @param int<0, 100>|int|int<50, max>|int<-100, max> $intrange2 * @param 234|-234 $literal1 - * @param int-mask<1, 2, 4>|int-mask-of<1|2|4> $intmask1 + * @param int-mask<1, 2, 4> $intmask1 */ public function integer_types(int $int, int $intrange1, int $intrange2, int $literal1, int $intmask1): void { @@ -147,18 +145,17 @@ public function integer_types(int $int, int $intrange1, int $intrange2, /** * Integer types complex - * @param int $intrange3 * @param 1_000|-1_000 $literal2 - * @param int-mask|int-mask> $intmask2 + * @param int-mask $intmask2 * @param int-mask-of|int-mask-of> $intmask3 */ - public function integer_types_complex(int $intrange3, int $literal2, int $intmask2, int $intmask3): void { + public function integer_types_complex(int $literal2, int $intmask2, int $intmask3): void { } /** * Float types * @param float|double $float - * @param 1.0|-1.0|.1|-.1 $literal + * @param 1.0|-1.0 $literal */ public function float_types(float $float, float $literal): void { } @@ -166,7 +163,7 @@ public function float_types(float $float, float $literal): void { /** * String types * @param string $string - * @param class-string|class-string|class-string $classstring1 + * @param class-string|class-string $classstring1 * @param callable-string|numeric-string|non-empty-string|non-falsy-string|truthy-string|literal-string $other * @param 'foo'|'bar' $literal */ @@ -253,7 +250,7 @@ public function user_defined_type(types_valid $class): void { /** * Callable types * @param callable|callable(int, int): string|callable(int, int=): string $callable1 - * @param callable(int $foo, string $bar): void|callable(string &$bar): mixed $callable2 + * @param callable(int $foo, string $bar): void $callable2 * @param callable(float ...$floats): (int|null)|callable(float...): (int|null) $callable3 * @param \Closure|\Closure(int, int): string $closure * @param callable-string $callablestring @@ -369,11 +366,9 @@ public function basic_structure( * @param int|int[] $unionarray * @param (types_valid&object{additionalproperty: string}) $intersectionbracket1 * @param types_valid&(object{additionalproperty: string}) $intersectionbracket2 - * @param array&object{additionalproperty: string}[] $intersectionarray * @param (int)[] $bracketarray1 * @param (int[]) $bracketarray2 * @param int|(types_valid&object{additionalproperty: string}) $dnf - * @param array-key&(int|string) $nondnf */ public function structure_combos( $multipleunion, @@ -388,11 +383,9 @@ public function structure_combos( $unionarray, object $intersectionbracket1, object $intersectionbracket2, - array $intersectionarray, array $bracketarray1, array $bracketarray2, - $dnf, - $nondnf + $dnf ): void { } diff --git a/tests/moodlecheck_rules_test.php b/tests/moodlecheck_rules_test.php index 5200385..898db04 100644 --- a/tests/moodlecheck_rules_test.php +++ b/tests/moodlecheck_rules_test.php @@ -750,6 +750,6 @@ public function test_phpdoc_types_invalid() { $xpath = new \DOMXpath($xmlresult); $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(17, $found->length); // 15 Type errors, 2 complaints about package annotations. + $this->assertSame(26, $found->length); // 24 Type errors, 2 complaints about package annotations. } } diff --git a/tests/phpdocs_basic_test.php b/tests/phpdocs_basic_test.php index d65ebb2..d74ad9b 100644 --- a/tests/phpdocs_basic_test.php +++ b/tests/phpdocs_basic_test.php @@ -91,6 +91,21 @@ public static function local_moodlecheck_normalise_function_type_provider(): arr 'Type1&Type2|Type3', ], + 'Array key' => [ + 'int|string', + 'array-key', + ], + + 'Number' => [ + 'int|float', + 'number', + ], + + 'Scalar' => [ + 'bool|int|float|string', + 'scalar', + ], + ]; } } From a1d491c6535e964b9edf2a80b00bf946adf5c83e Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Wed, 20 Sep 2023 16:44:20 +1200 Subject: [PATCH 18/26] Test case fix --- tests/phpdocs_basic_test.php | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/phpdocs_basic_test.php b/tests/phpdocs_basic_test.php index d74ad9b..aa0db7a 100644 --- a/tests/phpdocs_basic_test.php +++ b/tests/phpdocs_basic_test.php @@ -64,7 +64,7 @@ public static function local_moodlecheck_normalise_function_type_provider(): arr ], 'Unioned simple case' => [ - 'stdClass|object', 'Stdclass|object', + 'stdClass|object', 'object', ], 'Unioned fully-qualfied case' => [ @@ -82,28 +82,23 @@ public static function local_moodlecheck_normalise_function_type_provider(): arr ], 'Intersection type' => [ - 'Type2&Type1', - 'Type1&Type2', + 'Type2&Type1', 'Type1&Type2', ], 'DNF type' => [ - 'Type3|(Type2&Type1)', - 'Type1&Type2|Type3', + 'Type3|(Type2&Type1)', 'Type1&Type2|Type3', ], 'Array key' => [ - 'int|string', - 'array-key', + 'int|string', 'array-key', ], 'Number' => [ - 'int|float', - 'number', + 'int|float', 'number', ], 'Scalar' => [ - 'bool|int|float|string', - 'scalar', + 'bool|int|float|string', 'scalar', ], ]; From bfc24fa0c05ecbbcacb3a0b5668e9578542d0362 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Wed, 20 Sep 2023 18:18:04 +1200 Subject: [PATCH 19/26] Small fix --- classes/type_parser.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index b9bd714..2a6e22c 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -399,7 +399,8 @@ protected function parse_any_type(bool $inbrackets = false): string { foreach ($intersectiontypes as $intersectiontype) { assert ($intersectiontype != ''); $supertypes = static::super_types($intersectiontype); - if (!($intersectiontype == 'object' || in_array('object', $supertypes))) { + if (!(in_array($intersectiontype, ['object', 'iterable', 'callable']) + || in_array('object', $supertypes))) { throw new \Exception("Error parsing type, intersection can only be used with objects."); } foreach ($supertypes as $supertype) { From 441baf48f47cdb8b380c02af4edc0ddc338f4984 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Fri, 29 Sep 2023 15:36:02 +1300 Subject: [PATCH 20/26] Compare ret & var types, use class tree (in file) --- classes/type_parser.php | 248 +++++++++++++++++++++----- file.php | 159 +++++++++++++++-- rules/phpdocs_basic.php | 88 +++++++-- tests/fixtures/phpdoc_types_valid.php | 53 +++++- tests/phpdocs_basic_test.php | 4 - 5 files changed, 472 insertions(+), 80 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 2a6e22c..2377be8 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -20,8 +20,7 @@ * Type parser * * Checks that PHPDoc types are well formed, and returns a simplified version if so, or null otherwise. - * Global constants and the Collection|Type[] construct aren't supported, - * because this would require looking up type and global definitions. + * Global constants, templates, and the Collection|Type[] construct, aren't supported. * * @package local_moodlecheck * @copyright 2023 Te Pūkenga – New Zealand Institute of Skills and Technology @@ -30,20 +29,133 @@ */ class type_parser { + /** @var array predefined and SPL classes */ + protected $library = [ + // Predefined general. + 'Arrayaccess' => [], + 'Backedenum' => ['Unitenum'], + 'Closure' => ['callable'], + 'Directory' => [], + 'Fiber' => [], + 'Php_user_filter' => [], + 'Sensitiveparametervalue' => [], + 'Serializable' => [], + 'Stdclass' => [], + 'Stringable' => [], + 'Unitenum' => [], + 'Weakreference' => [], + // Predefined iterables. + 'Generator' => ['Iterator'], + 'Internaliterator' => ['Iterator'], + 'Iterator' => ['Traversable'], + 'Iteratoraggregate' => ['Traversable'], + 'Traversable' => ['iterable'], + 'Weakmap' => ['Arrayaccess', 'Countable', 'Iteratoraggregate'], + // Predefined throwables. + 'Arithmeticerror' => ['Error'], + 'Assertionerror' => ['Error'], + 'Compileerror' => ['Error'], + 'Divisionbyzeroerror' => ['Arithmeticerror'], + 'Error' => ['Throwable'], + 'Errorexception' => ['Exception'], + 'Exception' => ['Throwable'], + 'Parseerror' => ['Compileerror'], + 'Throwable' => ['Stringable'], + 'Typeerror' => ['Error'], + // SPL Data structures. + 'Spldoublylinkedlist' => ['Iterator', 'Countable', 'Arrayaccess', 'Serializable'], + 'Splstack' => ['Spldoublylinkedlist'], + 'Splqueue' => ['Spldoublylinkedlist'], + 'Splheap' => ['Iterator', 'Countable'], + 'Splmaxheap' => ['Splheap'], + 'Splminheap' => ['Splheap'], + 'Splpriorityqueue' => ['Iterator', 'Countable'], + 'Splfixedarray' => ['Iteratoraggregate', 'Arrayaccess', 'Countable', 'Jsonserializable'], + 'Splobjectstorage' => ['Countable', 'Iterator', 'Serializable', 'Arrayaccess'], + // SPL iterators. + 'Appenditerator' => ['Iteratoriterator'], + 'Arrayiterator' => ['Seekableiterator', 'Arrayaccess', 'Serializable', 'Countable'], + 'Cachingiterator' => ['Iteratoriterator', 'Arrayaccess', 'Countable', 'Stringable'], + 'Callbackfilteriterator' => ['Filteriterator'], + 'Directoryiterator' => ['Splfileinfo', 'Seekableiterator'], + 'Emptyiterator' => ['Iterator'], + 'Filesystemiterator' => ['Directoryiterator'], + 'Filteriterator' => ['Iteratoriterator'], + 'Globaliterator' => ['Filesystemiterator', 'Countable'], + 'Infiniteiterator' => ['Iteratoriterator'], + 'Iteratoriterator' => ['Outeriterator'], + 'Limititerator' => ['Iteratoriterator'], + 'Multipleiterator' => ['Iterator'], + 'Norewinditerator' => ['Iteratoriterator'], + 'Parentiterator' => ['Recursivefilteriterator'], + 'Recursivearrayiterator' => ['Arrayiterator', 'Recursiveiterator'], + 'Recursivecachingiterator' => ['Cachingiterator', 'Recursiveiterator'], + 'Recursivecallbackfilteriterator' => ['Callbackfilteriterator', 'Recursiveiterator'], + 'Recursivedirectoryiterator' => ['Filesystemiterator', 'Recursiveiterator'], + 'Recursivefilteriterator' => ['Filteriterator', 'Recursiveiterator'], + 'Recursiveiteratoriterator' => ['Outeriterator'], + 'Recursiveregexiterator' => ['Regexiterator', 'Recursiveiterator'], + 'Recursivetreeiterator' => ['Recursiveiteratoriterator'], + 'Regexiterator' => ['Filteriterator'], + // SPL interfaces. + 'Countable' => [], + 'Outeriterator' => ['Iterator'], + 'Recursiveiterator' => ['Iterator'], + 'Seekableiterator' => ['Iterator'], + // SPL exceptions. + 'Badfunctioncallexception' => ['Logicexception'], + 'Badmethodcallexception' => ['Badfunctioncallexception'], + 'Domainexception' => ['Logicexception'], + 'Invalidargumentexception' => ['Logicexception'], + 'Lengthexception' => ['Logicexception'], + 'Logicexception' => ['Exception'], + 'Outofboundsexception' => ['Runtimeexception'], + 'Outofrangeexception' => ['Logicexception'], + 'Overflowexception' => ['Runtimeexception'], + 'Rangeexception' => ['Runtimeexception'], + 'Runtimeexception' => ['Exception'], + 'Underflowexception' => ['Runtimeexception'], + 'Unexpectedvalueexception' => ['Runtimeexception'], + // SPL file handling. + 'Splfileinfo' => ['Stringable'], + 'Splfileobject' => ['Splfileinfo', 'Recursiveiterator', 'Seekableiterator'], + 'Spltempfileobject' => ['Splfileobject'], + // SPL misc. + 'Arrayobject' => ['Iteratoraggregate', 'Arrayaccess', 'Serializable', 'Countable'], + 'Splobserver' => [], + 'Splsubject' => [], + ]; + + /** @var array use aliases, aliases are keys, class names are values */ + protected $usealiases; + + /** @var array inheritance heirarchy */ + protected $artifacts; + /** @var string the text to be parsed */ - protected $text; + protected $text = ''; /** @var string the text to be parsed, with case retained */ - protected $textwithcase; + protected $textwithcase = ''; /** @var bool when we encounter an unknown type, should we go wide or narrow */ - protected $gowide; + protected $gowide = false; /** @var object{startpos: non-negative-int, endpos: non-negative-int, text: ?non-empty-string}[] next tokens */ - protected $nexts; + protected $nexts = []; /** @var ?non-empty-string the next token */ - protected $next; + protected $next = null; + + /** + * Constructor + * @param array $usealiases aliases are keys, class names are values + * @param array $artifacts inherit tree + */ + public function __construct(array $usealiases = [], array $artifacts = []) { + $this->usealiases = $usealiases; + $this->artifacts = $artifacts; + } /** * Parse a type and possibly variable name @@ -128,6 +240,30 @@ public function parse_type_and_var(string $text, int $getwhat, bool $gowide): ar return [$type, $variable, trim(substr($text, $this->nexts[0]->startpos)), $explicitnullable]; } + /** + * Substitute owner and parent names + * + * @param non-empty-string $type the simplified type + * @param ?non-empty-string $ownername + * @param ?non-empty-string $parentname + * @return non-empty-string + */ + public static function substitute_names(string $type, ?string $ownername, ?string $parentname): string { + if ($ownername) { + $ownername = ucfirst(strtolower($ownername)); + $type = preg_replace('/\bself\b/', $ownername, $type); + assert($type != null); + $type = preg_replace('/\bstatic\b/', "static({$ownername})", $type); + assert($type != null); + } + if ($parentname) { + $parentname = ucfirst(strtolower($parentname)); + $type = preg_replace('/\bparent\b/', $parentname, $type); + assert($type != null); + } + return $type; + } + /** * Compare types * @@ -135,7 +271,7 @@ public function parse_type_and_var(string $text, int $getwhat, bool $gowide): ar * @param ?non-empty-string $narrowtype the type that should be narrower, e.g. PHPDoc type * @return bool whether $narrowtype has the same or narrower scope as $widetype */ - public static function compare_types(?string $widetype, ?string $narrowtype): bool { + public function compare_types(?string $widetype, ?string $narrowtype): bool { if ($narrowtype == null) { return false; } else if ($widetype == null || $widetype == 'mixed' || $narrowtype == 'never') { @@ -154,7 +290,7 @@ public static function compare_types(?string $widetype, ?string $narrowtype): bo $narrowadditions = []; foreach ($narrowsingles as $narrowsingle) { assert($narrowsingle != ''); - $supertypes = static::super_types($narrowsingle); + $supertypes = $this->super_types($narrowsingle); $narrowadditions = array_merge($narrowadditions, $supertypes); } $narrowsingles = array_merge($narrowsingles, $narrowadditions); @@ -195,26 +331,56 @@ public static function compare_types(?string $widetype, ?string $narrowtype): bo * @param non-empty-string $basetype * @return non-empty-string[] super types */ - protected static function super_types(string $basetype): array { - if ($basetype == 'int') { - $supertypes = ['array-key', 'number', 'scaler']; - } else if ($basetype == 'string') { + protected function super_types(string $basetype): array { + if (in_array($basetype, ['int', 'string'])) { $supertypes = ['array-key', 'scaler']; } else if ($basetype == 'callable-string') { $supertypes = ['callable', 'string', 'array-key', 'scalar']; - } else if ($basetype == 'float') { - $supertypes = ['number', 'scalar']; - } else if (in_array($basetype, ['array-key', 'number', 'bool'])) { + } else if (in_array($basetype, ['array-key', 'float', 'bool'])) { $supertypes = ['scalar']; } else if ($basetype == 'array') { $supertypes = ['iterable']; - } else if ($basetype == 'Traversable') { - $supertypes = ['iterable', 'object']; - } else if ($basetype == 'Closure') { - $supertypes = ['callable', 'object']; - } else if (in_array($basetype, ['self', 'parent', 'static']) - || ctype_upper($basetype[0]) || $basetype[0] == '_') { + } else if ($basetype == 'static') { + $supertypes = ['self', 'parent', 'object']; + } else if ($basetype == 'self') { + $supertypes = ['parent', 'object']; + } else if ($basetype == 'parent') { $supertypes = ['object']; + } else if (strpos($basetype, 'static(') === 0 || ctype_upper($basetype[0]) || $basetype[0] == '_') { + if (strpos($basetype, 'static(') === 0) { + $supertypes = ['static', 'self', 'parent', 'object']; + $supertypequeue = [substr($basetype, 7, -1)]; + $ignore = false; + } else { + $supertypes = ['object']; + $supertypequeue = [$basetype]; + $ignore = true; + } + while ($supertype = array_shift($supertypequeue)) { + if (in_array($supertype, $supertypes)) { + $ignore = false; + continue; + } + if (!$ignore) { + $supertypes[] = $supertype; + } + if ($librarysupers = $this->library[$supertype] ?? null) { + $supertypequeue = array_merge($supertypequeue, $librarysupers); + } else if ($supertypeobj = $this->artifacts[strtolower($supertype)] ?? null) { + if ($supertypeobj->extends) { + $supertypequeue[] = ucfirst(strtolower($supertypeobj->extends)); + } + if (count($supertypeobj->implements) > 0) { + foreach ($supertypeobj->implements as $implements) { + $supertypequeue[] = ucfirst(strtolower($implements)); + } + } + } else if (!$ignore) { + $supertypes = array_merge($supertypes, $this->super_types($supertype)); + } + $ignore = false; + } + $supertypes = array_unique($supertypes); } else { $supertypes = []; } @@ -296,7 +462,7 @@ protected function next(int $lookahead = 0, bool $getcase = false): ?string { // Store token. $next = substr($this->text, $startpos, $endpos - $startpos); - assert ($next !== false); + assert($next !== false); if ($stringunterminated) { // If we have an unterminated string, we've reached the end of usable tokens. $next = ''; @@ -397,8 +563,8 @@ protected function parse_any_type(bool $inbrackets = false): string { // Tidy and store intersection list. if (count($intersectiontypes) > 1) { foreach ($intersectiontypes as $intersectiontype) { - assert ($intersectiontype != ''); - $supertypes = static::super_types($intersectiontype); + assert($intersectiontype != ''); + $supertypes = $this->super_types($intersectiontype); if (!(in_array($intersectiontype, ['object', 'iterable', 'callable']) || in_array('object', $supertypes))) { throw new \Exception("Error parsing type, intersection can only be used with objects."); @@ -433,13 +599,10 @@ protected function parse_any_type(bool $inbrackets = false): string { // Tidy and return union list. if (count($uniontypes) > 1) { - if ((in_array('int', $uniontypes) || in_array('number', $uniontypes)) && in_array('string', $uniontypes)) { + if (in_array('int', $uniontypes) && in_array('string', $uniontypes)) { $uniontypes[] = 'array-key'; } - if ((in_array('int', $uniontypes) || in_array('array-key', $uniontypes)) && in_array('float', $uniontypes)) { - $uniontypes[] = 'number'; - } - if (in_array('bool', $uniontypes) && in_array('number', $uniontypes) && in_array('array-key', $uniontypes)) { + if (in_array('bool', $uniontypes) && in_array('float', $uniontypes) && in_array('array-key', $uniontypes)) { $uniontypes[] = 'scalar'; } if (in_array('Traversable', $uniontypes) && in_array('array', $uniontypes)) { @@ -456,10 +619,10 @@ protected function parse_any_type(bool $inbrackets = false): string { unset($uniontypes[$neverpos]); } foreach ($uniontypes as $uniontype) { - assert ($uniontype != ''); + assert($uniontype != ''); foreach ($uniontypes as $key => $uniontype2) { assert($uniontype2 != ''); - if ($uniontype2 != $uniontype && static::compare_types($uniontype, $uniontype2)) { + if ($uniontype2 != $uniontype && $this->compare_types($uniontype, $uniontype2)) { unset($uniontypes[$key]); } } @@ -538,7 +701,7 @@ protected function parse_basic_type(): string { $this->parse_token('<'); do { $mask = $this->parse_basic_type(); - if (!static::compare_types('int', $mask)) { + if (!$this->compare_types('int', $mask)) { throw new \Exception("Error parsing type, invalid int mask."); } $haveseperator = $this->next == ','; @@ -551,7 +714,7 @@ protected function parse_basic_type(): string { // Integer mask of. $this->parse_token('<'); $mask = $this->parse_basic_type(); - if (!static::compare_types('int', $mask)) { + if (!$this->compare_types('int', $mask)) { throw new \Exception("Error parsing type, invalid int mask."); } $this->parse_token('>'); @@ -570,7 +733,7 @@ protected function parse_basic_type(): string { if ($strtype == 'class-string' && $this->next == '<') { $this->parse_token('<'); $stringtype = $this->parse_any_type(); - if (!static::compare_types('object', $stringtype)) { + if (!$this->compare_types('object', $stringtype)) { throw new \Exception("Error parsing type, class-string type isn't class."); } $this->parse_token('>'); @@ -592,7 +755,7 @@ protected function parse_basic_type(): string { throw new \Exception("Error parsing type, lists cannot have keys specified."); } $key = $firsttype; - if (!static::compare_types('array-key', $key)) { + if (!$this->compare_types('array-key', $key)) { throw new \Exception("Error parsing type, invalid array key."); } $this->parse_token(','); @@ -741,10 +904,6 @@ protected function parse_basic_type(): string { // Array-key (int|string). $this->parse_token('array-key'); $type = 'array-key'; - } else if ($next == 'number') { - // Number (int|float). - $this->parse_token('number'); - $type = 'number'; } else if ($next == 'scalar') { // Scalar can be (bool|int|float|string). $this->parse_token('scalar'); @@ -754,7 +913,7 @@ protected function parse_basic_type(): string { $this->parse_token('key-of'); $this->parse_token('<'); $iterable = $this->parse_any_type(); - if (!(static::compare_types('iterable', $iterable) || static::compare_types('object', $iterable))) { + if (!($this->compare_types('iterable', $iterable) || $this->compare_types('object', $iterable))) { throw new \Exception("Error parsing type, can't get key of non-iterable."); } $this->parse_token('>'); @@ -764,7 +923,7 @@ protected function parse_basic_type(): string { $this->parse_token('value-of'); $this->parse_token('<'); $iterable = $this->parse_any_type(); - if (!(static::compare_types('iterable', $iterable) || static::compare_types('object', $iterable))) { + if (!($this->compare_types('iterable', $iterable) || $this->compare_types('object', $iterable))) { throw new \Exception("Error parsing type, can't get value of non-iterable."); } $this->parse_token('>'); @@ -780,6 +939,9 @@ protected function parse_basic_type(): string { throw new \Exception("Error parsing type, class name has trailing slash."); } } + if (array_key_exists($type, $this->usealiases)) { + $type = strtolower($this->usealiases[$type]); + } $type = ucfirst($type); assert($type != ''); if ($this->next == '<') { @@ -802,7 +964,7 @@ protected function parse_basic_type(): string { // Suffix. // We can't embed this in the class name section, because it could apply to relative classes. - if ($this->next == '::' && (in_array('object', static::super_types($type)))) { + if ($this->next == '::' && (in_array('object', $this->super_types($type)))) { // Class constant. $this->parse_token('::'); $nextchar = ($this->next == null) ? null : $this->next[0]; diff --git a/file.php b/file.php index 81ef67b..dac4481 100644 --- a/file.php +++ b/file.php @@ -39,6 +39,7 @@ class local_moodlecheck_file { protected $errors = null; protected $tokens = null; protected $tokenscount = 0; + protected $usealiases = null; protected $classes = null; protected $interfaces = null; protected $traits = null; @@ -48,6 +49,7 @@ class local_moodlecheck_file { protected $variables = null; protected $defines = null; protected $constants = null; + protected $typeparser = null; /** * Creates an object from path to the file @@ -64,6 +66,7 @@ public function __construct($filepath) { protected function clear_memory() { $this->tokens = null; $this->tokenscount = 0; + $this->usealiases = null; $this->classes = null; $this->interfaces = null; $this->traits = null; @@ -73,6 +76,7 @@ protected function clear_memory() { $this->variables = null; $this->defines = null; $this->constants = null; + $this->typeparser = null; } /** @@ -189,6 +193,55 @@ public function &get_tokens() { return $this->tokens; } + /** + * Returns all use aliases + * + * @return array aliases are keys, class names are values + */ + public function get_use_aliases() { + if ($this->usealiases) { + return $this->usealiases; + } + $tokens = &$this->get_tokens(); + $usealiases = []; + $after = null; + $classname = null; + $alias = null; + for ($tid = 0; $tid < $this->tokenscount; $tid++) { + if ($tokens[$tid][0] == T_USE) { + $after = T_USE; + $classname = null; + $alias = null; + } else if ($after == T_USE && $tokens[$tid][0] == T_STRING) { + $classname = $tokens[$tid][1]; + if (strrpos($classname, '\\') !== false) { + $classname = substr($classname, strrpos($classname, '\\') + 1); + } + } else if ($after == T_USE && $tokens[$tid][0] == T_AS) { + $after = T_AS; + } else if ($after == T_AS && $tokens[$tid][0] == T_STRING) { + $alias = $tokens[$tid][1]; + } else if (($after == T_USE || $after == T_AS) && in_array($tokens[$tid][1], [',', ';'])) { + if ($after == T_AS && $classname && $alias) { + $usealiases[strtolower($alias)] = $classname; + } + if ($tokens[$tid][1] == ',') { + $after = T_USE; + } else { + $after = null; + } + $classname = null; + $alias = null; + } else if (!in_array($tokens[$tid][0], [T_WHITESPACE, T_COMMENT, T_NS_SEPARATOR])) { + $after = null; + $classname = null; + $alias = null; + } + } + $this->usealiases = $usealiases; + return $usealiases; + } + /** * Returns all artifacts (classes, interfaces, traits) found in file * @@ -200,7 +253,9 @@ public function &get_tokens() { * ->phpdocs : phpdocs for this artifact (instance of local_moodlecheck_phpdocs or false if not found) * ->boundaries : array with ids of first and last token for this artifact. * ->hasextends : boolean indicating whether this artifact has an `extends` clause + * ->extends : name of base class * ->hasimplements : boolean indicating whether this artifact has an `implements` clause + * ->implements : array of names of implemented interfaces * * @return array with 3 elements (classes, interfaces & traits), each being an array. */ @@ -267,30 +322,58 @@ public function get_artifacts() { $artifact->tagpair = $this->find_tag_pair($tid, '{', '}'); $artifact->hasextends = false; + $artifact->extends = null; $artifact->hasimplements = false; + $artifact->implements = []; if ($artifact->tagpair) { + $after = null; + $implements = null; // Iterate over the remaining tokens in the class definition (until opening {). foreach (array_slice($this->tokens, $tid, $artifact->tagpair[0] - $tid) as $token) { + if ($after == T_IMPLEMENTS && $implements + && !in_array($token[0], [T_WHITESPACE, T_COMMENT, T_NS_SEPARATOR, T_STRING])) { + $artifact->implements[] = $implements; + $implements = null; + } if ($token[0] == T_EXTENDS) { $artifact->hasextends = true; - } - if ($token[0] == T_IMPLEMENTS) { + $after = T_EXTENDS; + } else if ($after == T_EXTENDS && $token[0] == T_STRING) { + $extends = $token[1]; + if (strrpos($extends, '\\') !== false) { + $extends = substr($extends, strrpos($extends, '\\') + 1); + } + $artifact->extends = $extends; + } else if ($token[0] == T_IMPLEMENTS) { $artifact->hasimplements = true; + $after = T_IMPLEMENTS; + $implements = null; + } else if ($after == T_IMPLEMENTS && $token[0] == T_STRING) { + $implements = $token[1]; + if (strrpos($implements, '\\') !== false) { + $implements = substr($implements, strrpos($implements, '\\') + 1); + } + } else if (!in_array($token[0], [T_WHITESPACE, T_COMMENT, T_NS_SEPARATOR, T_STRING]) + && $token[1] !== ',') { + $after = null; } } + if ($after == T_IMPLEMENTS && $implements) { + $artifact->implements[] = $implements; + } } $artifact->boundaries = $this->find_object_boundaries($artifact); switch ($artifact->type) { case T_CLASS: - $this->classes[] = $artifact; + $this->classes[strtolower($artifact->name)] = $artifact; break; case T_INTERFACE: - $this->interfaces[] = $artifact; + $this->interfaces[strtolower($artifact->name)] = $artifact; break; case T_TRAIT: - $this->traits[] = $artifact; + $this->traits[strtolower($artifact->name)] = $artifact; break; } } @@ -323,6 +406,18 @@ public function &get_classes() { return $this->get_artifacts()[T_CLASS]; } + /** + * Return type parser + * + * @return \local_moodlecheck\type_parser + */ + public function get_type_parser() { + if (!$this->typeparser) { + $this->typeparser = new \local_moodlecheck\type_parser($this->get_use_aliases(), $this->get_artifacts_flat()); + } + return $this->typeparser; + } + /** * Returns all functions (including class methods) found in file * @@ -344,7 +439,7 @@ public function &get_classes() { */ public function &get_functions() { if ($this->functions === null) { - $typeparser = new \local_moodlecheck\type_parser(); + $typeparser = $this->get_type_parser(); $this->functions = array(); $tokens = &$this->get_tokens(); for ($tid = 0; $tid < $this->tokenscount; $tid++) { @@ -410,6 +505,25 @@ public function &get_functions() { $function->arguments[] = array($type, $variable, $nullable); } + + // Get return type. + $returnpair = $this->find_tag_pair($argumentspair[1] + 1, ':', '{', ['{', ';']); + if ($returnpair !== false && $returnpair[1] - $returnpair[0] > 1) { + $rettokens = + array_slice($tokens, $returnpair[0] + 1, $returnpair[1] - $returnpair[0] - 1); + } else { + $rettokens = []; + } + $text = ''; + for ($j = 0; $j < count($rettokens); $j++) { + if ($rettokens[$j][0] != T_COMMENT) { + $text .= $rettokens[$j][1]; + } + } + list($type, $varname, $default, $nullable) = $typeparser->parse_type_and_var($text, 0, true); + + $function->return = $type; + $function->boundaries = $this->find_object_boundaries($function); $this->functions[] = $function; } @@ -429,11 +543,13 @@ public function &get_functions() { * $variable->fullname : name of the variable with class name (i.e. classname::$varname) * $variable->accessmodifiers : tokens like static, public, protected, abstract, etc. * $variable->boundaries : array with ids of first and last token for this variable + * $variable->type : type of variable * * @return array */ public function &get_variables() { if ($this->variables === null) { + $typeparser = $this->get_type_parser(); $this->variables = array(); $this->get_tokens(); for ($tid = 0; $tid < $this->tokenscount; $tid++) { @@ -446,6 +562,20 @@ public function &get_variables() { $variable->fullname = $class->name . '::' . $variable->name; $beforetype = $this->skip_preceding_type($tid); + + if ($beforetype > 0) { + $text = ''; + for ($typetid = $beforetype + 1; $typetid <= $tid; $typetid++) { + if ($this->tokens[$typetid][0] != T_COMMENT) { + $text .= $this->tokens[$typetid][1]; + } + } + list($type, $varname, $default, $nullable) = $typeparser->parse_type_and_var($text, 1, true); + $variable->type = $type; + } else { + $variable->type = null; + } + $variable->accessmodifiers = $this->find_access_modifiers($beforetype); $variable->phpdocs = $this->find_preceeding_phpdoc($beforetype); @@ -590,10 +720,9 @@ public function find_object_boundaries($obj) { */ public function is_inside_class($tid) { $classes = &$this->get_classes(); - $classescnt = count($classes); - for ($clid = 0; $clid < $classescnt; $clid++) { - if ($classes[$clid]->boundaries[0] <= $tid && $classes[$clid]->boundaries[1] >= $tid) { - return $classes[$clid]; + foreach ($classes as $class) { + if ($class->boundaries[0] <= $tid && $class->boundaries[1] >= $tid) { + return $class; } } return false; @@ -943,7 +1072,7 @@ public function &get_all_phpdocs() { $this->get_tokens(); for ($id = 0; $id < $this->tokenscount; $id++) { if (($this->tokens[$id][0] == T_DOC_COMMENT || $this->tokens[$id][0] === T_COMMENT)) { - $this->allphpdocs[$id] = new local_moodlecheck_phpdocs($this->tokens[$id], $id); + $this->allphpdocs[$id] = new local_moodlecheck_phpdocs($this, $this->tokens[$id], $id); } } } @@ -1169,6 +1298,8 @@ class local_moodlecheck_phpdocs { 'link', 'see' ); + /** @var local_moodlecheck_file the containing file */ + protected $file; /** @var array stores the original token for this phpdocs */ protected $originaltoken = null; /** @var int stores id the original token for this phpdocs */ @@ -1188,10 +1319,12 @@ class local_moodlecheck_phpdocs { /** * Constructor. Creates an object and parses it * + * @param local_moodlecheck_file $file the containing file * @param array $token corresponding token parsed from file * @param int $tid id of token in the file */ - public function __construct($token, $tid) { + public function __construct($file, $token, $tid) { + $this->file = $file; $this->originaltoken = $token; $this->originaltid = $tid; if (preg_match('|^///|', $token[1])) { @@ -1336,7 +1469,7 @@ public function get_shortdescription() { * @return array */ public function get_params(string $tag, int $getwhat) { - $typeparser = new \local_moodlecheck\type_parser(); + $typeparser = $this->file->get_type_parser(); $params = array(); foreach ($this->get_tags($tag) as $token) { diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index cb7c793..cb19f18 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -420,17 +420,24 @@ function local_moodlecheck_functiondescription(local_moodlecheck_file $file) { * @return array of found errors */ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { + $typeparser = $file->get_type_parser(); $errors = array(); foreach ($file->get_functions() as $function) { if ($function->phpdocs !== false) { + $ownername = $function->owner ? $function->owner->name : null; + $parentname = $function->owner && $function->owner->extends ? $function->owner->extends : null; + $errorlevel = 0; + $documentedarguments = $function->phpdocs->get_params('param', 2); - $match = (count($documentedarguments) == count($function->arguments)); - for ($i = 0; $match && $i < count($documentedarguments); $i++) { + if (count($documentedarguments) != count($function->arguments)) { + $errorlevel = 2; + } + for ($i = 0; $errorlevel < 2 && $i < count($documentedarguments); $i++) { if (count($documentedarguments[$i]) < 2 || $documentedarguments[$i][0] == null || $documentedarguments[$i][1] == null) { // Must be at least type and parameter name. - $match = false; + $errorlevel = 2; } else { $expectedtype = $function->arguments[$i][0]; $expectedparam = (string)$function->arguments[$i][1]; @@ -438,22 +445,50 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { $documentedtype = $documentedarguments[$i][0]; $documentedparam = $documentedarguments[$i][1]; - $match = ($expectedparam === $documentedparam) - && \local_moodlecheck\type_parser::compare_types($expectedtype, $documentedtype) - // Code smell check follows. - && (!$expectednullable || strpos("|{$documentedtype}|", "|void|") !== false); + if ($expectedparam !== $documentedparam) { + $errorlevel = 2; + } else if ($expectedtype != null) { + $expectedtype = $typeparser->substitute_names($expectedtype, $ownername, $parentname); + $documentedtype = $typeparser->substitute_names($documentedtype, $ownername, $parentname); + if (!$typeparser->compare_types($expectedtype, $documentedtype)) { + $errorlevel = 2; + } else if ($expectednullable && strpos("|{$documentedtype}|", '|void|') === false) { + $errorlevel = 1; + } + } } } + $documentedreturns = $function->phpdocs->get_params('return', 0); - for ($i = 0; $match && $i < count($documentedreturns); $i++) { - if (empty($documentedreturns[$i][0])) { - $match = false; + if (count($documentedreturns) > 1) { + $errorlevel = 2; + } else if (count($documentedreturns) == 1) { + if (empty($documentedreturns[0][0])) { + $errorlevel = 2; + } else if ($function->return != null) { + $expectedtype = $function->return; + $documentedtype = $documentedreturns[0][0]; + $expectedtype = $typeparser->substitute_names($expectedtype, $ownername, $parentname); + $documentedtype = $typeparser->substitute_names($documentedtype, $ownername, $parentname); + if (!$typeparser->compare_types($expectedtype, $documentedtype)) { + $errorlevel = 2; + } else if ($errorlevel < 2 + && strpos("|{$expectedtype}|", '|void|') !== false + && strpos("|{$documentedtype}|", '|void|') === false + && $documentedtype != 'never') { + $errorlevel = 1; + } } } - if (!$match) { - $errors[] = array( + + if ($errorlevel > 0) { + $error = array( 'line' => $function->phpdocs->get_line_number($file, '@param'), 'function' => $function->fullname); + if ($errorlevel < 2) { + $error['severity'] = 'warning'; + } + $errors[] = $error; } } } @@ -479,14 +514,39 @@ function local_moodlecheck_normalise_function_type(string $typelist): ?string { * @return array of found errors */ function local_moodlecheck_variableshasvar(local_moodlecheck_file $file) { + $typeparser = $file->get_type_parser(); $errors = array(); foreach ($file->get_variables() as $variable) { if ($variable->phpdocs !== false) { + $ownername = $variable->class ? $variable->class->name : null; + $parentname = $variable->class && $variable->class->extends ? $variable->class->extends : null; + $errorlevel = 0; + $documentedvars = $variable->phpdocs->get_params('var', 0); - if (!count($documentedvars) || $documentedvars[0][0] == null) { - $errors[] = array( + if (count($documentedvars) != 1 || $documentedvars[0][0] == null) { + $errorlevel = 2; + } else if ($variable->type != null) { + $expectedtype = $variable->type; + $documentedtype = $documentedvars[0][0]; + $expectedtype = $typeparser->substitute_names($expectedtype, $ownername, $parentname); + $documentedtype = $typeparser->substitute_names($documentedtype, $ownername, $parentname); + if (!$typeparser->compare_types($expectedtype, $documentedtype)) { + $errorlevel = 2; + } else if ($errorlevel < 2 + && strpos("|{$expectedtype}|", '|void|') !== false + && strpos("|{$documentedtype}|", '|void|') === false) { + $errorlevel = 1; + } + } + + if ($errorlevel > 0) { + $error = array( 'line' => $variable->phpdocs->get_line_number($file, '@var'), 'variable' => $variable->fullname); + if ($errorlevel < 2) { + $error['severity'] = 'warning'; + } + $errors[] = $error; } } } diff --git a/tests/fixtures/phpdoc_types_valid.php b/tests/fixtures/phpdoc_types_valid.php index d8b3582..a080ffa 100644 --- a/tests/fixtures/phpdoc_types_valid.php +++ b/tests/fixtures/phpdoc_types_valid.php @@ -109,7 +109,7 @@ public function basic_type_equivalence( * @return never */ public function non_native_types($parameterisedarray, $resource, $static, $parameterisediterable, - $arraykey, $scalar, $mixed): void { + $arraykey, $scalar, $mixed) { throw new \Exception(); } @@ -222,21 +222,21 @@ public function object_types_complex(object $traversable2): void { * Never type * @return never|never-return|never-returns|no-return */ - public function never_type(): void { + public function never_type() { throw new \Exception(); } /** * Void type + * @param null $standalonenull * @param ?int $explicitnullable * @param ?int $implicitnullable - * @param null $standalonenull * @return void */ public function void_type( + $standalonenull, ?int $explicitnullable, - int $implicitnullable=null, - $standalonenull + int $implicitnullable=null ): void { } @@ -285,7 +285,7 @@ public function key_and_value_of(int $keyof1, string $valueof1): void { } /** - * Key and valud of complex + * Key and value of complex * @param key-of> $keyof2 * @param value-of> $valueof2 */ @@ -389,4 +389,45 @@ public function structure_combos( ): void { } + /** + * Inheritance + * @param types_valid $basic + * @param self|static|$this $relative1 + * @param types_valid $relative2 + */ + public function inheritance( + types_valid_parent $basic, + parent $relative1, + parent $relative2 + ): void { + } + + /** + * Built-in classes + * @param Traversable|Iterator|Generator|IteratorAggregate $traversable + * @param Iterator|Generator $iterator + * @param Throwable|Exception|Error $throwable + * @param Exception|ErrorException $exception + * @param Error|ArithmeticError|AssertionError|ParseError|TypeError $error + * @param ArithmeticError|DivisionByZeroError $arithmeticerror + * @param CompileError|ParseError $compileerror + */ + public function builtin_classes( + Traversable $traversable, Iterator $iterator, + Throwable $throwable, Exception $exception, Error $error, + ArithmeticError $arithmeticerror, CompileError $compileerror + ): void { + } + + /** + * SPL classes + * @param Iterator|SeekableIterator|ArrayIterator $iterator + * @param SeekableIterator|ArrayIterator $seekableiterator + * @param Countable|ArrayIterator $countable + */ + public function spl_classes( + Iterator $iterator, SeekableIterator $seekableiterator, Countable $countable + ): void { + } + } diff --git a/tests/phpdocs_basic_test.php b/tests/phpdocs_basic_test.php index aa0db7a..2da1617 100644 --- a/tests/phpdocs_basic_test.php +++ b/tests/phpdocs_basic_test.php @@ -93,10 +93,6 @@ public static function local_moodlecheck_normalise_function_type_provider(): arr 'int|string', 'array-key', ], - 'Number' => [ - 'int|float', 'number', - ], - 'Scalar' => [ 'bool|int|float|string', 'scalar', ], From 48bbc1549a6b7511b1d52d4cf278bd918ab4fbb6 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Fri, 29 Sep 2023 16:07:44 +1300 Subject: [PATCH 21/26] Adhere to new CI rules --- classes/type_parser.php | 6 +++--- file.php | 2 +- rules/phpdocs_basic.php | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 2377be8..4c0075c 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -468,7 +468,7 @@ protected function next(int $lookahead = 0, bool $getcase = false): ?string { $next = ''; } $this->nexts[] = (object)['startpos' => $startpos, 'endpos' => $endpos, - 'text' => ($next !== '') ? $next : null]; + 'text' => ($next !== '') ? $next : null,]; } // Return the needed token. @@ -675,7 +675,7 @@ protected function parse_basic_type(): string { $type = 'bool'; } else if (in_array($next, ['int', 'integer', 'positive-int', 'negative-int', 'non-positive-int', 'non-negative-int', - 'int-mask', 'int-mask-of']) + 'int-mask', 'int-mask-of',]) || (ctype_digit($nextchar) || $nextchar == '-') && strpos($next, '.') === false) { // Int. $inttype = $this->parse_token(); @@ -726,7 +726,7 @@ protected function parse_basic_type(): string { $this->parse_token(); $type = 'float'; } else if (in_array($next, ['string', 'class-string', 'numeric-string', 'literal-string', - 'non-empty-string', 'non-falsy-string', 'truthy-string']) + 'non-empty-string', 'non-falsy-string', 'truthy-string',]) || $nextchar == '"' || $nextchar == '\'') { // String. $strtype = $this->parse_token(); diff --git a/file.php b/file.php index dac4481..a3daa62 100644 --- a/file.php +++ b/file.php @@ -503,7 +503,7 @@ public function &get_functions() { } list($type, $variable, $default, $nullable) = $typeparser->parse_type_and_var($text, 3, true); - $function->arguments[] = array($type, $variable, $nullable); + $function->arguments[] = [$type, $variable, $nullable]; } // Get return type. diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index cb19f18..e83be20 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -482,9 +482,9 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { } if ($errorlevel > 0) { - $error = array( + $error = [ 'line' => $function->phpdocs->get_line_number($file, '@param'), - 'function' => $function->fullname); + 'function' => $function->fullname,]; if ($errorlevel < 2) { $error['severity'] = 'warning'; } @@ -540,9 +540,9 @@ function local_moodlecheck_variableshasvar(local_moodlecheck_file $file) { } if ($errorlevel > 0) { - $error = array( + $error = [ 'line' => $variable->phpdocs->get_line_number($file, '@var'), - 'variable' => $variable->fullname); + 'variable' => $variable->fullname,]; if ($errorlevel < 2) { $error['severity'] = 'warning'; } From f24ffc5ca1763ba417ad4fd0803f3dce6992217d Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Fri, 29 Sep 2023 23:11:37 +1300 Subject: [PATCH 22/26] PHP 8.0 fix --- classes/type_parser.php | 6 +++--- file.php | 26 +++++++++++++++++++++----- rules/phpdocs_basic.php | 4 ++-- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 4c0075c..b1e1407 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -468,7 +468,7 @@ protected function next(int $lookahead = 0, bool $getcase = false): ?string { $next = ''; } $this->nexts[] = (object)['startpos' => $startpos, 'endpos' => $endpos, - 'text' => ($next !== '') ? $next : null,]; + 'text' => ($next !== '') ? $next : null, ]; } // Return the needed token. @@ -675,7 +675,7 @@ protected function parse_basic_type(): string { $type = 'bool'; } else if (in_array($next, ['int', 'integer', 'positive-int', 'negative-int', 'non-positive-int', 'non-negative-int', - 'int-mask', 'int-mask-of',]) + 'int-mask', 'int-mask-of', ]) || (ctype_digit($nextchar) || $nextchar == '-') && strpos($next, '.') === false) { // Int. $inttype = $this->parse_token(); @@ -726,7 +726,7 @@ protected function parse_basic_type(): string { $this->parse_token(); $type = 'float'; } else if (in_array($next, ['string', 'class-string', 'numeric-string', 'literal-string', - 'non-empty-string', 'non-falsy-string', 'truthy-string',]) + 'non-empty-string', 'non-falsy-string', 'truthy-string', ]) || $nextchar == '"' || $nextchar == '\'') { // String. $strtype = $this->parse_token(); diff --git a/file.php b/file.php index a3daa62..0d5b802 100644 --- a/file.php +++ b/file.php @@ -212,14 +212,20 @@ public function get_use_aliases() { $after = T_USE; $classname = null; $alias = null; - } else if ($after == T_USE && $tokens[$tid][0] == T_STRING) { + } else if ($after == T_USE + && ($tokens[$tid][0] == T_STRING + || PHP_VERSION_ID >= 80000 + && in_array($tokens[$tid][0], [T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED, T_NAME_RELATIVE]))) { $classname = $tokens[$tid][1]; if (strrpos($classname, '\\') !== false) { $classname = substr($classname, strrpos($classname, '\\') + 1); } } else if ($after == T_USE && $tokens[$tid][0] == T_AS) { $after = T_AS; - } else if ($after == T_AS && $tokens[$tid][0] == T_STRING) { + } else if ($after == T_AS + && ($tokens[$tid][0] == T_STRING + || PHP_VERSION_ID >= 80000 + && in_array($tokens[$tid][0], [T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED, T_NAME_RELATIVE]))) { $alias = $tokens[$tid][1]; } else if (($after == T_USE || $after == T_AS) && in_array($tokens[$tid][1], [',', ';'])) { if ($after == T_AS && $classname && $alias) { @@ -332,14 +338,19 @@ public function get_artifacts() { // Iterate over the remaining tokens in the class definition (until opening {). foreach (array_slice($this->tokens, $tid, $artifact->tagpair[0] - $tid) as $token) { if ($after == T_IMPLEMENTS && $implements - && !in_array($token[0], [T_WHITESPACE, T_COMMENT, T_NS_SEPARATOR, T_STRING])) { + && !in_array($token[0], [T_WHITESPACE, T_COMMENT, T_NS_SEPARATOR, T_STRING]) + && !(PHP_VERSION_ID >= 80000 + && in_array($token[0], [T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED, T_NAME_RELATIVE]))) { $artifact->implements[] = $implements; $implements = null; } if ($token[0] == T_EXTENDS) { $artifact->hasextends = true; $after = T_EXTENDS; - } else if ($after == T_EXTENDS && $token[0] == T_STRING) { + } else if ($after == T_EXTENDS + && ($token[0] == T_STRING + || PHP_VERSION_ID >= 80000 + && in_array($token[0], [T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED, T_NAME_RELATIVE]))) { $extends = $token[1]; if (strrpos($extends, '\\') !== false) { $extends = substr($extends, strrpos($extends, '\\') + 1); @@ -349,12 +360,17 @@ public function get_artifacts() { $artifact->hasimplements = true; $after = T_IMPLEMENTS; $implements = null; - } else if ($after == T_IMPLEMENTS && $token[0] == T_STRING) { + } else if ($after == T_IMPLEMENTS + && ($token[0] == T_STRING + || PHP_VERSION_ID >= 80000 + && in_array($token[0], [T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED, T_NAME_RELATIVE]))) { $implements = $token[1]; if (strrpos($implements, '\\') !== false) { $implements = substr($implements, strrpos($implements, '\\') + 1); } } else if (!in_array($token[0], [T_WHITESPACE, T_COMMENT, T_NS_SEPARATOR, T_STRING]) + && !(PHP_VERSION_ID >= 80000 + && in_array($token[0], [T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED, T_NAME_RELATIVE])) && $token[1] !== ',') { $after = null; } diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index e83be20..941de6f 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -484,7 +484,7 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { if ($errorlevel > 0) { $error = [ 'line' => $function->phpdocs->get_line_number($file, '@param'), - 'function' => $function->fullname,]; + 'function' => $function->fullname, ]; if ($errorlevel < 2) { $error['severity'] = 'warning'; } @@ -542,7 +542,7 @@ function local_moodlecheck_variableshasvar(local_moodlecheck_file $file) { if ($errorlevel > 0) { $error = [ 'line' => $variable->phpdocs->get_line_number($file, '@var'), - 'variable' => $variable->fullname,]; + 'variable' => $variable->fullname, ]; if ($errorlevel < 2) { $error['severity'] = 'warning'; } From bcd838608063914687b82d844b8ca5a90bfbbacd Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Tue, 3 Oct 2023 12:01:58 +1300 Subject: [PATCH 23/26] Code checker array changes missed --- rules/phpdocs_basic.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index 1cf02d5..b6a5f32 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -515,7 +515,7 @@ function local_moodlecheck_normalise_function_type(string $typelist): ?string { */ function local_moodlecheck_variableshasvar(local_moodlecheck_file $file) { $typeparser = $file->get_type_parser(); - $errors = array(); + $errors = []; foreach ($file->get_variables() as $variable) { if ($variable->phpdocs !== false) { $ownername = $variable->class ? $variable->class->name : null; From 4ad34bc462d635b5d791e37ce5d3022c92b8bbb0 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:04:52 +1300 Subject: [PATCH 24/26] Support templates --- classes/type_parser.php | 28 +++++++++++++++++-------- file.php | 45 +++++++++++++++++++++++++++++++++++------ rules/phpdocs_basic.php | 2 +- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index b1e1407..304c779 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -141,6 +141,9 @@ class type_parser { /** @var bool when we encounter an unknown type, should we go wide or narrow */ protected $gowide = false; + /** @var array type templates */ + protected $templates = []; + /** @var object{startpos: non-negative-int, endpos: non-negative-int, text: ?non-empty-string}[] next tokens */ protected $nexts = []; @@ -149,26 +152,34 @@ class type_parser { /** * Constructor - * @param array $usealiases aliases are keys, class names are values - * @param array $artifacts inherit tree + * @param \local_moodlecheck_file $file */ - public function __construct(array $usealiases = [], array $artifacts = []) { - $this->usealiases = $usealiases; - $this->artifacts = $artifacts; + public function __construct(\local_moodlecheck_file $file) { + $this->usealiases = $file->get_use_aliases(); + $this->artifacts = $file->get_artifacts_flat(); } /** * Parse a type and possibly variable name * + * @param ?\local_moodlecheck_phpdocs $phpdocs * @param string $text the text to parse * @param 0|1|2|3 $getwhat what to get 0=type only 1=also var 2=also modifiers (& ...) 3=also default * @param bool $gowide if we can't determine the type, should we assume wide (for native type) or narrow (for PHPDoc)? * @return array{?non-empty-string, ?non-empty-string, string, bool} the simplified type, variable, remaining text, * and whether the type is explicitly nullable */ - public function parse_type_and_var(string $text, int $getwhat, bool $gowide): array { + public function parse_type_and_var(?\local_moodlecheck_phpdocs $phpdocs, + string $text, int $getwhat, bool $gowide): array { // Initialise variables. + if ($phpdocs) { + $artifact = $phpdocs->get_artifact(); + $artifacttemplates = ($artifact && $artifact->phpdocs) ? $artifact->phpdocs->get_templates() : []; + $this->templates = array_merge($artifacttemplates, $phpdocs->get_templates()); + } else { + $this->templates = []; + } $this->text = strtolower($text); $this->textwithcase = $text; $this->gowide = $gowide; @@ -944,7 +955,9 @@ protected function parse_basic_type(): string { } $type = ucfirst($type); assert($type != ''); - if ($this->next == '<') { + if ($this->templates[strtolower($type)] ?? null) { + $type = $this->templates[strtolower($type)]; + } else if ($this->next == '<') { // Collection / Traversable. $this->parse_token('<'); $firsttype = $this->parse_any_type(); @@ -978,7 +991,6 @@ protected function parse_basic_type(): string { $type = $this->gowide ? 'mixed' : 'never'; } - assert(strpos($type, '|') === false && strpos($type, '&') === false); return $type; } diff --git a/file.php b/file.php index ac7196a..e82734d 100644 --- a/file.php +++ b/file.php @@ -429,7 +429,7 @@ public function &get_classes() { */ public function get_type_parser() { if (!$this->typeparser) { - $this->typeparser = new \local_moodlecheck\type_parser($this->get_use_aliases(), $this->get_artifacts_flat()); + $this->typeparser = new \local_moodlecheck\type_parser($this); } return $this->typeparser; } @@ -517,13 +517,13 @@ public function &get_functions() { $text .= $argtokens[$j][1]; } } - list($type, $variable, $default, $nullable) = $typeparser->parse_type_and_var($text, 3, true); + list($type, $variable, $default, $nullable) = $typeparser->parse_type_and_var(null, $text, 3, true); $function->arguments[] = [$type, $variable, $nullable]; } // Get return type. - $returnpair = $this->find_tag_pair($argumentspair[1] + 1, ':', '{', ['{', ';']); + $returnpair = $this->find_tag_pair($argumentspair ? $argumentspair[1] + 1 : $tid + 1, ':', '{', ['{', ';']); if ($returnpair !== false && $returnpair[1] - $returnpair[0] > 1) { $rettokens = array_slice($tokens, $returnpair[0] + 1, $returnpair[1] - $returnpair[0] - 1); @@ -536,7 +536,7 @@ public function &get_functions() { $text .= $rettokens[$j][1]; } } - list($type, $varname, $default, $nullable) = $typeparser->parse_type_and_var($text, 0, true); + list($type, $varname, $default, $nullable) = $typeparser->parse_type_and_var(null, $text, 0, true); $function->return = $type; @@ -586,7 +586,7 @@ public function &get_variables() { $text .= $this->tokens[$typetid][1]; } } - list($type, $varname, $default, $nullable) = $typeparser->parse_type_and_var($text, 1, true); + list($type, $varname, $default, $nullable) = $typeparser->parse_type_and_var(null, $text, 1, true); $variable->type = $type; } else { $variable->type = null; @@ -1237,6 +1237,7 @@ class local_moodlecheck_phpdocs { 'static', 'staticvar', 'subpackage', + // 'template', 'throws', 'todo', 'tutorial', @@ -1283,6 +1284,7 @@ class local_moodlecheck_phpdocs { 'see', 'since', 'subpackage', + // 'template', 'throws', 'todo', 'uses', @@ -1378,6 +1380,10 @@ public function __construct($file, $token, $tid) { $this->description = trim($this->description); } + public function get_artifact() { + return $this->file->is_inside_artifact($this->originaltid); + } + /** * Returns all tags found in phpdocs * @@ -1475,6 +1481,32 @@ public function get_shortdescription() { } } + public function get_templates() { + $typeparser = $this->file->get_type_parser(); + $templates = []; + + foreach ($this->get_tags('template') as $token) { + $token = trim($token); + $nameend = 0; + while ($nameend < strlen($token) && (ctype_alnum($token[$nameend]) || $token[$nameend] == '_')) { + $nameend++; + } + if ($nameend > 0) { + $ofstart = $nameend; + while ($ofstart < strlen($token) && ctype_space($token[$ofstart])) { + $ofstart++; + } + $type = 'mixed'; + if (substr($token, $ofstart, 2) == 'of') { + list($type) = $typeparser->parse_type_and_var(null, substr($token, $ofstart+2), 0, false); + } + $templates[strtolower(substr($token, 0, $nameend))] = $type; + } + } + + return $templates; + } + /** * Returns list of parsed param tokens found in phpdocs * @@ -1489,7 +1521,8 @@ public function get_params(string $tag, int $getwhat) { $params = []; foreach ($this->get_tags($tag) as $token) { - list($type, $variable, $description) = $typeparser->parse_type_and_var($token, $getwhat, false); + list($type, $variable, $description) = + $typeparser->parse_type_and_var($this, $token, $getwhat, false); $param = []; $param[] = $type; if ($getwhat >= 1) { diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index b6a5f32..13f5a63 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -503,7 +503,7 @@ function local_moodlecheck_functionarguments(local_moodlecheck_file $file) { */ function local_moodlecheck_normalise_function_type(string $typelist): ?string { $typeparser = new \local_moodlecheck\type_parser(); - list($type, $variable, $remainder) = $typeparser->parse_type_and_var($typelist, 0, true); + list($type, $variable, $remainder) = $typeparser->parse_type_and_var(null, $typelist, 0, true); return $type; } From 42734456760acda63d4a911c2e43e91eeab25c35 Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:28:16 +1300 Subject: [PATCH 25/26] Code checker fixes --- classes/type_parser.php | 8 ++++---- file.php | 8 ++++---- tests/moodlecheck_rules_test.php | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/classes/type_parser.php b/classes/type_parser.php index 304c779..d535d6d 100644 --- a/classes/type_parser.php +++ b/classes/type_parser.php @@ -152,11 +152,11 @@ class type_parser { /** * Constructor - * @param \local_moodlecheck_file $file + * @param ?\local_moodlecheck_file $file */ - public function __construct(\local_moodlecheck_file $file) { - $this->usealiases = $file->get_use_aliases(); - $this->artifacts = $file->get_artifacts_flat(); + public function __construct(\local_moodlecheck_file $file = null) { + $this->usealiases = $file ? $file->get_use_aliases() : []; + $this->artifacts = $file ? $file->get_artifacts_flat() : []; } /** diff --git a/file.php b/file.php index e82734d..6fb7e4e 100644 --- a/file.php +++ b/file.php @@ -1237,7 +1237,7 @@ class local_moodlecheck_phpdocs { 'static', 'staticvar', 'subpackage', - // 'template', + // Maybe add: 'template', 'throws', 'todo', 'tutorial', @@ -1284,7 +1284,7 @@ class local_moodlecheck_phpdocs { 'see', 'since', 'subpackage', - // 'template', + // Maybe add: 'template', 'throws', 'todo', 'uses', @@ -1498,12 +1498,12 @@ public function get_templates() { } $type = 'mixed'; if (substr($token, $ofstart, 2) == 'of') { - list($type) = $typeparser->parse_type_and_var(null, substr($token, $ofstart+2), 0, false); + list($type) = $typeparser->parse_type_and_var(null, substr($token, $ofstart + 2), 0, false); } $templates[strtolower(substr($token, 0, $nameend))] = $type; } } - + return $templates; } diff --git a/tests/moodlecheck_rules_test.php b/tests/moodlecheck_rules_test.php index 6021862..37e42f2 100644 --- a/tests/moodlecheck_rules_test.php +++ b/tests/moodlecheck_rules_test.php @@ -713,7 +713,7 @@ public function test_html_format_errors_and_warnings(): void { * * @covers \local_moodlecheck\type_parser */ - public function test_phpdoc_types_valid() { + public function test_phpdoc_types_valid(): void { global $PAGE; $output = $PAGE->get_renderer('local_moodlecheck'); $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_types_valid.php', null); @@ -735,7 +735,7 @@ public function test_phpdoc_types_valid() { * * @covers \local_moodlecheck\type_parser */ - public function test_phpdoc_types_invalid() { + public function test_phpdoc_types_invalid(): void { global $PAGE; $output = $PAGE->get_renderer('local_moodlecheck'); $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_types_invalid.php', null); From d97f7bb0fed822394c7566977ad9b5890817ef4d Mon Sep 17 00:00:00 2001 From: James C <5689414+james-cnz@users.noreply.github.com> Date: Wed, 28 Feb 2024 17:32:35 +1300 Subject: [PATCH 26/26] Code checker fixes 2 --- file.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/file.php b/file.php index 6fb7e4e..1bd20c8 100644 --- a/file.php +++ b/file.php @@ -1237,7 +1237,7 @@ class local_moodlecheck_phpdocs { 'static', 'staticvar', 'subpackage', - // Maybe add: 'template', + // Maybe add: 'template', . 'throws', 'todo', 'tutorial', @@ -1284,7 +1284,7 @@ class local_moodlecheck_phpdocs { 'see', 'since', 'subpackage', - // Maybe add: 'template', + // Maybe add: 'template', . 'throws', 'todo', 'uses',