diff --git a/moodle/Sniffs/Commenting/InlineCommentSniff.php b/moodle/Sniffs/Commenting/InlineCommentSniff.php index 916fd756..74ac1284 100644 --- a/moodle/Sniffs/Commenting/InlineCommentSniff.php +++ b/moodle/Sniffs/Commenting/InlineCommentSniff.php @@ -419,7 +419,9 @@ public function process(File $phpcsFile, $stackPtr) { return; } - if (preg_match('!^([A-Z0-9]|\.{3})!', $commentText) === 0) { + // Enforce capital letter, digit or 3-dots sequence. Also allow @codeCoverageIgnore + // for better handling unreachable/uncovered code for coverage purposes. + if (preg_match('!^([A-Z0-9]|\.{3}|@codeCoverageIgnore)!', $commentText) === 0) { $error = 'Inline comments must start with a capital letter, digit or 3-dots sequence'; $phpcsFile->addWarning($error, $stackPtr, 'NotCapital'); } diff --git a/moodle/Sniffs/Files/BoilerplateCommentSniff.php b/moodle/Sniffs/Files/BoilerplateCommentSniff.php index d0aff345..eb6ab29c 100644 --- a/moodle/Sniffs/Files/BoilerplateCommentSniff.php +++ b/moodle/Sniffs/Files/BoilerplateCommentSniff.php @@ -55,7 +55,7 @@ public function process(File $file, $stackptr) { // We only want to do this once per file. $prevopentag = $file->findPrevious(T_OPEN_TAG, $stackptr - 1); if ($prevopentag !== false) { - return; + return; // @codeCoverageIgnore } if ($stackptr > 0) { diff --git a/moodle/Sniffs/Files/MoodleInternalSniff.php b/moodle/Sniffs/Files/MoodleInternalSniff.php index 9a715301..cf3bcbda 100644 --- a/moodle/Sniffs/Files/MoodleInternalSniff.php +++ b/moodle/Sniffs/Files/MoodleInternalSniff.php @@ -75,7 +75,7 @@ public function process(File $file, $pointer) { // We only want to do this once per file. $prevopentag = $file->findPrevious(T_OPEN_TAG, $pointer - 1); if ($prevopentag !== false) { - return; + return; // @codeCoverageIgnore } // Find where real code is and check from there. @@ -321,7 +321,6 @@ private function code_changes_global_state(File $file, $start, $end) { continue; } - // Ignore function/class prefixes. if (isset(Tokens::$methodPrefixes[$tokens[$i]['code']]) === true) { continue; diff --git a/moodle/Sniffs/Files/RequireLoginSniff.php b/moodle/Sniffs/Files/RequireLoginSniff.php index a68f6ef2..3f3e2354 100644 --- a/moodle/Sniffs/Files/RequireLoginSniff.php +++ b/moodle/Sniffs/Files/RequireLoginSniff.php @@ -47,7 +47,7 @@ public function process(File $file, $pointer) { // We only want to do this once per file. $prevopentag = $file->findPrevious(T_OPEN_TAG, $pointer - 1); if ($prevopentag !== false) { - return; + return; // @codeCoverageIgnore } $pointer = $this->get_config_inclusion_position($file, $pointer); diff --git a/moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php b/moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php index b461fa55..2c80845b 100644 --- a/moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php +++ b/moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php @@ -14,6 +14,14 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +namespace MoodleCodeSniffer\moodle\Sniffs\PHP; + +// phpcs:disable moodle.NamingConventions + +use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\DeprecatedFunctionsSniff as GenericDeprecatedFunctionsSniff; +use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Files\File; + /** * Sniff for various Moodle deprecated functions which uses should be replaced. * @@ -38,13 +46,6 @@ * @copyright 2021 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -namespace MoodleCodeSniffer\moodle\Sniffs\PHP; - -use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\DeprecatedFunctionsSniff as GenericDeprecatedFunctionsSniff; -use PHP_CodeSniffer\Sniffs\Sniff; -use PHP_CodeSniffer\Files\File; - class DeprecatedFunctionsSniff extends GenericDeprecatedFunctionsSniff { /** @@ -83,8 +84,7 @@ class DeprecatedFunctionsSniff extends GenericDeprecatedFunctionsSniff { * @todo: This method can be removed once/if this PR accepted: * https://github.com/squizlabs/PHP_CodeSniffer/pull/3295 */ - protected function addError($phpcsFile, $stackPtr, $function, $pattern=null) - { + protected function addError($phpcsFile, $stackPtr, $function, $pattern=null) { $data = [$function]; $error = 'Function %s() has been deprecated'; $type = 'Deprecated'; diff --git a/moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php b/moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php index 62262373..fb517039 100644 --- a/moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php +++ b/moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php @@ -14,6 +14,14 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +namespace MoodleCodeSniffer\moodle\Sniffs\PHP; + +// phpcs:disable moodle.NamingConventions + +use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff as GenericForbiddenFunctionsSniff; +use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Files\File; + /** * Sniff for debugging and other functions that we don't want used in finished code. * @@ -38,13 +46,6 @@ * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -namespace MoodleCodeSniffer\moodle\Sniffs\PHP; - -use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff as GenericForbiddenFunctionsSniff; -use PHP_CodeSniffer\Sniffs\Sniff; -use PHP_CodeSniffer\Files\File; - class ForbiddenFunctionsSniff extends GenericForbiddenFunctionsSniff { /** diff --git a/moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php b/moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php index ea8c69b7..6e8206d1 100644 --- a/moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php +++ b/moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php @@ -52,6 +52,9 @@ public function process(File $file, $pointer) { // Get the moodle branch being analysed. $moodleBranch = MoodleUtil::getMoodleBranch($file); + // Detect if we are running PHPUnit. + $runningPHPUnit = defined('PHPUNIT_TEST') && PHPUNIT_TEST; + // We have all we need from core, let's start processing the file. // Get the file tokens, for ease of use. @@ -64,29 +67,25 @@ public function process(File $file, $pointer) { // We only want to do this once per file. $prevopentag = $file->findPrevious(T_OPEN_TAG, $pointer - 1); if ($prevopentag !== false) { - return; + return; // @codeCoverageIgnore } // If we aren't checking Moodle 4.0dev (400) and up, nothing to check. - if (isset($moodleBranch) && $moodleBranch < 400) { - // Make and exception for codechecker phpunit tests, so they are run always. - if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) { - return; - } + // Make and exception for codechecker phpunit tests, so they are run always. + if (isset($moodleBranch) && $moodleBranch < 400 && !$runningPHPUnit) { + return; // @codeCoverageIgnore } // If the file isn't under tests directory, nothing to check. if (strpos($file->getFilename(), '/tests/') === false) { - return; + return; // @codeCoverageIgnore } // If the file isn't called, _test.php, nothing to check. + // Make an exception for codechecker own phpunit fixtures here, allowing any name for them. $fileName = basename($file->getFilename()); - if (substr($fileName, -9) !== '_test.php') { - // Make an exception for codechecker own phpunit fixtures here, allowing any name for them. - if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) { - return; - } + if (substr($fileName, -9) !== '_test.php' && !$runningPHPUnit) { + return; // @codeCoverageIgnore } // Iterate over all the classes (hopefully only one, but that's not this sniff problem). diff --git a/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php b/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php index dd9456f6..6b27c9d4 100644 --- a/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php +++ b/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php @@ -68,6 +68,9 @@ public function process(File $file, $pointer) { // Guess moodle component (from $file being processed). $moodleComponent = MoodleUtil::getMoodleComponent($file); + // Detect if we are running PHPUnit. + $runningPHPUnit = defined('PHPUNIT_TEST') && PHPUNIT_TEST; + // We have all we need from core, let's start processing the file. // Get the file tokens, for ease of use. @@ -76,27 +79,25 @@ public function process(File $file, $pointer) { // We only want to do this once per file. $prevopentag = $file->findPrevious(T_OPEN_TAG, $pointer - 1); if ($prevopentag !== false) { - return; + return; // @codeCoverageIgnore } // If the file isn't under tests directory, nothing to check. if (strpos($file->getFilename(), '/tests/') === false) { - return; + return; // @codeCoverageIgnore } // If the file isn't called, _test.php, nothing to check. + // Make an exception for codechecker own phpunit fixtures here, allowing any name for them. $fileName = basename($file->getFilename()); - if (substr($fileName, -9) !== '_test.php') { - // Make an exception for codechecker own phpunit fixtures here, allowing any name for them. - if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) { - return; - } + if (substr($fileName, -9) !== '_test.php' && !$runningPHPUnit) { + return; // @codeCoverageIgnore } // In order to cover the duplicates detection, we need to set some // properties (caches) here. It's extremely hard to do // this via mocking / extending (at very least for this humble developer). - if (defined('PHPUNIT_TEST') && PHPUNIT_TEST) { + if ($runningPHPUnit) { $this->prepareCachesForPHPUnit(); } diff --git a/moodle/tests/fixtures/moodle_comenting_inlinecomment.php b/moodle/tests/fixtures/moodle_comenting_inlinecomment.php index a41b4c0e..6eb44b90 100644 --- a/moodle/tests/fixtures/moodle_comenting_inlinecomment.php +++ b/moodle/tests/fixtures/moodle_comenting_inlinecomment.php @@ -129,3 +129,9 @@ final function afunction() {} new class testphpdoc {} /** This is a phpdoc block */ return new class implements something {} + +// Allow @codeCoverageIgnore inline comments. +$something = 1; // @codeCoverageIgnore +$something = 1;// @codeCoverageIgnoreStart +$something = 1; // @codeCoverageIgnoreEnd +$something = 1; // @codeCoverageIgnoreAnythingInvented diff --git a/moodle/tests/moodlestandard_test.php b/moodle/tests/moodlestandard_test.php index 4bebe322..cfbc5602 100644 --- a/moodle/tests/moodlestandard_test.php +++ b/moodle/tests/moodlestandard_test.php @@ -91,7 +91,7 @@ public function test_moodle_commenting_inlinecomment() { // - line => number of problems, or // - line => array of contents for message / source problem matching. // - line => string of contents for message / source problem matching (only 1). - $this->set_errors(array( + $this->set_errors([ 4 => array('3 slashes comments are not allowed'), 6 => 1, 8 => 'No space found before comment text', @@ -108,8 +108,13 @@ public function test_moodle_commenting_inlinecomment() { 124 => 1, 126 => 1, 128 => 1, - 130 => 1)); - $this->set_warnings(array( + 130 => 1, + 134 => 0, + 135 => 0, + 136 => 0, + 137 => 0, + ]); + $this->set_warnings([ 4 => 0, 6 => array(null, 'Commenting.InlineComment.InvalidEndChar'), 55 => array('19 found'), @@ -125,7 +130,8 @@ public function test_moodle_commenting_inlinecomment() { 77 => 1, 79 => 1, 118 => 0, - 122 => 0)); + 122 => 0, + ]); // Let's do all the hard work! $this->verify_cs_results(); diff --git a/moodle/tests/moodleutil_test.php b/moodle/tests/moodleutil_test.php index 82c1725a..0ce9295a 100644 --- a/moodle/tests/moodleutil_test.php +++ b/moodle/tests/moodleutil_test.php @@ -83,6 +83,19 @@ public function test_calculateAllComponents() { $this->assertSame(["{$moodleRoot}/mod/forum", "{$moodleRoot}/local/codechecker"], array_values($loadedComponents)); // Verify component paths are also the expected ones. + // Now be evil and try with an unreadable file, it must throw an exception. + + $this->cleanMoodleUtilCaches(); // Need to clean previous cached values. + Config::setConfigData('moodleComponentsListPath', '/path/to/non/readable/file', true); + + // We cannot use expectException() here, because we need to clean caches at the end. + try { + $method->invokeArgs(null, [$moodleRoot]); + $this->fail('\PHP_CodeSniffer\Exceptions\DeepExitException was expected, got none'); + } catch (\Exception $e) { + $this->assertInstanceOf(\PHP_CodeSniffer\Exceptions\DeepExitException::class, $e); + } + // Ensure cached information doesn't affect other tests. $this->cleanMoodleUtilCaches(); Config::setConfigData('moodleComponentsListPath', null, true);