Merge pull request moodlehq#91 from stronk7/todo_commenting_sniff

Check TODO and @todo comments looking for extra info

moodle/Sniffs/Commenting/TodoCommentSniff.php

@@ -0,0 +1,186 @@
+<?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/>.
+
+/**
+ * Checks that some comments have a corresponding issue in the tracker.
+ *
+ * This sniff checks that both inline TODO comments and phpdoc @todo tags
+ * have a corresponding issue in the tracker.
+ *
+ * @copyright 2024 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
+ * @license   https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace MoodleHQ\MoodleCS\moodle\Sniffs\Commenting;
+
+// phpcs:disable moodle.NamingConventions
+
+use PHP_CodeSniffer\Config;
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Sniffs\Sniff;
+
+class TodoCommentSniff implements Sniff
+{
+    /**
+     * The regular expression to match comments against.
+     *
+     * Note that the regular expression is applied as is,
+     * so it must be escaped if needed to. Partial matching
+     * is enough to consider the comment as correct TODO.
+     *
+     * Note that, if the regular expression is the empty string,
+     * then this Sniff will do nothing.
+     *
+     * Example values:
+     * - 'MDL-[0-9]+': The default value, a Moodle core issue is required.
+     * - 'CONTRIB-[0-9]+': A Moodle plugin issue is required.
+     * - '(MDL|CONTRIB)-[0-9]+': A Moodle core or plugin issue is required.
+     * - 'https://': Any URL is required.
+     * - '' (empty string or null): No check is done.
+     */
+    public ?string $commentRequiredRegex = 'MDL-[0-9]+';
+
+    /**
+     * Returns an array of tokens this Sniff wants to listen for.
+     *
+     * @return array<int|string>
+     */
+    public function register(): array {
+        return [T_COMMENT, T_DOC_COMMENT_TAG];
+    }
+
+    /**
+     * Processes this Sniff, when one of its tokens is encountered.
+     *
+     * @param File $phpcsFile The file being scanned.
+     * @param int $stackPtr The position of the current token in the stack passed in $tokens.
+     *
+     * @return void
+     */
+    public function process(File $phpcsFile, $stackPtr): void {
+        // If specified, get the regular expression from the config.
+        if (($regex = Config::getConfigData('moodleTodoCommentRegex')) !== null) {
+            $this->commentRequiredRegex = $regex;
+        }
+
+        // If the regular expression is empty, then we don't want to do anything.
+        if (empty($this->commentRequiredRegex)) {
+            return;
+        }
+
+        $tokens = $phpcsFile->getTokens();
+
+        if ($tokens[$stackPtr]['code'] === T_COMMENT) {
+            // Found an inline comment, let's process it.
+            $this->processInlineComment($phpcsFile, $stackPtr);
+        } elseif ($tokens[$stackPtr]['code'] === T_DOC_COMMENT_TAG) {
+            // Found a phpdoc tag, let's process it.
+            $this->processDocCommentTag($phpcsFile, $stackPtr);
+        }
+    }
+
+    protected function processInlineComment(File $phpcsFile, int $stackPtr): void {
+        $tokens = $phpcsFile->getTokens();
+
+        // If the previous token is also an inline comment, then
+        // we already have processed this comment before.
+        $previousToken = $phpcsFile->findPrevious(T_COMMENT, ($stackPtr - 1), ($stackPtr - 1), false);
+        if ($tokens[$stackPtr]['line'] === ($tokens[$previousToken]['line'] + 1)) {
+            return;
+        }
+
+        // Only want inline comments.
+        if (substr($tokens[$stackPtr]['content'], 0, 2) !== '//') {
+            return;
+        }
+
+        // Get the content of the whole inline comment, excluding whitespace.
+        $commentContent = trim($tokens[$stackPtr]['content']);
+        $nextComment = $stackPtr;
+        $lastComment = $stackPtr;
+        while (($nextComment = $phpcsFile->findNext(T_COMMENT, ($nextComment + 1), null, false)) !== false) {
+            // Until we get a token in non-consecutive line (inline comments are consecutive).
+            if ($tokens[$nextComment]['line'] !== ($tokens[$lastComment]['line'] + 1)) {
+                break;
+            }
+            $commentContent .= ' ' . trim(substr($tokens[$nextComment]['content'], 2));
+            $lastComment = $nextComment;
+        }
+
+        // Time to analise the comment contents.
+
+        // Only want inline comments starting with TODO (ignoring the colon existence /
+        // absence on purpose, it's not important for this Sniff).
+        if (strpos($commentContent, '// TODO') === false) {
+            return;
+        }
+
+        // Check if the inline comment has the required information.
+        $this->evaluateComment($phpcsFile, $stackPtr, 'inline', $commentContent);
+    }
+
+    protected function processDocCommentTag(File $phpcsFile, int $stackPtr): void {
+        $tokens = $phpcsFile->getTokens();
+
+        // We are only interested in @todo tags.
+        if ($tokens[$stackPtr]['content'] !== '@todo') {
+            return;
+        }
+
+        // Get the content of the whole @todo tag, until another tag or phpdoc block ends.
+        $commentContent = '';
+        $nextComment = $stackPtr;
+        $tags = [T_DOC_COMMENT_STRING, T_DOC_COMMENT_TAG, T_DOC_COMMENT_CLOSE_TAG];
+        while (($nextComment = $phpcsFile->findNext($tags, ($nextComment + 1), null, false)) !== false) {
+            // Until we get another tag or the end of the phpdoc block.
+            if (
+                $tokens[$nextComment]['code'] === T_DOC_COMMENT_TAG ||
+                $tokens[$nextComment]['code'] === T_DOC_COMMENT_CLOSE_TAG
+            ) {
+                break;
+            }
+            $commentContent .= ' ' . trim($tokens[$nextComment]['content']);
+        }
+
+        // Time to analise the comment contents.
+
+        // Check if the inline comment has the required information.
+        $this->evaluateComment($phpcsFile, $stackPtr, 'phpdoc', $commentContent);
+    }
+
+    protected function evaluateComment(
+        File $phpcsFile,
+        int $stackPtr,
+        string $type,
+        string $commentContent
+    ): void {
+        // Just verify that the comment matches the required regular expression.
+        if (preg_match('~' . $this->commentRequiredRegex . '~', $commentContent)) {
+            return;
+        }
+
+        // Arrived here, no match, create a warning with all the info.
+        $error = 'Missing required "%s" information in %s comment: %s';
+        $errorParams = [
+            $this->commentRequiredRegex,
+            $type,
+            trim($commentContent),
+        ];
+        $code = $type === 'phpdoc' ? 'MissingInfoPhpdoc' : 'MissingInfoInline';
+        $phpcsFile->addWarning($error, $stackPtr, $code, $errorParams);
+    }
+}

