Generic/Fixme: improve handling of "fixme" tags in docblocks

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.
PHPCSStandards · Nov 9, 2023 · 4b9da4f · 4b9da4f
1 parent 40d5473
commit 4b9da4f
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 16 deletions.
diff --git a/src/Standards/Generic/Sniffs/Commenting/FixmeSniff.php b/src/Standards/Generic/Sniffs/Commenting/FixmeSniff.php
@@ -12,7 +12,6 @@
 
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
-use PHP_CodeSniffer\Util\Tokens;
 
 class FixmeSniff implements Sniff
 {
@@ -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()
 
 

diff --git a/src/Standards/Generic/Tests/Commenting/FixmeUnitTest.inc b/src/Standards/Generic/Tests/Commenting/FixmeUnitTest.inc
@@ -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
+ */
diff --git a/src/Standards/Generic/Tests/Commenting/FixmeUnitTest.js b/src/Standards/Generic/Tests/Commenting/FixmeUnitTest.js
@@ -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
+ */
diff --git a/src/Standards/Generic/Tests/Commenting/FixmeUnitTest.php b/src/Standards/Generic/Tests/Commenting/FixmeUnitTest.php
@@ -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()