From 5e23744da58fd05719bb3f52ca9ba75c721c6de3 Mon Sep 17 00:00:00 2001 From: Klaus Purer Date: Sun, 6 Sep 2020 21:10:29 +0200 Subject: [PATCH 1/6] feat(ScopeKeywordSpacing): Include scope keyword spacing sniff from Squiz (#3161389 by klausi) --- coder_sniffer/Drupal/ruleset.xml | 1 + tests/Drupal/bad/BadUnitTest.php | 3 ++- tests/Drupal/bad/bad.php | 14 ++++++++++++++ tests/Drupal/bad/bad.php.fixed | 14 ++++++++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/coder_sniffer/Drupal/ruleset.xml b/coder_sniffer/Drupal/ruleset.xml index dadea2bf..bc8ec695 100644 --- a/coder_sniffer/Drupal/ruleset.xml +++ b/coder_sniffer/Drupal/ruleset.xml @@ -280,6 +280,7 @@ + diff --git a/tests/Drupal/bad/BadUnitTest.php b/tests/Drupal/bad/BadUnitTest.php index 43b5c203..473b24d4 100644 --- a/tests/Drupal/bad/BadUnitTest.php +++ b/tests/Drupal/bad/BadUnitTest.php @@ -376,7 +376,8 @@ protected function getErrorList(string $testFile): array 827 => 1, 829 => 1, 836 => 1, - 838 => 2, + 846 => 2, + 852 => 2, ]; }//end switch diff --git a/tests/Drupal/bad/bad.php b/tests/Drupal/bad/bad.php index 6e9fa345..bcdfa21d 100644 --- a/tests/Drupal/bad/bad.php +++ b/tests/Drupal/bad/bad.php @@ -835,4 +835,18 @@ function test28() { // Multiple statements on one line are not allowed. echo 'Hi!';; +/** + * A test class. + */ +class ScopeKeyword { + + /** + * Much weird spacing here. + */ + public static function test() { + + } + +} + ?> \ No newline at end of file diff --git a/tests/Drupal/bad/bad.php.fixed b/tests/Drupal/bad/bad.php.fixed index 4eeb3137..bb85c050 100644 --- a/tests/Drupal/bad/bad.php.fixed +++ b/tests/Drupal/bad/bad.php.fixed @@ -882,3 +882,17 @@ function test28() { // Multiple statements on one line are not allowed. echo 'Hi!'; ; + +/** + * A test class. + */ +class ScopeKeyword { + + /** + * Much weird spacing here. + */ + public static function test() { + + } + +} From dcdf6578190d4d497d6f1b85bde6c3fdd4371d44 Mon Sep 17 00:00:00 2001 From: Jonathan Smith Date: Sun, 20 Sep 2020 22:35:52 +0100 Subject: [PATCH 2/6] fix(Array): Use $parenthesisCloser column instead of full line length (#3169452 by jonathan1055) --- coder_sniffer/Drupal/Sniffs/Arrays/ArraySniff.php | 15 +++++---------- tests/Drupal/Arrays/ArrayUnitTest.1.inc | 4 ++++ tests/Drupal/Arrays/ArrayUnitTest.php | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/coder_sniffer/Drupal/Sniffs/Arrays/ArraySniff.php b/coder_sniffer/Drupal/Sniffs/Arrays/ArraySniff.php index 1487570c..93660b3c 100644 --- a/coder_sniffer/Drupal/Sniffs/Arrays/ArraySniff.php +++ b/coder_sniffer/Drupal/Sniffs/Arrays/ArraySniff.php @@ -113,18 +113,13 @@ public function process(File $phpcsFile, $stackPtr) if ($isInlineArray === true) { // Check if this array has more than one element and exceeds the // line length defined by $this->lineLimit. - $currentLine = $tokens[$stackPtr]['line']; - $tokenCount = $stackPtr; - while ($tokenCount < ($phpcsFile->numTokens - 1) && $tokens[($tokenCount + 1)]['line'] === $currentLine) { - $tokenCount++; - }; - $lineLength = ($tokens[$tokenCount]['column'] + $tokens[$tokenCount]['length'] - 1); - - if ($lineLength > $this->lineLimit) { + $arrayEnding = $tokens[$tokens[$stackPtr][$parenthesisCloser]]['column']; + + if ($arrayEnding > $this->lineLimit) { $comma1 = $phpcsFile->findNext(T_COMMA, ($stackPtr + 1), $tokens[$stackPtr][$parenthesisCloser]); if ($comma1 !== false) { - $error = 'The array declaration line has %s characters (the limit is %s). The array content should be split up over multiple lines'; - $phpcsFile->addError($error, $stackPtr, 'LongLineDeclaration', [$lineLength, $this->lineLimit]); + $error = 'The array declaration extends to column %s (the limit is %s). The array content should be split up over multiple lines'; + $phpcsFile->addError($error, $stackPtr, 'LongLineDeclaration', [$arrayEnding, $this->lineLimit]); } } diff --git a/tests/Drupal/Arrays/ArrayUnitTest.1.inc b/tests/Drupal/Arrays/ArrayUnitTest.1.inc index 8a09e63d..101b2737 100644 --- a/tests/Drupal/Arrays/ArrayUnitTest.1.inc +++ b/tests/Drupal/Arrays/ArrayUnitTest.1.inc @@ -18,4 +18,8 @@ $array = array( 'inline_two_elements_ok' => array('one-two-three', 'the-2nd-element-is-within-the-limit'), 'inline_two_elements_ok2' => array('one-two-three-four', 'the-2nd-element-is-right-on-the-limit'), 'inline_two_elements_not_ok' => array('one-two-three-four-five', 'the-2nd-element-extends-beyond-the-limit'), + 'inline_two_elements_ok3' => func(['one-two-three-four', 'five'], 'other text which goes past the limit'), + 'inline_two_elements_ok4' => func(['one-two-three-four', 'this-2nd-element-is-right-on-the-limit'], 'other text'), + 'inline_two_elements_ok5' => func(['one-two-three-four'], ['second_array' => 'this-is-ok'], 'other text'), + 'inline_two_elements_not_ok' => func(['one-two'], ['second_array' => 'three', 'four-five' => 'six'], 'other text'), ); diff --git a/tests/Drupal/Arrays/ArrayUnitTest.php b/tests/Drupal/Arrays/ArrayUnitTest.php index bd954561..f6e38b7b 100644 --- a/tests/Drupal/Arrays/ArrayUnitTest.php +++ b/tests/Drupal/Arrays/ArrayUnitTest.php @@ -32,9 +32,9 @@ protected function getErrorList(string $testFile): array case 'ArrayUnitTest.1.inc': return [ 14 => 1, - 15 => 1, 17 => 1, 20 => 1, + 24 => 1, ]; } From 5afcab7de3617959bafed8d9cefd86d0344fa737 Mon Sep 17 00:00:00 2001 From: Jonathan Smith Date: Mon, 28 Sep 2020 14:31:53 +0100 Subject: [PATCH 3/6] 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 4/6] 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 5/6] 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 6/6] 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++;