Skip to content

Commit

Permalink
Remove functionsdocumented
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewnicols committed Mar 18, 2024
1 parent 81247ae commit 14c497f
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 77 deletions.
2 changes: 0 additions & 2 deletions lang/en/local_moodlecheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@

$string['error_emptynophpfile'] = 'The file is empty or doesn\'t contain PHP code. Skipped.';

$string['rule_functionsdocumented'] = 'All functions are documented';
$string['error_functionsdocumented'] = 'Function <b>{$a->function}</b> is not documented';
$string['rule_variablesdocumented'] = 'All variables are documented';
$string['error_variablesdocumented'] = 'Variable <b>{$a->variable}</b> is not documented';
$string['rule_constsdocumented'] = 'All constants are documented';
Expand Down
39 changes: 0 additions & 39 deletions rules/phpdocs_basic.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

defined('MOODLE_INTERNAL') || die;

local_moodlecheck_registry::add_rule('functionsdocumented')->set_callback('local_moodlecheck_functionsdocumented');
local_moodlecheck_registry::add_rule('variablesdocumented')->set_callback('local_moodlecheck_variablesdocumented');
local_moodlecheck_registry::add_rule('constsdocumented')->set_callback('local_moodlecheck_constsdocumented');
local_moodlecheck_registry::add_rule('definesdocumented')->set_callback('local_moodlecheck_definesdocumented');
Expand All @@ -45,44 +44,6 @@
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 functions have phpdocs blocks.
*
* For methods of a class which extends another or implements interfaces, a violation in only a warning, since the
* method might be overriding a sufficiently documented method. In all other cases, it is an error.
*
* @param local_moodlecheck_file $file
* @return array of found errors
*/
function local_moodlecheck_functionsdocumented(local_moodlecheck_file $file) {
$isphpunitfile = preg_match('#/tests/.+_test\.php$#', $file->get_filepath());
$errors = [];
foreach ($file->get_functions() as $function) {
if ($function->phpdocs === false) {
// Exception is made for plain phpunit test methods MDLSITE-3282, MDLSITE-3856.
$istestmethod = (strpos($function->name, 'test_') === 0 ||
stripos($function->name, 'setup') === 0 ||
stripos($function->name, 'teardown') === 0);

$isinsubclass = $function->owner && ($function->owner->hasextends || $function->owner->hasimplements);

if (!($isphpunitfile && $istestmethod)) {
$error = [
'function' => $function->fullname,
'line' => $file->get_line_number($function->boundaries[0]),
];

if ($isinsubclass) {
$error["severity"] = "warning";
}

$errors[] = $error;
}
}
}
return $errors;
}

/**
* Checks if all variables have phpdocs blocks
*
Expand Down
39 changes: 3 additions & 36 deletions tests/moodlecheck_rules_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ public static function empty_nophp_files_provider(): array {
*
* @covers ::local_moodlecheck_functionarguments
*/
public function test_functionsdocumented_method_multiline(): void {
public function test_j_method_multiline(): void {
$file = __DIR__ . "/fixtures/phpdoc_method_multiline.php";

global $PAGE;
Expand All @@ -379,43 +379,12 @@ public function test_functionsdocumented_method_multiline(): void {
$this->assertSame(0, $found->length); // All examples in fixtures are ok.
}


/**
* Verify that top-level methods without docs are errors but methods in subclasses without docs are warnings.
*
* @covers ::local_moodlecheck_functionsdocumented
*/
public function test_functionsdocumented_method_overrides(): void {
$file = __DIR__ . "/fixtures/phpdoc_method_overrides.php";

global $PAGE;
$output = $PAGE->get_renderer('local_moodlecheck');
$path = new local_moodlecheck_path($file, null);
$result = $output->display_path($path, 'xml');

// Convert results to XML Object.
$xmlresult = new \DOMDocument();
$xmlresult->loadXML($result);

$xpath = new \DOMXpath($xmlresult);
$found = $xpath->query('//file/error[@source="functionsdocumented"]');
// TODO: Change to DOMNodeList::count() when php71 support is gone.
$this->assertSame(4, $found->length);

$severities = array_map(function (\DOMElement $element) {
return $element->getAttribute("severity");
}, iterator_to_array($found));

$this->assertEquals(["error", "warning", "warning", "warning"], $severities);
}

/**
* Verify that "use function" statements are ignored.
*
* @covers ::local_moodlecheck_functionsdocumented
* @covers ::local_moodlecheck_constsdocumented
*/
public function test_functionsdocumented_constsdocumented_ignore_uses(): void {
public function test_constsdocumented_ignore_uses(): void {
$file = __DIR__ . "/fixtures/uses.php";

global $PAGE;
Expand All @@ -428,7 +397,7 @@ public function test_functionsdocumented_constsdocumented_ignore_uses(): void {
$xmlresult->loadXML($result);

$xpath = new \DOMXpath($xmlresult);
$found = $xpath->query('//file/error[@source="functionsdocumented" or @source="constsdocumented"]');
$found = $xpath->query('//file/error[@source="constsdocumented"]');
// TODO: Change to DOMNodeList::count() when php71 support is gone.
$this->assertSame(0, $found->length);
}
Expand Down Expand Up @@ -498,7 +467,6 @@ public function test_text_format_errors_and_warnings(): void {
$result = $output->display_path($path, 'text');

$this->assertStringContainsString('tests/fixtures/error_and_warning.php', $result);
$this->assertStringContainsString('12: Function someclass::somefunc is not documented (warning)', $result);
}

/**
Expand All @@ -515,6 +483,5 @@ public function test_html_format_errors_and_warnings(): void {
$result = $output->display_path($path, 'html');

$this->assertStringContainsString('tests/fixtures/error_and_warning.php</span>', $result);
$this->assertStringContainsString('<b>12</b>: Function <b>someclass::somefunc</b> is not documented (warning)', $result);
}
}

0 comments on commit 14c497f

Please sign in to comment.