diff --git a/lang/en/local_moodlecheck.php b/lang/en/local_moodlecheck.php index d493704..aeffcaf 100644 --- a/lang/en/local_moodlecheck.php +++ b/lang/en/local_moodlecheck.php @@ -90,12 +90,6 @@ $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['error_packagespecified'] = 'Package is not specified for {$a->object}. It is also not specified in file-level phpdocs'; -$string['rule_packagespecified'] = 'All functions (which are not methods) and classes have package specified or inherited'; - -$string['rule_packagevalid'] = 'Package tag is valid'; -$string['error_packagevalid'] = 'Package {$a->package} is not valid'; - $string['rule_categoryvalid'] = 'Category tag is valid'; $string['error_categoryvalid'] = 'Category {$a->category} is not valid'; diff --git a/rules/phpdocs_package.php b/rules/phpdocs_package.php index ccbb700..c616ebb 100644 --- a/rules/phpdocs_package.php +++ b/rules/phpdocs_package.php @@ -24,68 +24,8 @@ defined('MOODLE_INTERNAL') || die; -local_moodlecheck_registry::add_rule('packagespecified')->set_callback('local_moodlecheck_packagespecified'); -local_moodlecheck_registry::add_rule('packagevalid')->set_callback('local_moodlecheck_packagevalid'); local_moodlecheck_registry::add_rule('categoryvalid')->set_callback('local_moodlecheck_categoryvalid'); -/** - * Checks if all functions (outside class) and classes have package - * - * package tag may be inherited from file-level phpdocs - * - * @param local_moodlecheck_file $file - * @return array of found errors - */ -function local_moodlecheck_packagespecified(local_moodlecheck_file $file) { - $errors = []; - $phpdocs = $file->find_file_phpdocs(); - if ($phpdocs && count($phpdocs->get_tags('package', true))) { - // Package is specified on file level, it is automatically inherited. - return []; - } - - foreach ($file->get_artifacts_flat() as $artifact) { - if (!$artifact->phpdocs || !count($artifact->phpdocs->get_tags('package', true))) { - $errors[] = [ - 'line' => $file->get_line_number($artifact->boundaries[0]), - 'object' => "$artifact->typestring $artifact->name", - ]; - } - } - - foreach ($file->get_functions() as $object) { - if ($object->owner === false) { - if (!$object->phpdocs || !count($object->phpdocs->get_tags('package', true))) { - $errors[] = [ - 'line' => $file->get_line_number($object->boundaries[0]), - 'object' => 'function ' . $object->fullname, - ]; - } - } - } - return $errors; -} - -/** - * Checks that wherever the package token is specified it is valid - * - * @param local_moodlecheck_file $file - * @return array of found errors - */ -function local_moodlecheck_packagevalid(local_moodlecheck_file $file) { - $errors = []; - - $allowedpackages = local_moodlecheck_package_names($file); - foreach ($file->get_all_phpdocs() as $phpdoc) { - foreach ($phpdoc->get_tags('package') as $package) { - if (!in_array($package, $allowedpackages)) { - $errors[] = ['line' => $phpdoc->get_line_number($file, '@package'), 'package' => $package]; - } - } - } - return $errors; -} - /** * Checks that wherever the category token is specified it is valid * @@ -105,89 +45,6 @@ function local_moodlecheck_categoryvalid(local_moodlecheck_file $file) { return $errors; } -/** - * Returns package names available for the file location - * - * If the file is inside plugin directory only frankenstyle name for this plugin is returned - * Otherwise returns list of available core packages - * - * @param local_moodlecheck_file $file - * @return array - */ -function local_moodlecheck_package_names(local_moodlecheck_file $file) { - static $allplugins = []; - static $allsubsystems = []; - static $corepackages = []; - // Get and cache the list of plugins. - if (empty($allplugins)) { - $components = local_moodlecheck_path::get_components(); - // First try to get the list from file components. - if (isset($components['plugin'])) { - $allplugins = $components['plugin']; - } else { - $allplugins = local_moodlecheck_get_plugins(); - } - } - // Get and cache the list of subsystems. - if (empty($allsubsystems)) { - $components = local_moodlecheck_path::get_components(); - // First try to get the list from file components. - if (isset($components['subsystem'])) { - $allsubsystems = $components['subsystem']; - } else { - $allsubsystems = get_core_subsystems(true); - } - // Prepare the list of core packages. - foreach ($allsubsystems as $subsystem => $dir) { - // Subsytems may come with the valid component name (core_ prefixed) already. - if (strpos($subsystem, 'core_') === 0 || $subsystem === 'core') { - $corepackages[] = $subsystem; - } else { - $corepackages[] = 'core_' . $subsystem; - } - } - // Add "core" if missing. - if (!in_array('core', $corepackages)) { - $corepackages[] = 'core'; - } - } - - // Return valid plugin if the $file belongs to it. - foreach ($allplugins as $pluginfullname => $dir) { - if ($file->is_in_dir($dir)) { - return [$pluginfullname]; - } - } - - // If not return list of valid core packages. - return $corepackages; -} - -/** - * Returns all installed plugins - * - * Returns all installed plugins as an associative array - * with frankenstyle name as a key and plugin directory as a value - * - * @return array - */ -function &local_moodlecheck_get_plugins() { - static $allplugins = []; - if (empty($allplugins)) { - $plugintypes = get_plugin_types(); - foreach ($plugintypes as $plugintype => $pluginbasedir) { - if ($plugins = get_plugin_list($plugintype)) { - foreach ($plugins as $plugin => $plugindir) { - $allplugins[$plugintype.'_'.$plugin] = $plugindir; - } - } - } - asort($allplugins); - $allplugins = array_reverse($allplugins, true); - } - return $allplugins; -} - /** * Reads the list of Core APIs from internet (or local copy) and returns the list of categories * diff --git a/tests/fixtures/phpdoc_tags_packagespecified.php b/tests/fixtures/phpdoc_tags_packagespecified.php deleted file mode 100644 index d5f13d3..0000000 --- a/tests/fixtures/phpdoc_tags_packagespecified.php +++ /dev/null @@ -1,118 +0,0 @@ -. - -/** - * A minimalistic file phpdoc block without package tag. - * - * @copyright 2022 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} - * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); - -// These are missing any package tag. - -/** - * This is a class without package tag. - */ -class missingclass { - /** - * This is a method. - */ - public function somemethod() { - return; - } -} - -/** - * This is a trait without package tag. - */ -trait missingtrait { - /** - * This is a method. - */ - public function somemethod() { - return; - } -} - -/** - * This is an interface without package tag. - */ -interface missinginterface { - /** - * This is a method. - */ - public function somemethod(); -} - -/** - * This is a global scope function without package tag. - */ -function missingfunction() { - return; -} - -// These have parent package tag. - -/** - * Lovely class with package tag. - * - * @package local_moodlecheck - */ -class packagedclass { - /** - * This is a method. - */ - public function somemethod() { - return; - } -} - -/** - * Lovely trait with package tag. - * - * @package local_moodlecheck - */ -trait packagedtrait { - /** - * This is a method - */ - public function somemethod() { - return; - } -} - -/** - * Lovely interface with package tag. - * - * @package local_moodlecheck - */ -interface packagedinterface { - /** - * This is a method - */ - public function somemethod(); -} - -/** - * Lovely global scope function with package tag. - * - * @package local_moodlecheck - */ -function packagedfunction() { - return; -} diff --git a/tests/moodlecheck_rules_test.php b/tests/moodlecheck_rules_test.php index 4333217..5b569c8 100644 --- a/tests/moodlecheck_rules_test.php +++ b/tests/moodlecheck_rules_test.php @@ -70,11 +70,9 @@ public function test_constantclass(): void { $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(2, $found->length); + $this->assertSame(0, $found->length); // Also verify that contents do not include any problem with line 42 / classesdocumented. Use simple string matching here. - $this->assertStringContainsString('line="20"', $result); - $this->assertStringContainsString('packagevalid', $result); $this->assertStringNotContainsString('line="42"', $result); $this->assertStringNotContainsString('classesdocumented', $result); } @@ -172,10 +170,9 @@ public function test_phpdoc_tags_general(): void { $xpath = new \DOMXpath($xmlresult); $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(19, $found->length); + $this->assertSame(17, $found->length); // Also verify various bits by content. - $this->assertStringContainsString('packagevalid', $result); $this->assertStringContainsString('incomplete_param_annotation has incomplete parameters list', $result); $this->assertStringContainsString('missing_param_defintion has incomplete parameters list', $result); $this->assertStringContainsString('missing_param_annotation has incomplete parameters list', $result); @@ -218,8 +215,7 @@ public function test_phpdoc_constructor_property_promotion(): void { $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(1, $found->length); - $this->assertStringContainsString('packagevalid', $result); + $this->assertSame(0, $found->length); $this->assertStringNotContainsString('constructor_property_promotion::__construct has incomplete parameters list', $result); } @@ -244,8 +240,7 @@ public function test_phpdoc_union_types(): void { $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(1, $found->length); - $this->assertStringContainsString('packagevalid', $result); + $this->assertSame(0, $found->length); $this->assertStringNotContainsString( 'constructor_property_promotion::__construct has incomplete parameters list', $result @@ -293,10 +288,9 @@ public function test_phpdoc_tags_tests(): void { $xpath = new \DOMXpath($xmlresult); $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(5, $found->length); + $this->assertSame(3, $found->length); // Also verify various bits by content. - $this->assertStringContainsString('packagevalid', $result); $this->assertStringContainsString('Invalid phpdocs tag @small', $result); $this->assertStringContainsString('Invalid phpdocs tag @zzzing', $result); $this->assertStringContainsString('Invalid phpdocs tag @inheritdoc', $result); @@ -326,10 +320,7 @@ public function test_phpdoc_phpunit_coverage_ignored_for_file(): void { $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(1, $found->length); - - // Also verify various bits by content. - $this->assertStringContainsString('packagevalid', $result); + $this->assertSame(0, $found->length); } /** @@ -353,10 +344,9 @@ public function test_phpdoc_tags_inline(): void { $xpath = new \DOMXpath($xmlresult); $found = $xpath->query("//file/error"); // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(8, $found->length); + $this->assertSame(6, $found->length); // Also verify various bits by content. - $this->assertStringContainsString('packagevalid', $result); $this->assertStringContainsString('Invalid inline phpdocs tag @param found', $result); $this->assertStringContainsString('Invalid inline phpdocs tag @throws found', $result); $this->assertStringContainsString('Inline phpdocs tag {@link tags need to have a valid URL} with incorrect', $result); @@ -371,36 +361,6 @@ public function test_phpdoc_tags_inline(): void { $this->assertStringNotContainsString('ba8by}', $result); } - /** - * Verify the package tag is required for class/trait/interface/global scope functions. - * - * @covers ::local_moodlecheck_packagespecified - */ - public function test_phpdoc_tags_packagespecified(): void { - global $PAGE; - $output = $PAGE->get_renderer('local_moodlecheck'); - $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_tags_packagespecified.php', null); - $result = $output->display_path($path, 'xml'); - - // Convert results to XML Object. - $xmlresult = new \DOMDocument(); - $xmlresult->loadXML($result); - - // Let's verify we have received a xml with file top element and 4 children. - $xpath = new \DOMXpath($xmlresult); - $found = $xpath->query('//file/error[@source="packagespecified"]'); - // TODO: Change to DOMNodeList::count() when php71 support is gone. - $this->assertSame(4, $found->length); - - // Also verify various bits by content. - $this->assertStringContainsString('Package is not specified for class missingclass', $result); - $this->assertStringContainsString('Package is not specified for interface missinginterface', $result); - $this->assertStringContainsString('Package is not specified for trait missingtrait', $result); - $this->assertStringContainsString('Package is not specified for function missingfunction', $result); - $this->assertStringNotContainsString('packaged', $result); - $this->assertStringNotContainsString('somemethod', $result); - } - /** * Test that {@see local_moodlecheck_get_categories()} returns the correct list of allowed categories. *