From 5afcab7de3617959bafed8d9cefd86d0344fa737 Mon Sep 17 00:00:00 2001 From: Jonathan Smith Date: Mon, 28 Sep 2020 14:31:53 +0100 Subject: [PATCH 1/8] after 19 commits --- .../Sniffs/Commenting/TodoCommentSniff.php | 100 ++++++ .../Commenting/TodoCommentUnitTest.inc.php | 320 ++++++++++++++++++ .../Drupal/Commenting/TodoCommentUnitTest.php | 50 +++ tests/Drupal/good/good.php | 8 +- 4 files changed, 474 insertions(+), 4 deletions(-) create mode 100644 coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php create mode 100644 tests/Drupal/Commenting/TodoCommentUnitTest.inc.php create mode 100644 tests/Drupal/Commenting/TodoCommentUnitTest.php diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php new file mode 100644 index 00000000..41193b68 --- /dev/null +++ b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php @@ -0,0 +1,100 @@ + + */ + public function register() + { + return [ + T_COMMENT, + T_DOC_COMMENT_TAG, + T_DOC_COMMENT_STRING, + ]; + + }//end register() + + + /** + * Processes this test, when one of its tokens is encountered. + * + * @param \PHP_CodeSniffer\Files\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) + { + $tokens = $phpcsFile->getTokens(); + // Standard comments and multi-line comments where the "@" is missing so + // it does not register as a T_DOC_COMMENT_TAG. + if ($tokens[$stackPtr]['code'] === T_COMMENT || $tokens[$stackPtr]['code'] === T_DOC_COMMENT_STRING) { + $comment = $tokens[$stackPtr]['content']; + $this->checkTodoFormat($phpcsFile, $stackPtr, $comment); + } else if ($tokens[$stackPtr]['code'] === T_DOC_COMMENT_TAG) { + // Document comment tag (i.e. comments that begin with "@"). + // Determine if this is related at all and build the full comment line + // from the various segments that the line is parsed into. + $expression = '/^@to/i'; + if ((bool) preg_match($expression, $tokens[$stackPtr]['content']) === true) { + $index = $stackPtr; + $comment = ''; + while ($tokens[$index]['line'] === $tokens[$stackPtr]['line']) { + $comment .= $tokens[$index]['content']; + $index++; + } + + $this->checkTodoFormat($phpcsFile, $stackPtr, $comment); + }//end if + } + + }//end process() + + + /** + * Checks a comment string for the correct syntax. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token + * in the stack passed in $tokens. + * @param string $comment The comment text. + * + * @return void + */ + private function checkTodoFormat(File $phpcsFile, $stackPtr, string $comment) + { + $expression = '/^(\/\/|\*)*\s*(?i)(?=(@*to(-|\s|)+do))(?-i)(?!@todo\s(?!-|:)\S)/m'; + if ((bool) preg_match($expression, $comment) === true) { + $comment = trim($comment, " /\r\n"); + $phpcsFile->addError("'%s' should match the format '@todo Some task'", $stackPtr, 'TodoFormat', [$comment]); + } + + }//end checkTodoFormat() + + +}//end class diff --git a/tests/Drupal/Commenting/TodoCommentUnitTest.inc.php b/tests/Drupal/Commenting/TodoCommentUnitTest.inc.php new file mode 100644 index 00000000..39f16694 --- /dev/null +++ b/tests/Drupal/Commenting/TodoCommentUnitTest.inc.php @@ -0,0 +1,320 @@ + + */ + protected function getErrorList(string $testFile): array + { + $errorList = array_fill_keys(range(19, 37), 1); + for ($i = 102; $i < 320; $i += 14) { + $errorList[$i] = 1; + } + + return $errorList; + + }//end getErrorList() + + + /** + * Returns the lines where warnings should occur. + * + * The key of the array should represent the line number and the value + * should represent the number of warnings that should occur on that line. + * + * @param string $testFile The name of the file being tested. + * + * @return array + */ + protected function getWarningList(string $testFile): array + { + return []; + + }//end getWarningList() + + +}//end class diff --git a/tests/Drupal/good/good.php b/tests/Drupal/good/good.php index 0efcf91c..2f36e561 100644 --- a/tests/Drupal/good/good.php +++ b/tests/Drupal/good/good.php @@ -1185,10 +1185,10 @@ function test6(array $names) { /** * Some short description. * - * @todo TODOs are allowed here. - * * @param string $x * Some parameter. + * + * @todo These are allowed here. */ function test7($x) { @@ -1205,8 +1205,8 @@ class ListContainsTest extends RulesIntegrationTestBase {} /** * Provides a 'Delete any path alias' action. * - * @todo: Add access callback information from Drupal 7. - * @todo: Add group information from Drupal 7. + * @todo Add access callback information from Drupal 7. + * @todo Add group information from Drupal 7. * * @Action( * id = "rules_path_alias_delete", From 9abf898155619f8d227aaf82d9e3f7f02525e7ed Mon Sep 17 00:00:00 2001 From: Jonathan Smith Date: Mon, 28 Sep 2020 14:48:26 +0100 Subject: [PATCH 2/8] Simplify test file --- .../Sniffs/Commenting/TodoCommentSniff.php | 43 ++- .../Drupal/Commenting/TodoCommentUnitTest.inc | 61 ++++ .../Commenting/TodoCommentUnitTest.inc.php | 320 ------------------ .../Drupal/Commenting/TodoCommentUnitTest.php | 6 +- 4 files changed, 104 insertions(+), 326 deletions(-) create mode 100644 tests/Drupal/Commenting/TodoCommentUnitTest.inc delete mode 100644 tests/Drupal/Commenting/TodoCommentUnitTest.inc.php diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php index 41193b68..04747fe2 100644 --- a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php @@ -11,6 +11,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Config; /** * Parses and verifies that comments use the correct @todo format. @@ -22,6 +23,15 @@ class TodoCommentSniff implements Sniff { + /** + * Show debug output for this sniff. + * + * Use phpcs --runtime-set todo_debug true + * + * @var boolean + */ + private $debug = false; + /** * Returns an array of tokens this test wants to listen for. @@ -30,6 +40,10 @@ class TodoCommentSniff implements Sniff */ public function register() { + if (defined('PHP_CODESNIFFER_IN_TESTS') === true) { + $this->debug = false; + } + return [ T_COMMENT, T_DOC_COMMENT_TAG, @@ -50,16 +64,36 @@ public function register() */ public function process(File $phpcsFile, $stackPtr) { + $debug = Config::getConfigData('todo_debug'); + if ($debug !== null) { + $this->debug = (bool) $debug; + } + $tokens = $phpcsFile->getTokens(); + if ($this->debug === true) { + echo "\n------\n\$tokens[$stackPtr] = ".print_r($tokens[$stackPtr], true).PHP_EOL; + echo 'code = '.$tokens[$stackPtr]['code'].', type = '.$tokens[$stackPtr]['type']."\n"; + } + // Standard comments and multi-line comments where the "@" is missing so // it does not register as a T_DOC_COMMENT_TAG. if ($tokens[$stackPtr]['code'] === T_COMMENT || $tokens[$stackPtr]['code'] === T_DOC_COMMENT_STRING) { $comment = $tokens[$stackPtr]['content']; + if ($this->debug === true) { + echo "\$comment = '$comment'".PHP_EOL; + } + $this->checkTodoFormat($phpcsFile, $stackPtr, $comment); } else if ($tokens[$stackPtr]['code'] === T_DOC_COMMENT_TAG) { // Document comment tag (i.e. comments that begin with "@"). // Determine if this is related at all and build the full comment line // from the various segments that the line is parsed into. + $pointer = $phpcsFile->findNext(T_DOC_COMMENT_STRING, ($stackPtr + 1)); + $comment = $tokens[$pointer]['content']; + if ($this->debug === true) { + echo "FindNext \$comment = '$comment'".PHP_EOL; + } + $expression = '/^@to/i'; if ((bool) preg_match($expression, $tokens[$stackPtr]['content']) === true) { $index = $stackPtr; @@ -67,11 +101,14 @@ public function process(File $phpcsFile, $stackPtr) while ($tokens[$index]['line'] === $tokens[$stackPtr]['line']) { $comment .= $tokens[$index]['content']; $index++; + if ($this->debug === true) { + echo "TAG \$comment = '$comment'".PHP_EOL; + } } $this->checkTodoFormat($phpcsFile, $stackPtr, $comment); }//end if - } + }//end if }//end process() @@ -90,6 +127,10 @@ private function checkTodoFormat(File $phpcsFile, $stackPtr, string $comment) { $expression = '/^(\/\/|\*)*\s*(?i)(?=(@*to(-|\s|)+do))(?-i)(?!@todo\s(?!-|:)\S)/m'; if ((bool) preg_match($expression, $comment) === true) { + if ($this->debug === true) { + echo "Failed regex for '$comment'".PHP_EOL; + } + $comment = trim($comment, " /\r\n"); $phpcsFile->addError("'%s' should match the format '@todo Some task'", $stackPtr, 'TodoFormat', [$comment]); } diff --git a/tests/Drupal/Commenting/TodoCommentUnitTest.inc b/tests/Drupal/Commenting/TodoCommentUnitTest.inc new file mode 100644 index 00000000..fd06b1b5 --- /dev/null +++ b/tests/Drupal/Commenting/TodoCommentUnitTest.inc @@ -0,0 +1,61 @@ + Date: Mon, 28 Sep 2020 21:12:25 +0100 Subject: [PATCH 3/8] Format regex as multi-line with comments. --- .../Sniffs/Commenting/TodoCommentSniff.php | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php index 04747fe2..cb9a199b 100644 --- a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php @@ -80,7 +80,7 @@ public function process(File $phpcsFile, $stackPtr) if ($tokens[$stackPtr]['code'] === T_COMMENT || $tokens[$stackPtr]['code'] === T_DOC_COMMENT_STRING) { $comment = $tokens[$stackPtr]['content']; if ($this->debug === true) { - echo "\$comment = '$comment'".PHP_EOL; + echo "Getting \$comment from \$tokens[$stackPtr]['content']\n"; } $this->checkTodoFormat($phpcsFile, $stackPtr, $comment); @@ -88,22 +88,21 @@ public function process(File $phpcsFile, $stackPtr) // Document comment tag (i.e. comments that begin with "@"). // Determine if this is related at all and build the full comment line // from the various segments that the line is parsed into. - $pointer = $phpcsFile->findNext(T_DOC_COMMENT_STRING, ($stackPtr + 1)); - $comment = $tokens[$pointer]['content']; - if ($this->debug === true) { - echo "FindNext \$comment = '$comment'".PHP_EOL; - } - + $comment = $tokens[$stackPtr]['content']; $expression = '/^@to/i'; - if ((bool) preg_match($expression, $tokens[$stackPtr]['content']) === true) { - $index = $stackPtr; - $comment = ''; + if ((bool) preg_match($expression, $comment) === true) { + if ($this->debug === true) { + echo "Attempting to build comment\n"; + } + + $index = $stackPtr + 1; while ($tokens[$index]['line'] === $tokens[$stackPtr]['line']) { $comment .= $tokens[$index]['content']; $index++; - if ($this->debug === true) { - echo "TAG \$comment = '$comment'".PHP_EOL; - } + } + + if ($this->debug === true) { + echo "Result comment = $comment\n"; } $this->checkTodoFormat($phpcsFile, $stackPtr, $comment); @@ -125,10 +124,28 @@ public function process(File $phpcsFile, $stackPtr) */ private function checkTodoFormat(File $phpcsFile, $stackPtr, string $comment) { - $expression = '/^(\/\/|\*)*\s*(?i)(?=(@*to(-|\s|)+do))(?-i)(?!@todo\s(?!-|:)\S)/m'; + if ($this->debug === true) { + echo "Checking \$comment = '$comment'\n"; + } + + $expression = '/(?x) # Set free-space mode to allow this commenting + ^(\/\/)? # At the start optionally match two forward slashes + \s* # then any amount of whitespace + (?i) # set case-insensitive mode + (?=( # start a postive non-consuming look-ahead to find all possible todos + @+to(-|\s|)+do # if one or more @ allow space or - between the to and do + | # or + to(-)*do # if no @ then only accept todo or to-do or to--do, etc + )) + (?-i) # Reset to case-sensitive + (?! # Start another non-consuming look-ahead, this time negative + @todo\s # It has to match lower-case @todo followed by one space + (?!-|:)\S # and then any non-space except - or : + )/m'; + if ((bool) preg_match($expression, $comment) === true) { if ($this->debug === true) { - echo "Failed regex for '$comment'".PHP_EOL; + echo "Failed regex - give message\n"; } $comment = trim($comment, " /\r\n"); From 4f82878faa3c1e3e4a16d61edaab5a09211c68f4 Mon Sep 17 00:00:00 2001 From: Jonathan Smith Date: Mon, 28 Sep 2020 21:18:00 +0100 Subject: [PATCH 4/8] fix coding standards in sniff --- coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php index cb9a199b..c804a87c 100644 --- a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php @@ -88,14 +88,14 @@ public function process(File $phpcsFile, $stackPtr) // Document comment tag (i.e. comments that begin with "@"). // Determine if this is related at all and build the full comment line // from the various segments that the line is parsed into. - $comment = $tokens[$stackPtr]['content']; $expression = '/^@to/i'; + $comment = $tokens[$stackPtr]['content']; if ((bool) preg_match($expression, $comment) === true) { if ($this->debug === true) { echo "Attempting to build comment\n"; } - $index = $stackPtr + 1; + $index = ($stackPtr + 1); while ($tokens[$index]['line'] === $tokens[$stackPtr]['line']) { $comment .= $tokens[$index]['content']; $index++; From 6b2dff4ac65831f83f005fe2734326f1b40cf32f Mon Sep 17 00:00:00 2001 From: Jonathan Smith Date: Tue, 29 Sep 2020 12:06:52 +0100 Subject: [PATCH 5/8] Amend comments in regex, add test for normal to do in comment, remove dup test cases --- .../Sniffs/Commenting/TodoCommentSniff.php | 7 +++---- tests/Drupal/Commenting/TodoCommentUnitTest.inc | 17 ++++++++++------- tests/Drupal/Commenting/TodoCommentUnitTest.php | 2 +- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php index c804a87c..a5d4623b 100644 --- a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php @@ -129,13 +129,12 @@ private function checkTodoFormat(File $phpcsFile, $stackPtr, string $comment) } $expression = '/(?x) # Set free-space mode to allow this commenting - ^(\/\/)? # At the start optionally match two forward slashes - \s* # then any amount of whitespace + ^(\/|\s)* # At the start optionally match any forward slashes and spaces (?i) # set case-insensitive mode (?=( # start a postive non-consuming look-ahead to find all possible todos - @+to(-|\s|)+do # if one or more @ allow space or - between the to and do + @+to(-|\s|)+do # if one or more @ allow spaces and - between the to and do | # or - to(-)*do # if no @ then only accept todo or to-do or to--do, etc + to(-)*do # if no @ then only accept todo or to-do or to--do, etc, no spaces )) (?-i) # Reset to case-sensitive (?! # Start another non-consuming look-ahead, this time negative diff --git a/tests/Drupal/Commenting/TodoCommentUnitTest.inc b/tests/Drupal/Commenting/TodoCommentUnitTest.inc index fd06b1b5..0a4cdcc5 100644 --- a/tests/Drupal/Commenting/TodoCommentUnitTest.inc +++ b/tests/Drupal/Commenting/TodoCommentUnitTest.inc @@ -15,20 +15,20 @@ * @TODo Error * @ToDO Error * @todo: Error + * @todo : Error * @to-do Error * @TO-DO Error * @To-Do Error * @TO do Error - * @to do Error - * @todo: Error - * @todo : Error + * @to do Error * @todo- Error * @todo - Error * @todoError + * @todo Error * todo Error * TODO Error * ToDo Error - * @todo Error + * to-do Error */ function foo() { @@ -38,24 +38,27 @@ function foo() { // @todo $can start with a $ // @todo \also with backslash + // This is not a todo tag. It is a general comment and we do not want + // to do the standards checking here. + // These are all incorrect. // @TODO Error // @ToDo Error // @TODo Error // @ToDO Error // @todo: Error + // @todo : Error // @to-do Error // @TO-DO Error // @To-Do Error // @TO do Error // @to do Error - // @todo: Error - // @todo : Error + // @todo Error // @todo- Error // @todo - Error // @todoError // todo Error // TODO Error // ToDo Error - // @todo Error + // to-do Error } diff --git a/tests/Drupal/Commenting/TodoCommentUnitTest.php b/tests/Drupal/Commenting/TodoCommentUnitTest.php index f315a283..85ff1d4f 100644 --- a/tests/Drupal/Commenting/TodoCommentUnitTest.php +++ b/tests/Drupal/Commenting/TodoCommentUnitTest.php @@ -20,7 +20,7 @@ class TodoCommentUnitTest extends CoderSniffUnitTest */ protected function getErrorList(string $testFile): array { - $errorList = (array_fill_keys(range(13, 31), 1) + array_fill_keys(range(42, 60), 1)); + $errorList = (array_fill_keys(range(13, 31), 1) + array_fill_keys(range(45, 63), 1)); return $errorList; }//end getErrorList() From cdcf84dec43462cdd01b055bd7b8d90bb5f409da Mon Sep 17 00:00:00 2001 From: Adam Zimmermann Date: Wed, 7 Oct 2020 15:20:32 -0500 Subject: [PATCH 6/8] Change error to warning. --- coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php | 2 +- tests/Drupal/Commenting/TodoCommentUnitTest.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php index a5d4623b..1a1238c2 100644 --- a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php @@ -148,7 +148,7 @@ private function checkTodoFormat(File $phpcsFile, $stackPtr, string $comment) } $comment = trim($comment, " /\r\n"); - $phpcsFile->addError("'%s' should match the format '@todo Some task'", $stackPtr, 'TodoFormat', [$comment]); + $phpcsFile->addWarning("'%s' should match the format '@todo Some task'", $stackPtr, 'TodoFormat', [$comment]); } }//end checkTodoFormat() diff --git a/tests/Drupal/Commenting/TodoCommentUnitTest.php b/tests/Drupal/Commenting/TodoCommentUnitTest.php index 85ff1d4f..49908b74 100644 --- a/tests/Drupal/Commenting/TodoCommentUnitTest.php +++ b/tests/Drupal/Commenting/TodoCommentUnitTest.php @@ -20,8 +20,7 @@ class TodoCommentUnitTest extends CoderSniffUnitTest */ protected function getErrorList(string $testFile): array { - $errorList = (array_fill_keys(range(13, 31), 1) + array_fill_keys(range(45, 63), 1)); - return $errorList; + return []; }//end getErrorList() @@ -38,7 +37,8 @@ protected function getErrorList(string $testFile): array */ protected function getWarningList(string $testFile): array { - return []; + $errorList = (array_fill_keys(range(13, 31), 1) + array_fill_keys(range(45, 63), 1)); + return $errorList; }//end getWarningList() From 1f56ef01c49370bc2296dcc301d6fe22ea08f8c1 Mon Sep 17 00:00:00 2001 From: Adam Zimmermann Date: Wed, 7 Oct 2020 15:22:03 -0500 Subject: [PATCH 7/8] Fix typo. --- coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php index 1a1238c2..f3d64718 100644 --- a/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php +++ b/coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php @@ -131,7 +131,7 @@ private function checkTodoFormat(File $phpcsFile, $stackPtr, string $comment) $expression = '/(?x) # Set free-space mode to allow this commenting ^(\/|\s)* # At the start optionally match any forward slashes and spaces (?i) # set case-insensitive mode - (?=( # start a postive non-consuming look-ahead to find all possible todos + (?=( # start a positive non-consuming look-ahead to find all possible todos @+to(-|\s|)+do # if one or more @ allow spaces and - between the to and do | # or to(-)*do # if no @ then only accept todo or to-do or to--do, etc, no spaces From 9db45e1891dfb15699bc56e3f495dba2f46dd171 Mon Sep 17 00:00:00 2001 From: Adam Zimmermann Date: Wed, 7 Oct 2020 15:37:24 -0500 Subject: [PATCH 8/8] Change variable name. --- tests/Drupal/Commenting/TodoCommentUnitTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Drupal/Commenting/TodoCommentUnitTest.php b/tests/Drupal/Commenting/TodoCommentUnitTest.php index 49908b74..7b1e8ff0 100644 --- a/tests/Drupal/Commenting/TodoCommentUnitTest.php +++ b/tests/Drupal/Commenting/TodoCommentUnitTest.php @@ -37,8 +37,8 @@ protected function getErrorList(string $testFile): array */ protected function getWarningList(string $testFile): array { - $errorList = (array_fill_keys(range(13, 31), 1) + array_fill_keys(range(45, 63), 1)); - return $errorList; + $warningList = (array_fill_keys(range(13, 31), 1) + array_fill_keys(range(45, 63), 1)); + return $warningList; }//end getWarningList()