Skip to content

Commit

Permalink
Generic/Fixme: improve handling of "fixme" tags in docblocks
Browse files Browse the repository at this point in the history
Essentially the same fix as applied in the sister-commit for the `Generic.Commenting.Todo` sniff.

Until now, the sniff would only examine an individual comment token, while when a `@fixme` tag is used in a docblock, the "task description" is normally in the next `T_DOC_COMMENT_STRING` token.

This commit fixes this and the sniff will now take docblock `@fixme` tags into account.

Includes additional unit tests.
  • Loading branch information
jrfnl committed Nov 11, 2023
1 parent 30e7f92 commit 120c223
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 16 deletions.
44 changes: 28 additions & 16 deletions src/Standards/Generic/Sniffs/Commenting/FixmeSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;

class FixmeSniff implements Sniff
{
Expand Down Expand Up @@ -56,27 +55,40 @@ public function register()
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

$tokens = $phpcsFile->getTokens();
$content = $tokens[$stackPtr]['content'];
$matches = [];
preg_match('/(?:\A|[^\p{L}]+)fixme([^\p{L}]+(.*)|\Z)/ui', $content, $matches);
if (empty($matches) === false) {
// Clear whitespace and some common characters not required at
// the end of a fixme message to make the error more informative.
$type = 'CommentFound';
$fixmeMessage = trim($matches[1]);
$fixmeMessage = trim($fixmeMessage, '-:[](). ');
$error = 'Comment refers to a FIXME task';
$data = [$fixmeMessage];
if ($fixmeMessage !== '') {
$type = 'TaskFound';
$error .= ' "%s"';

if (preg_match('/(?:\A|[^\p{L}]+)fixme([^\p{L}]+(.*)|\Z)/ui', $content, $matches) !== 1) {
return;
}

$fixmeMessage = trim($matches[1]);
// Clear whitespace and some common characters not required at
// the end of a to-do message to make the warning more informative.
$fixmeMessage = trim($fixmeMessage, '-:[](). ');

if ($tokens[$stackPtr]['code'] === T_DOC_COMMENT_TAG
&& $fixmeMessage === ''
) {
$nextNonEmpty = $phpcsFile->findNext(T_DOC_COMMENT_WHITESPACE, ($stackPtr + 1), null, true);
if ($nextNonEmpty !== false
&& $tokens[$nextNonEmpty]['code'] === T_DOC_COMMENT_STRING
) {
$fixmeMessage = trim($tokens[$nextNonEmpty]['content'], '-:[](). ');
}
}

$phpcsFile->addError($error, $stackPtr, $type, $data);
$error = 'Comment refers to a FIXME task';
$type = 'CommentFound';
$data = [$fixmeMessage];
if ($fixmeMessage !== '') {
$error .= ' "%s"';
$type = 'TaskFound';
}

$phpcsFile->addError($error, $stackPtr, $type, $data);

}//end process()


Expand Down
14 changes: 14 additions & 0 deletions src/Standards/Generic/Tests/Commenting/FixmeUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,17 @@ Debug::bam('test');
//FIXME.
//éfixme
//fixmeé

/**
* While there is no official "fix me" tag, only `@todo`, let's support a tag for the purpose of this sniff anyway.
*
* @fixme This message should be picked up.
* @fixme: This message should be picked up too.
* @fixme - here is a message
*
* The below should not show a message as there is no description associated with the tag.
* @fixme
* @anothertag
*
* @param string $something FIXME: add description
*/
14 changes: 14 additions & 0 deletions src/Standards/Generic/Tests/Commenting/FixmeUnitTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,17 @@ alert('test');
//FIXME.
//éfixme
//fixmeé

/**
* While there is no official "fix me" tag, only `@todo`, let's support a tag for the purpose of this sniff anyway.
*
* @fixme This message should be picked up.
* @fixme: This message should be picked up too.
* @fixme - here is a message
*
* The below should not show a message as there is no description associated with the tag.
* @fixme
* @anothertag
*
* @param string $something FIXME: add description
*/
5 changes: 5 additions & 0 deletions src/Standards/Generic/Tests/Commenting/FixmeUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ public function getErrorList($testFile='FixmeUnitTest.inc')
16 => 1,
18 => 1,
21 => 1,
28 => 1,
29 => 1,
30 => 1,
33 => 1,
36 => 1,
];

}//end getErrorList()
Expand Down

0 comments on commit 120c223

Please sign in to comment.