diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ebc9e34..f7d30ce8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -155,7 +155,7 @@ jobs: - name: PHPUnit tests if: ${{ always() }} - run: moodle-plugin-ci phpunit --coverage-text + run: moodle-plugin-ci phpunit - name: Behat features if: ${{ always() }} diff --git a/index.php b/index.php index 6a27c25b..581ba387 100644 --- a/index.php +++ b/index.php @@ -91,6 +91,9 @@ } if ($fullpaths && !$failed) { + // Calculate the ignores. + $ignores = local_codesniffer_get_ignores($exclude); + // Let's use our own Runner, all we need is to pass some // configuration settings (reportfile, show warnings) and // override the init() method to set all our config options. @@ -101,7 +104,7 @@ $reportfile = make_temp_directory('phpcs') . '/phpcs_' . random_string(10) . '.xml'; $runner->set_reportfile($reportfile); $runner->set_includewarnings($includewarnings); - $runner->set_ignorepatterns(local_codesniffer_get_ignores($exclude)); + $runner->set_ignorepatterns($ignores); $runner->set_files($fullpaths); $runner->run(); @@ -109,8 +112,8 @@ // Load the XML file to proceed with the rest of checks. $xml = simplexml_load_file($reportfile); - // Look for other problems, not handled by codesniffer. - local_codechecker_check_other_files(local_codechecker_clean_path($fullpath), $xml); + // Look for other problems, not handled by codesniffer. Use same list of ignored (originally in keys, now in values). + local_codechecker_check_other_files(local_codechecker_clean_path($fullpath), $xml, array_keys($ignores)); list($numerrors, $numwarnings) = local_codechecker_count_problems($xml); // Output the results report. diff --git a/locallib.php b/locallib.php index 4f4eb082..38244251 100644 --- a/locallib.php +++ b/locallib.php @@ -158,8 +158,11 @@ function local_codesniffer_get_ignores($extraignorelist = '') { if ($extraignorelist) { $extraignorearr = explode(',', $extraignorelist); foreach ($extraignorearr as $extraignore) { - $extrapath = trim($extraignore); - $finalpaths[$extrapath] = 'absolute'; + // Don't register empty ignores. + if (trim($extraignore)) { + $extrapath = trim($extraignore); + $finalpaths[$extrapath] = 'absolute'; + } } } @@ -221,30 +224,60 @@ function local_codechecker_clean_path($path) { * * @param array $arr Array to add file paths to * @param string $folder Path to search (or may be a single file) + * @param array $ignores list of paths (substring matching, asterisk as wild-char that must be ignored). * @param array $extensions File extensions to include (not including .) */ -function local_codechecker_find_other_files(&$arr, $folder, - $extensions = array('txt', 'html', 'csv')) { - $regex = '~\.(' . implode('|', $extensions) . ')$~'; +function local_codechecker_find_other_files(&$arr, $folder, $ignores, $extensions = ['txt', 'html', 'csv']) { + + // To detect changes in the passed params. + static $stignores = []; + static $stextensions = []; + + // To set the regex only once while the params don't change). + static $regex = ''; + static $ignoresregex = ''; + + // If the ignores or the extensions have changed, recalculate regex expressions. + if ($stignores !== $ignores || $stextensions !== $extensions) { + // Save last params. + $stignores = $ignores; + $stextensions = $extensions; + + // Finder regex. + $regex = '~\.(' . implode('|', $extensions) . ')$~'; + + // Ignores regex. + $ignoresarr = []; + $ignoresregex = '~THIS_IS_A_NON_MATCHER~'; + foreach ($ignores as $ignore) { + $ignore = preg_quote($ignore); + $ignore = str_replace('\*', '.*', $ignore); + $ignoresarr[] = $ignore; + } + if ($ignoresarr) { + $ignoresregex = '~(' . implode('|', $ignoresarr) . ')~'; + } + } // Handle if this is called directly with a file and not folder. if (is_file($folder)) { - if (preg_match($regex, $folder)) { + if (preg_match($regex, $folder) && !preg_match($ignoresregex, $folder)) { $arr[] = $folder; } return; } - if ($handle = opendir($folder)) { + if (is_dir($folder)) { + $handle = opendir($folder); while (($file = readdir($handle)) !== false) { $fullpath = $folder . '/' . $file; if ($file === '.' || $file === '..') { continue; } else if (is_file($fullpath)) { - if (preg_match($regex, $fullpath)) { + if (preg_match($regex, $fullpath) && !preg_match($ignoresregex, $fullpath)) { $arr[] = $fullpath; } } else if (is_dir($fullpath)) { - local_codechecker_find_other_files($arr, $fullpath); + local_codechecker_find_other_files($arr, $fullpath, $ignores, $extensions); } } closedir($handle); @@ -348,10 +381,11 @@ function local_codechecker_check_other_file($file, $xml) { * @param string $path Path to search (may be file or folder) * @param SimpleXMLElement $xml structure containin all violations. * to which new problems will be added + * @param array $ignores list of paths (substring matching, asterisk as wild-char that must be ignored). */ -function local_codechecker_check_other_files($path, $xml) { +function local_codechecker_check_other_files($path, $xml, $ignores) { $files = array(); - local_codechecker_find_other_files($files, $path); + local_codechecker_find_other_files($files, $path, $ignores); foreach ($files as $file) { local_codechecker_check_other_file($file, $xml); } diff --git a/moodle/Util/MoodleUtil.php b/moodle/Util/MoodleUtil.php index 5f997be0..69c9092d 100644 --- a/moodle/Util/MoodleUtil.php +++ b/moodle/Util/MoodleUtil.php @@ -56,6 +56,7 @@ abstract class MoodleUtil { * @return bool True if the file has been loaded, false if not. */ protected static function loadCoreComponent(string $moodleRoot): bool { + global $CFG; // Safety check, in case core_component is missing. if (!file_exists($moodleRoot . '/lib/classes/component.php')) { @@ -66,13 +67,27 @@ protected static function loadCoreComponent(string $moodleRoot): bool { defined('IGNORE_COMPONENT_CACHE') ?: define('IGNORE_COMPONENT_CACHE', 1); defined('MOODLE_INTERNAL') ?: define('MOODLE_INTERNAL', 1); - unset($CFG); - global $CFG; - $CFG = new \stdClass(); - $CFG->dirroot = $moodleRoot; - $CFG->libdir = $CFG->dirroot . '/lib'; - $CFG->admin = 'admin'; - require_once($CFG->dirroot . '/lib/classes/component.php'); + // Save current CFG values. + $olddirroot = $CFG->dirroot ?? null; + $oldlibdir = $CFG->libdir ?? null; + $oldadmin = $CFG->admin ?? null; + + if (!isset($CFG->dirroot)) { // No defined, let's start from scratch. + $CFG = new \stdClass(); + } + + if ($CFG->dirroot !== $moodleRoot) { // Different, set the minimum required. + $CFG->dirroot = $moodleRoot; + $CFG->libdir = $CFG->dirroot . '/lib'; + $CFG->admin = 'admin'; + } + + require_once($CFG->dirroot . '/lib/classes/component.php'); // Load the class. + + // Restore original CFG values. + $CFG->dirroot = $olddirroot ?? null; + $CFG->libdir = $oldlibdir ?? null; + $CFG->admin = $oldadmin ?? null; return true; } diff --git a/tests/behat/ui.feature b/tests/behat/ui.feature index d3ac48a6..4a27ebc6 100644 --- a/tests/behat/ui.feature +++ b/tests/behat/ui.feature @@ -19,7 +19,7 @@ Feature: Codechecker UI works as expected | index2.php | Invalid path index2.php | Files found: 1 | | local/codechecker/version.php | Well done! | Invalid path | | local/codechecker/tests/ | local_codechecker_testcase.php | Invalid path | - | local/codechecker/tests/ | Files found: 2 | Invalid path | + | local/codechecker/tests/ | Files found: 6 | Invalid path | | local/codechecker/tests/ | Well done! | Invalid path | | local/codechecker/moodle/tests/fixtures/files/moodleinternal/problem.php | Files found: 1 | Invalid path | | local/codechecker/moodle/tests/fixtures/files/moodleinternal/problem.php | Total: 2 error(s) and 1 warning(s) | Well done! | diff --git a/tests/fixtures/one.txt b/tests/fixtures/one.txt new file mode 100644 index 00000000..5626abf0 --- /dev/null +++ b/tests/fixtures/one.txt @@ -0,0 +1 @@ +one diff --git a/tests/fixtures/three.txt b/tests/fixtures/three.txt new file mode 100644 index 00000000..2bdf67ab --- /dev/null +++ b/tests/fixtures/three.txt @@ -0,0 +1 @@ +three diff --git a/tests/fixtures/two.txt b/tests/fixtures/two.txt new file mode 100644 index 00000000..f719efd4 --- /dev/null +++ b/tests/fixtures/two.txt @@ -0,0 +1 @@ +two diff --git a/tests/locallib_test.php b/tests/locallib_test.php new file mode 100644 index 00000000..b28f7167 --- /dev/null +++ b/tests/locallib_test.php @@ -0,0 +1,146 @@ +. + +namespace local_codechecker; + +/** + * Tests related with local_codechecker locallib.php + * + * @package local_codechecker + * @category test + * @copyright 2022 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class locallib_test extends \basic_testcase { + + /** + * Data provider for test_local_codechecker_find_other_files() + */ + public function local_codechecker_find_other_files_provider() { + $defaultextensions = ['txt', 'html', 'csv']; + return [ + 'one wrong file' => [ + 'path' => 'local/codechecker/tests/nonono_test.php', + 'ignores' => [], + 'extensions' => $defaultextensions, + 'matches' => [\moodle_exception::class], + 'notmatches' => [], + ], + 'one wrong dir' => [ + 'path' => 'local/codechecker/nononotests/', + 'ignores' => [], + 'extensions' => $defaultextensions, + 'matches' => [\moodle_exception::class], + 'notmatches' => [], + ], + 'one file' => [ + 'path' => 'local/codechecker/tests/locallib_test.php', + 'ignores' => [], + 'extensions' => $defaultextensions, + 'matches' => [], + 'notmatches' => [], + ], + 'one php file' => [ + 'path' => 'local/codechecker/tests/locallib_test.php', + 'ignores' => [], + 'extensions' => ['php'], + 'matches' => ['local/codechecker/tests/locallib_test.php'], + 'notmatches' => [], + ], + 'one dir' => [ + 'path' => 'local/codechecker/tests', + 'ignores' => [], + 'extensions' => $defaultextensions, + 'matches' => ['one.txt', 'two.txt'], + 'nomatches' => [], + ], + 'one dir with php files' => [ + 'path' => 'local/codechecker/tests', + 'ignores' => [], + 'extensions' => array_merge($defaultextensions, ['php']), + 'matches' => ['locallib_test.php', 'one.txt', 'two.txt'], + 'nomatches' => [], + ], + 'one dir one ignored file' => [ + 'path' => 'local/codechecker/tests', + 'ignores' => ['one.txt'], + 'extensions' => $defaultextensions, + 'matches' => ['two.txt'], + 'nomatches' => ['one.txt'], + ], + 'one dir many ignored files' => [ + 'path' => 'local/codechecker/tests', + 'ignores' => ['one.txt', 'three.txt'], + 'extensions' => $defaultextensions, + 'matches' => ['two.txt'], + 'nomatches' => ['one.txt', 'three.txt'], + ], + 'one dir one wildchar ignored' => [ + 'path' => 'local/codechecker/tests', + 'ignores' => ['fixtures*three'], + 'extensions' => $defaultextensions, + 'matches' => ['one.txt', 'two.txt'], + 'nomatches' => ['three.txt'], + ], + ]; + } + + /** + * Verify local_codechecker_find_other_files() behaviour. + * + * @param string $path dirroot relative path to be examined (file or folder). + * @param string[] $ignores substring-matching, accepting wild-chars array strings to ignore. + * @param string[] $extensions list of extensions to look for. + * @param string[] $matches list of substring-matching strings expected to be in the results. + * @param string[] $nomatches list of substring-matching strings not expected to be in the results. + * + * @dataProvider local_codechecker_find_other_files_provider() + * @covers ::local_codechecker_find_other_files() + */ + public function test_local_codechecker_find_other_files(string $path, array $ignores, + array $extensions, array $matches, array $nomatches) { + + global $CFG; + require_once(__DIR__ . '/../locallib.php'); + + $results = []; + + // If matches has moodle_exception, then we expect it to happen. + if ($matches && $matches[0] === \moodle_exception::class) { + $this->expectException(\moodle_exception::class); + } + + // Look for results. + local_codechecker_find_other_files($results, $CFG->dirroot . '/' . $path, $ignores, $extensions); + + // Empty results, means we expect also empty matches. + if (empty($results)) { + $this->assertSame($matches, $results); + return; // We have ended, no more assertions for this case. + } + + // We have results, let's check them. + $results = implode(' ', $results); + + // Let's perform simple substring matching, that's enough. + foreach ($matches as $match) { + $this->assertStringContainsString($match, $results); + } + foreach ($nomatches as $nomatch) { + $this->assertStringNotContainsString($nomatch, $results); + } + } +}