Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check TODO and @todo comments looking for extra info #91

Merged
merged 2 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 186 additions & 0 deletions moodle/Sniffs/Commenting/TodoCommentSniff.php
Original file line number Diff line number Diff line change
@@ -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]+';
stronk7 marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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);
}
}
25 changes: 25 additions & 0 deletions moodle/Tests/MoodleCSBaseTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

namespace MoodleHQ\MoodleCS\moodle\Tests;

use PHP_CodeSniffer\Config;

/**
* Specialized test case for easy testing of "moodle" standard sniffs.
*
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
*
Expand Down
109 changes: 109 additions & 0 deletions moodle/Tests/Sniffs/Commenting/TodoCommentSniffTest.php
Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading