Skip to content

Commit

Permalink
Remove all checks for define() (#141)
Browse files Browse the repository at this point in the history
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);
```
  • Loading branch information
andrewnicols committed Mar 22, 2024
1 parent 2c7f500 commit 534b65c
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 80 deletions.
38 changes: 0 additions & 38 deletions file.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class local_moodlecheck_file {
protected $filephpdocs = null;
protected $allphpdocs = null;
protected $variables = null;
protected $defines = null;
protected $constants = null;

/**
Expand Down Expand Up @@ -559,43 +558,6 @@ public function &get_constants() {
return $this->constants;
}

/**
* 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
*
Expand Down
5 changes: 0 additions & 5 deletions lang/en/local_moodlecheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@

$string['rule_constsdocumented'] = 'All constants are documented';
$string['error_constsdocumented'] = 'Constant <b>{$a->object}</b> is not documented';
$string['rule_definesdocumented'] = 'All define statements are documented';
$string['error_definesdocumented'] = 'Define statement for <b>{$a->object}</b> 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';
Expand All @@ -63,8 +61,5 @@
$string['error_functionarguments'] = 'Phpdocs for function <b>{$a->function}</b> 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: <b>{$a->object}</b>';
$string['rule_definedoccorrect'] = 'Check syntax for define statement';

$string['rule_categoryvalid'] = 'Category tag is valid';
$string['error_categoryvalid'] = 'Category <b>{$a->category}</b> is not valid';
37 changes: 0 additions & 37 deletions rules/phpdocs_basic.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,10 @@
defined('MOODLE_INTERNAL') || die;

local_moodlecheck_registry::add_rule('constsdocumented')->set_callback('local_moodlecheck_constsdocumented');
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('phpdocsfistline')->set_callback('local_moodlecheck_phpdocsfistline');
local_moodlecheck_registry::add_rule('functiondescription')->set_callback('local_moodlecheck_functiondescription');
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');
Expand All @@ -51,22 +49,6 @@ function local_moodlecheck_constsdocumented(local_moodlecheck_file $file) {
return $errors;
}

/**
* 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
*
Expand Down Expand Up @@ -321,22 +303,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;
}

0 comments on commit 534b65c

Please sign in to comment.