Skip to content

Commit 30e7f92

Browse files
committed
Generic/Todo: improve handling of "todo" tags in docblocks
Until now, the sniff would only examine an individual comment token, while when a `@todo` 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 `@todo` tags into account. Includes additional unit tests.
1 parent 223ec03 commit 30e7f92

File tree

4 files changed

+57
-16
lines changed

4 files changed

+57
-16
lines changed

src/Standards/Generic/Sniffs/Commenting/TodoSniff.php

+28-16
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
use PHP_CodeSniffer\Files\File;
1313
use PHP_CodeSniffer\Sniffs\Sniff;
14-
use PHP_CodeSniffer\Util\Tokens;
1514

1615
class TodoSniff implements Sniff
1716
{
@@ -55,27 +54,40 @@ public function register()
5554
*/
5655
public function process(File $phpcsFile, $stackPtr)
5756
{
58-
$tokens = $phpcsFile->getTokens();
59-
57+
$tokens = $phpcsFile->getTokens();
6058
$content = $tokens[$stackPtr]['content'];
6159
$matches = [];
62-
preg_match('/(?:\A|[^\p{L}]+)todo([^\p{L}]+(.*)|\Z)/ui', $content, $matches);
63-
if (empty($matches) === false) {
64-
// Clear whitespace and some common characters not required at
65-
// the end of a to-do message to make the warning more informative.
66-
$type = 'CommentFound';
67-
$todoMessage = trim($matches[1]);
68-
$todoMessage = trim($todoMessage, '-:[](). ');
69-
$error = 'Comment refers to a TODO task';
70-
$data = [$todoMessage];
71-
if ($todoMessage !== '') {
72-
$type = 'TaskFound';
73-
$error .= ' "%s"';
60+
61+
if (preg_match('/(?:\A|[^\p{L}]+)todo([^\p{L}]+(.*)|\Z)/ui', $content, $matches) !== 1) {
62+
return;
63+
}
64+
65+
$todoMessage = trim($matches[1]);
66+
// Clear whitespace and some common characters not required at
67+
// the end of a to-do message to make the warning more informative.
68+
$todoMessage = trim($todoMessage, '-:[](). ');
69+
70+
if ($tokens[$stackPtr]['code'] === T_DOC_COMMENT_TAG
71+
&& $todoMessage === ''
72+
) {
73+
$nextNonEmpty = $phpcsFile->findNext(T_DOC_COMMENT_WHITESPACE, ($stackPtr + 1), null, true);
74+
if ($nextNonEmpty !== false
75+
&& $tokens[$nextNonEmpty]['code'] === T_DOC_COMMENT_STRING
76+
) {
77+
$todoMessage = trim($tokens[$nextNonEmpty]['content'], '-:[](). ');
7478
}
79+
}
7580

76-
$phpcsFile->addWarning($error, $stackPtr, $type, $data);
81+
$error = 'Comment refers to a TODO task';
82+
$type = 'CommentFound';
83+
$data = [$todoMessage];
84+
if ($todoMessage !== '') {
85+
$error .= ' "%s"';
86+
$type = 'TaskFound';
7787
}
7888

89+
$phpcsFile->addWarning($error, $stackPtr, $type, $data);
90+
7991
}//end process()
8092

8193

src/Standards/Generic/Tests/Commenting/TodoUnitTest.inc

+12
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,15 @@ Debug::bam('test');
2121
//TODO.
2222
//étodo
2323
//todoé
24+
25+
/**
26+
* @todo This message should be picked up.
27+
* @todo: This message should be picked up too.
28+
* @todo - here is a message
29+
*
30+
* The below should not show a message as there is no description associated with the tag.
31+
* @todo
32+
* @anothertag
33+
*
34+
* @param string $something TODO: add description
35+
*/

src/Standards/Generic/Tests/Commenting/TodoUnitTest.js

+12
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,15 @@ alert('test');
2121
//TODO.
2222
//étodo
2323
//todoé
24+
25+
/**
26+
* @todo This message should be picked up.
27+
* @todo: This message should be picked up too.
28+
* @todo - here is a message
29+
*
30+
* The below should not show a message as there is no description associated with the tag.
31+
* @todo
32+
* @anothertag
33+
*
34+
* @param string $something TODO: add description
35+
*/

src/Standards/Generic/Tests/Commenting/TodoUnitTest.php

+5
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ public function getWarningList($testFile='TodoUnitTest.inc')
5353
16 => 1,
5454
18 => 1,
5555
21 => 1,
56+
26 => 1,
57+
27 => 1,
58+
28 => 1,
59+
31 => 1,
60+
34 => 1,
5661
];
5762

5863
}//end getWarningList()

0 commit comments

Comments
 (0)