From 48ff9d3717819ebe09eb2704bec3fc755ae0c408 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 22 Mar 2024 23:19:06 +0800 Subject: [PATCH 1/2] Remove all checks for define() (#141) These checks never worked. The `get_defines()` method built a list of definitions, added them to a `$defines` array, but return `$this->defines` instead. Furthermore there would be pandamonium if we actually fixed this because we use constants in scripts all over Moodle with the same value, for example: ``` define('CLI_SCRIPT', true); ``` --- file.php | 39 ----------------------------------- lang/en/local_moodlecheck.php | 6 ------ rules/phpdocs_basic.php | 37 --------------------------------- 3 files changed, 82 deletions(-) diff --git a/file.php b/file.php index 0541897..3be7416 100644 --- a/file.php +++ b/file.php @@ -46,7 +46,6 @@ class local_moodlecheck_file { protected $filephpdocs = null; protected $allphpdocs = null; protected $variables = null; - protected $defines = null; /** * Creates an object from path to the file @@ -70,7 +69,6 @@ protected function clear_memory() { $this->filephpdocs = null; $this->allphpdocs = null; $this->variables = null; - $this->defines = null; } /** @@ -516,43 +514,6 @@ public function &get_variables() { return $this->variables; } - /** - * Returns all 'define' statements found in file - * - * Returns array of objects where each element represents a define statement: - * $variable->tid : token id of the token with variable name - * $variable->name : name of the variable (starts with $) - * $variable->phpdocs : phpdocs for this variable (instance of local_moodlecheck_phpdocs or false if not found) - * $variable->class : containing class object - * $variable->fullname : name of the variable with class name (i.e. classname::$varname) - * $variable->boundaries : array with ids of first and last token for this constant - * - * @return array - */ - public function &get_defines() { - if ($this->defines === null) { - $this->defines = []; - $this->get_tokens(); - for ($tid = 0; $tid < $this->tokenscount; $tid++) { - if ($this->tokens[$tid][0] == T_STRING && $this->tokens[$tid][1] == 'define' && - !$this->is_inside_function($tid) && !$this->is_inside_class($tid)) { - $next1id = $this->next_nonspace_token($tid, true); - $next1 = $this->next_nonspace_token($tid, false); - $next2 = $this->next_nonspace_token($next1id, false); - $variable = new stdClass; - $variable->tid = $tid; - if ($next1 == '(' && preg_match("/^(['\"])(.*)\\1$/", $next2, $matches)) { - $variable->fullname = $variable->name = $matches[2]; - } - $variable->phpdocs = $this->find_preceeding_phpdoc($tid); - $variable->boundaries = $this->find_object_boundaries($variable); - $defines[] = $variable; - } - } - } - return $this->defines; - } - /** * Finds and returns object boundaries * diff --git a/lang/en/local_moodlecheck.php b/lang/en/local_moodlecheck.php index 79b07ff..8ee11fc 100644 --- a/lang/en/local_moodlecheck.php +++ b/lang/en/local_moodlecheck.php @@ -37,9 +37,6 @@ $string['error_emptynophpfile'] = 'The file is empty or doesn\'t contain PHP code. Skipped.'; -$string['rule_definesdocumented'] = 'All define statements are documented'; -$string['error_definesdocumented'] = 'Define statement for {$a->object} is not documented'; - $string['rule_noinlinephpdocs'] = 'There are no comments starting with three or more slashes'; $string['error_noinlinephpdocs'] = 'Found comment starting with three or more slashes'; @@ -55,8 +52,5 @@ $string['error_functionarguments'] = 'Phpdocs for function {$a->function} has incomplete parameters list'; $string['rule_functionarguments'] = 'Phpdocs for functions properly define all parameters'; -$string['error_definedoccorrect'] = 'Phpdocs for define statement must start with constant name and dash: {$a->object}'; -$string['rule_definedoccorrect'] = 'Check syntax for define statement'; - $string['rule_categoryvalid'] = 'Category tag is valid'; $string['error_categoryvalid'] = 'Category {$a->category} is not valid'; diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index 6d73ef5..1cceae9 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -24,30 +24,12 @@ defined('MOODLE_INTERNAL') || die; -local_moodlecheck_registry::add_rule('definesdocumented')->set_callback('local_moodlecheck_definesdocumented'); local_moodlecheck_registry::add_rule('noinlinephpdocs')->set_callback('local_moodlecheck_noinlinephpdocs'); local_moodlecheck_registry::add_rule('functionarguments')->set_callback('local_moodlecheck_functionarguments'); -local_moodlecheck_registry::add_rule('definedoccorrect')->set_callback('local_moodlecheck_definedoccorrect'); local_moodlecheck_registry::add_rule('phpdocsinvalidinlinetag')->set_callback('local_moodlecheck_phpdocsinvalidinlinetag'); local_moodlecheck_registry::add_rule('phpdocsuncurlyinlinetag')->set_callback('local_moodlecheck_phpdocsuncurlyinlinetag'); local_moodlecheck_registry::add_rule('phpdoccontentsinlinetag')->set_callback('local_moodlecheck_phpdoccontentsinlinetag'); -/** - * Checks if all variables have phpdocs blocks - * - * @param local_moodlecheck_file $file - * @return array of found errors - */ -function local_moodlecheck_definesdocumented(local_moodlecheck_file $file) { - $errors = []; - foreach ($file->get_defines() as $object) { - if ($object->phpdocs === false) { - $errors[] = ['object' => $object->fullname, 'line' => $file->get_line_number($object->tid)]; - } - } - return $errors; -} - /** * Checks that no comment starts with three or more slashes * @@ -257,22 +239,3 @@ function($type) { return implode('|', $types); } - -/** - * Checks that all define statement have constant name in phpdoc block - * - * @param local_moodlecheck_file $file - * @return array of found errors - */ -function local_moodlecheck_definedoccorrect(local_moodlecheck_file $file) { - $errors = []; - foreach ($file->get_defines() as $object) { - if ($object->phpdocs !== false) { - if (!preg_match('/^\s*'.$object->name.'\s+-\s+(.*)/', $object->phpdocs->get_description(), $matches) || - !strlen(trim($matches[1]))) { - $errors[] = ['line' => $object->phpdocs->get_line_number($file), 'object' => $object->fullname]; - } - } - } - return $errors; -} From b3f266e8810ce8b4a003bb62f318d00c1428a747 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 27 Mar 2024 22:16:07 +0800 Subject: [PATCH 2/2] Remove unused code (variables) (#143) The last usage of this was removed when we removed the variableshasvar sniff. --- file.php | 72 -------------------------------------------------------- 1 file changed, 72 deletions(-) diff --git a/file.php b/file.php index 3be7416..e24cb65 100644 --- a/file.php +++ b/file.php @@ -45,7 +45,6 @@ class local_moodlecheck_file { protected $functions = null; protected $filephpdocs = null; protected $allphpdocs = null; - protected $variables = null; /** * Creates an object from path to the file @@ -68,7 +67,6 @@ protected function clear_memory() { $this->functions = null; $this->filephpdocs = null; $this->allphpdocs = null; - $this->variables = null; } /** @@ -475,45 +473,6 @@ public function &get_functions() { return $this->functions; } - /** - * Returns all class properties (variables) found in file - * - * Returns array of objects where each element represents a variable: - * $variable->tid : token id of the token with variable name - * $variable->name : name of the variable (starts with $) - * $variable->phpdocs : phpdocs for this variable (instance of local_moodlecheck_phpdocs or false if not found) - * $variable->class : containing class object - * $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 - * - * @return array - */ - public function &get_variables() { - if ($this->variables === null) { - $this->variables = []; - $this->get_tokens(); - for ($tid = 0; $tid < $this->tokenscount; $tid++) { - if ($this->tokens[$tid][0] == T_VARIABLE && ($class = $this->is_inside_class($tid)) && - !$this->is_inside_function($tid)) { - $variable = new stdClass; - $variable->tid = $tid; - $variable->name = $this->tokens[$tid][1]; - $variable->class = $class; - $variable->fullname = $class->name . '::' . $variable->name; - - $beforetype = $this->skip_preceding_type($tid); - $variable->accessmodifiers = $this->find_access_modifiers($beforetype); - $variable->phpdocs = $this->find_preceeding_phpdoc($beforetype); - - $variable->boundaries = $this->find_object_boundaries($variable); - $this->variables[] = $variable; - } - } - } - return $this->variables; - } - /** * Finds and returns object boundaries * @@ -1092,11 +1051,6 @@ class local_moodlecheck_phpdocs { /** @var string text of phpdocs with trimmed start/end tags * as well as * in the beginning of the lines */ protected $trimmedtext = null; - /** @var boolean whether the phpdocs contains text after the tokens - * (possible in phpdocs but not recommended in Moodle) */ - protected $brokentext = false; - /** @var string the description found in phpdocs */ - protected $description; /** @var array array of string where each string * represents found token (may be also multiline) */ protected $tokens; @@ -1119,7 +1073,6 @@ public function __construct($token, $tid) { $lines = preg_split('/\n/', $this->trimmedtext); $this->tokens = []; - $this->description = ''; $istokenline = false; for ($i = 0; $i < count($lines); $i++) { if (preg_match('|^\s*\@(\w+)|', $lines[$i])) { @@ -1130,19 +1083,12 @@ public function __construct($token, $tid) { // Second/third line of token description. $this->tokens[count($this->tokens) - 1] .= "\n". $lines[$i]; } else { - // This is part of description. - if (strlen(trim($lines[$i])) && !empty($this->tokens)) { - // Some text appeared AFTER tokens. - $this->brokentext = true; - } - $this->description .= $lines[$i]."\n"; $istokenline = false; } } foreach ($this->tokens as $i => $token) { $this->tokens[$i] = trim($token); } - $this->description = trim($this->description); } /** @@ -1183,24 +1129,6 @@ public function get_tokens($tag = null, $nonempty = false) { return get_tags($tag, $nonempty); } - /** - * Returns the description without tokens found in phpdocs - * - * @return string - */ - public function get_description() { - return $this->description; - } - - /** - * Returns true if part of the text is after any of the tokens - * - * @return bool - */ - public function is_broken_description() { - return $this->brokentext; - } - /** * Returns true if this is an inline phpdoc comment (starting with three slashes) *