moodle/Tests/MoodleCSBaseTestCase.php

@@ -16,6 +16,8 @@
 
 namespace MoodleHQ\MoodleCS\moodle\Tests;
 
+use PHP_CodeSniffer\Config;
+
 /**
  * Specialized test case for easy testing of "moodle" standard sniffs.
  *
@@ -57,6 +59,11 @@ abstract class MoodleCSBaseTestCase extends \PHPUnit\Framework\TestCase {
      */
     protected $fixture = null;
 
+    /**
+     * @var array custom config elements to setup before running phpcs. name => value.
+     */
+    protected $customConfigs = [];
+
     /**
      * @var array error expectations to ve verified against execution results.
      */
@@ -69,6 +76,10 @@ abstract class MoodleCSBaseTestCase extends \PHPUnit\Framework\TestCase {
 
     public function tearDown(): void {
         \MoodleHQ\MoodleCS\moodle\Util\MoodleUtil::setMockedComponentMappings([]);
+        // If there are custom configs setup, remove them.
+        foreach (array_keys($this->customConfigs) as $key) {
+            Config::setConfigData($key, null, true);
+        }
     }
 
     public function set_component_mapping(array $mapping): void {
@@ -152,6 +163,20 @@ protected function set_errors(array $errors) {
         }
     }
 
+    /**
+     * Adds a custom config element to be setup before running phpcs.
+     *
+     * Note that those config elements will be automatically removed
+     * after each test case (by tearDown())
+     *
+     * @param string $key config key or name.
+     * @param string $value config value.
+     */
+    protected function add_custom_config(string $key, string $value): void {
+        $this->customConfigs[$key] = $value;
+        Config::setConfigData($key, $value, true);
+    }
+
     /**
      * Set the warning expectations to ve verified against execution results.
      *

moodle/Tests/Sniffs/Commenting/TodoCommentSniffTest.php

@@ -0,0 +1,109 @@
+<?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 MoodleHQ\MoodleCS\moodle\Tests\Sniffs\Commenting;
+
+use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase;
+
+// phpcs:disable moodle.NamingConventions
+
+/**
+ * Test the TestCaseNamesSniff sniff.
+ *
+ * @category   test
+ * @copyright  2024 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
+ * @license    https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ *
+ * @covers \MoodleHQ\MoodleCS\moodle\Sniffs\Commenting\TodoCommentSniff
+ */
+class TodoCommentSniffTest extends MoodleCSBaseTestCase
+{
+    public function testComentingTodoComment(): void {
+        // Define the standard, sniff and fixture to use.
+        $this->set_standard('moodle');
+        $this->set_sniff('moodle.Commenting.TodoComment');
+        $this->set_fixture(__DIR__ . '/../../fixtures/Commenting/TodoComment.php');
+
+        // Define expected results (errors and warnings). Format, array of:
+        // - 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).
+        $errors = [];
+        $warnings = [
+            8  => 'Missing required "MDL-[0-9]+"',
+            10 => 'Missing required "MDL-[0-9]+"',
+            12 => 'TodoComment.MissingInfoInline',
+            16 => 'TodoComment.MissingInfoPhpdoc',
+            23 => 'comment: In the middle',
+            25 => 'take 2',
+            27 => 'take 3',
+            33 => 'information in inline comment',
+            34 => 'information in phpdoc comment',
+        ];
+        $this->set_errors($errors);
+        $this->set_warnings($warnings);
+
+        // Let's do all the hard work!
+        $this->verify_cs_results();
+    }
+
+    public function testEmptyConfigValue(): void {
+        // Define the standard, sniff and fixture to use.
+        $this->set_standard('moodle');
+        $this->set_sniff('moodle.Commenting.TodoComment');
+        $this->set_fixture(__DIR__ . '/../../fixtures/Commenting/TodoCommentEmptyConfig.php');
+
+        // Try with an empty config value.
+        $this->add_custom_config('moodleTodoCommentRegex', '');
+
+        // Define expected results (errors and warnings). Format, array of:
+        // - 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).
+        $errors = [];
+        $warnings = [];
+        $this->set_errors($errors);
+        $this->set_warnings($warnings);
+
+        // Let's do all the hard work!
+        $this->verify_cs_results();
+    }
+
+    public function testCustomConfigValue(): void {
+            // Define the standard, sniff and fixture to use.
+            $this->set_standard('moodle');
+            $this->set_sniff('moodle.Commenting.TodoComment');
+            $this->set_fixture(__DIR__ . '/../../fixtures/Commenting/TodoCommentCustomConfig.php');
+
+            // Try with an empty config value.
+            $this->add_custom_config('moodleTodoCommentRegex', 'CUSTOM-[0-9]+');
+
+            // Define expected results (errors and warnings). Format, array of:
+            // - 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).
+            $errors = [];
+            $warnings = [
+                8 => 'Missing required "CUSTOM-[0-9]+"',
+                9 => 'Missing required "CUSTOM-[0-9]+"',
+            ];
+            $this->set_errors($errors);
+            $this->set_warnings($warnings);
+
+            // Let's do all the hard work!
+            $this->verify_cs_results();
+    }
+}