Skip to content

Commit

Permalink
Merge pull request #183 from stronk7/apply_ignores_to_other_checks
Browse files Browse the repository at this point in the history
Apply all the standard ignores to the "other" checks
  • Loading branch information
andrewnicols authored Feb 14, 2022
2 parents 31dc2ff + 8f8ca5d commit a556e9b
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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() }}
Expand Down
9 changes: 6 additions & 3 deletions index.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -101,16 +104,16 @@
$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();

// 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.
Expand Down
56 changes: 45 additions & 11 deletions locallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
29 changes: 22 additions & 7 deletions moodle/Util/MoodleUtil.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/behat/ui.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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! |
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/one.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
one
1 change: 1 addition & 0 deletions tests/fixtures/three.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
three
1 change: 1 addition & 0 deletions tests/fixtures/two.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
two
146 changes: 146 additions & 0 deletions tests/locallib_test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
<?php
// This file is part of Moodle - https://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <https://www.gnu.org/licenses/>.

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);
}
}
}

0 comments on commit a556e9b

Please sign in to comment.