Merge pull request #180 from stronk7/allow_codecoverageignore

Allow codecoverageignore
moodlehq · Feb 2, 2022 · 21cab22 · 21cab22
2 parents 1bb6603 + 271e36a
commit 21cab22
Show file tree

Hide file tree

Showing 11 changed files with 72 additions and 45 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -14,6 +14,14 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
+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
@@ -14,6 +14,14 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
+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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);