From a513f65c33cab047808c9068160f8ffa502ff1b0 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Wed, 1 May 2019 14:25:08 +0200 Subject: [PATCH 1/2] :sparkles: Better checks for array rule --- .../Sniffs/Arrays/ArrayDeclarationSniff.php | 238 +++++------------- .../Tests/Arrays/ArrayDeclarationUnitTest.inc | 36 ++- .../Arrays/ArrayDeclarationUnitTest.inc.fixed | 32 +++ .../Tests/Arrays/ArrayDeclarationUnitTest.php | 57 +++-- 4 files changed, 166 insertions(+), 197 deletions(-) diff --git a/SymfonyCustom/Sniffs/Arrays/ArrayDeclarationSniff.php b/SymfonyCustom/Sniffs/Arrays/ArrayDeclarationSniff.php index 61309e6..46120e4 100644 --- a/SymfonyCustom/Sniffs/Arrays/ArrayDeclarationSniff.php +++ b/SymfonyCustom/Sniffs/Arrays/ArrayDeclarationSniff.php @@ -325,53 +325,35 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ // Find all the double arrows that reside in this scope. for ($nextToken = ($stackPtr + 1); $nextToken < $arrayEnd; $nextToken++) { - // Skip bracketed statements, like function calls. - if (T_OPEN_PARENTHESIS === $tokens[$nextToken]['code'] - && (false === isset($tokens[$nextToken]['parenthesis_owner']) - || $tokens[$nextToken]['parenthesis_owner'] !== $stackPtr) - ) { - $nextToken = $tokens[$nextToken]['parenthesis_closer']; - continue; - } - - if (T_ARRAY === $tokens[$nextToken]['code'] - || T_OPEN_SHORT_ARRAY === $tokens[$nextToken]['code'] - || T_CLOSURE === $tokens[$nextToken]['code'] - ) { - // Let subsequent calls of this test handle nested arrays. - if (T_DOUBLE_ARROW !== $tokens[$lastToken]['code']) { - $indices[] = ['value' => $nextToken]; - $lastToken = $nextToken; - } - - if (T_ARRAY === $tokens[$nextToken]['code']) { + // Skip array or function calls + switch ($tokens[$nextToken]['code']) { + case T_ARRAY: $nextToken = $tokens[$tokens[$nextToken]['parenthesis_opener']]['parenthesis_closer']; - } elseif (T_OPEN_SHORT_ARRAY === $tokens[$nextToken]['code']) { + continue; + case T_OPEN_SHORT_ARRAY: $nextToken = $tokens[$nextToken]['bracket_closer']; - } else { - // T_CLOSURE. + continue; + case T_CLOSURE: $nextToken = $tokens[$nextToken]['scope_closer']; - } - - $nextToken = $phpcsFile->findNext(T_WHITESPACE, ($nextToken + 1), null, true); - if (T_COMMA !== $tokens[$nextToken]['code']) { - $nextToken--; - } else { - $lastToken = $nextToken; - } - - continue; + continue; + case T_OPEN_PARENTHESIS: + $parenthesisOwner = $tokens[$nextToken]['parenthesis_owner']; + if (false === isset($parenthesisOwner) || $parenthesisOwner !== $stackPtr) { + $nextToken = $tokens[$nextToken]['parenthesis_closer']; + continue; + } + break; } - if (T_DOUBLE_ARROW !== $tokens[$nextToken]['code'] - && T_COMMA !== $tokens[$nextToken]['code'] + if (!in_array($tokens[$nextToken]['code'], [T_DOUBLE_ARROW, T_COMMA]) + && $nextToken !== $arrayEnd - 1 ) { continue; } $currentEntry = []; - if (T_COMMA === $tokens[$nextToken]['code']) { + if (T_COMMA === $tokens[$nextToken]['code'] || $nextToken === $arrayEnd - 1) { $stackPtrCount = 0; if (true === isset($tokens[$stackPtr]['nested_parenthesis'])) { $stackPtrCount = count($tokens[$stackPtr]['nested_parenthesis']); @@ -393,43 +375,41 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ continue; } - if (true === $keyUsed && T_COMMA === $tokens[$lastToken]['code']) { - $error = 'No key specified for array entry; first entry specifies key'; - $phpcsFile->addError($error, $nextToken, 'NoKeySpecified'); - - return; - } + $valueContent = $phpcsFile->findNext( + Tokens::$emptyTokens, + ($lastToken + 1), + $nextToken, + true + ); - if (false === $keyUsed) { - if (T_WHITESPACE === $tokens[($nextToken - 1)]['code']) { - $content = $tokens[($nextToken - 2)]['content']; - if ($tokens[($nextToken - 1)]['content'] === $phpcsFile->eolChar) { - $spaceLength = 'newline'; - } else { - $spaceLength = $tokens[($nextToken - 1)]['length']; - } + if (false !== $valueContent && T_DOUBLE_ARROW !== $tokens[$lastToken]['code']) { + if (true === $keyUsed) { + $error = 'No key specified for array entry; first entry specifies key'; + $phpcsFile->addError($error, $nextToken, 'NoKeySpecified'); + } else { + $singleUsed = true; + } - $error = 'Expected 0 spaces between "%s" and comma; %s found'; - $data = [ - $content, - $spaceLength, - ]; + $indices[] = ['value' => $valueContent]; + } - $fix = $phpcsFile->addFixableError($error, $nextToken, 'SpaceBeforeComma', $data); - if (true === $fix) { - $phpcsFile->fixer->replaceToken(($nextToken - 1), ''); - } + if (T_COMMA === $tokens[$nextToken]['code'] + && T_WHITESPACE === $tokens[($nextToken - 1)]['code'] + ) { + $content = $tokens[($nextToken - 2)]['content']; + if ($tokens[($nextToken - 1)]['content'] === $phpcsFile->eolChar) { + $spaceLength = 'newline'; + } else { + $spaceLength = $tokens[($nextToken - 1)]['length']; } - $valueContent = $phpcsFile->findNext( - Tokens::$emptyTokens, - ($lastToken + 1), - $nextToken, - true - ); + $error = 'Expected 0 spaces between "%s" and comma; %s found'; + $data = [$content, $spaceLength]; - $indices[] = ['value' => $valueContent]; - $singleUsed = true; + $fix = $phpcsFile->addFixableError($error, $nextToken, 'SpaceBeforeComma', $data); + if (true === $fix) { + $phpcsFile->fixer->replaceToken(($nextToken - 1), ''); + } } $lastToken = $nextToken; @@ -440,12 +420,11 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ if (true === $singleUsed) { $error = 'Key specified for array entry; first entry has no key'; $phpcsFile->addError($error, $nextToken, 'KeySpecified'); - - return; + } else { + $keyUsed = true; } $currentEntry['arrow'] = $nextToken; - $keyUsed = true; // Find the start of index that uses this double arrow. $indexEnd = $phpcsFile->findPrevious(T_WHITESPACE, ($nextToken - 1), $arrayStart, true); @@ -481,18 +460,7 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ } } - /* - This section checks for arrays that don't specify keys. - - Arrays such as: - array( - 'aaa', - 'bbb', - 'd', - ); - */ - - if (false === $keyUsed && false === empty($indices)) { + if (false === empty($indices)) { $count = count($indices); $lastIndex = $indices[($count - 1)]['value']; @@ -514,15 +482,22 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ $phpcsFile->recordMetric($stackPtr, 'Array end comma', 'yes'); } - $lastValueLine = false; + $lastValueLine = $stackPtr; foreach ($indices as $value) { + if (false === empty($value['arrow'])) { + // Array value with arrow are checked later cause there is more checks. + continue; + } + if (true === empty($value['value'])) { - // Array was malformed and we couldn't figure out - // the array value correctly, so we have to ignore it. + // Array was malformed, so we have to ignore it. // Other parts of this sniff will correct the error. continue; } + $lastValue = $phpcsFile->findPrevious(T_COMMA, $value['value'] - 1, $lastValueLine); + $lastValueLine = $lastValue ? $tokens[$lastValue]['line'] : false; + if (false !== $lastValueLine && $tokens[$value['value']]['line'] === $lastValueLine) { $error = 'Each value in a multi-line array must be on a new line'; $fix = $phpcsFile->addFixableError($error, $value['value'], 'ValueNoNewline'); @@ -552,8 +527,6 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ } } } - - $lastValueLine = $tokens[$value['value']]['line']; } } @@ -583,14 +556,12 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ will also cause an error in the value's alignment. If the arrow were to be moved back one space however, then both errors would be fixed. */ - $numValues = count($indices); $indicesStart = ($currentIndent + $this->indent + 1); $arrowStart = ($indicesStart + $maxLength + 1); $valueStart = ($arrowStart + 3); - $indexLine = $tokens[$stackPtr]['line']; - $lastIndexLine = null; + $lastIndexLine = $tokens[$stackPtr]['line']; foreach ($indices as $index) { if (isset($index['index']) === false) { // Array value only. @@ -605,7 +576,8 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ continue; } - $lastIndexLine = $indexLine; + $lastIndex = $phpcsFile->findPrevious(T_COMMA, $index['index'] - 1, $lastIndexLine); + $lastIndexLine = $lastIndex ? $tokens[$lastIndex]['line'] : false; $indexLine = $tokens[$index['index']]['line']; if ($indexLine === $tokens[$stackPtr]['line']) { @@ -652,7 +624,8 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ } if ($tokens[$index['arrow']]['column'] !== $arrowStart) { - $expected = ($arrowStart - (mb_strlen($index['index_content']) + $tokens[$index['index']]['column'])); + $expected = ($arrowStart + - (mb_strlen($index['index_content']) + $tokens[$index['index']]['column'])); $found = $tokens[$index['arrow']]['column'] - (mb_strlen($index['index_content']) + $tokens[$index['index']]['column']); @@ -660,8 +633,8 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ $found = 'newline'; } - $error = 'Array double arrow not aligned correctly; expected %s space(s) but found %s'; - $data = [$expected, $found]; + $error = 'Array double arrow not aligned correctly; expected %s space(s) but found %s'; + $data = [$expected, $found]; if ('newline' !== $found || false === $this->ignoreNewLines) { $fix = $phpcsFile->addFixableError($error, $index['arrow'], 'DoubleArrowNotAligned', $data); @@ -697,10 +670,7 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ if ('newline' !== $found || false === $this->ignoreNewLines) { $error = 'Array value not aligned correctly; expected %s space(s) but found %s'; - $data = [ - $expected, - $found, - ]; + $data = [$expected, $found]; $fix = $phpcsFile->addFixableError($error, $index['arrow'], 'ValueNotAligned', $data); if (true === $fix) { @@ -721,78 +691,6 @@ public function processMultiLineArray(File $phpcsFile, $stackPtr, $arrayStart, $ } } } - - // Check each line ends in a comma. - $valueLine = $tokens[$index['value']]['line']; - $nextComma = false; - for ($i = $index['value']; $i < $arrayEnd; $i++) { - // Skip bracketed statements, like function calls. - if (T_OPEN_PARENTHESIS === $tokens[$i]['code']) { - $i = $tokens[$i]['parenthesis_closer']; - $valueLine = $tokens[$i]['line']; - continue; - } - - if (T_ARRAY === $tokens[$i]['code']) { - $i = $tokens[$tokens[$i]['parenthesis_opener']]['parenthesis_closer']; - $valueLine = $tokens[$i]['line']; - continue; - } - - // Skip to the end of multi-line strings. - if (isset(Tokens::$stringTokens[$tokens[$i]['code']]) === true) { - $i = $phpcsFile->findNext($tokens[$i]['code'], ($i + 1), null, true); - $i--; - $valueLine = $tokens[$i]['line']; - continue; - } - - if (T_OPEN_SHORT_ARRAY === $tokens[$i]['code']) { - $i = $tokens[$i]['bracket_closer']; - $valueLine = $tokens[$i]['line']; - continue; - } - - if (T_CLOSURE === $tokens[$i]['code']) { - $i = $tokens[$i]['scope_closer']; - $valueLine = $tokens[$i]['line']; - continue; - } - - if (T_COMMA === $tokens[$i]['code']) { - $nextComma = $i; - break; - } - } - - if (false === $nextComma || ($tokens[$nextComma]['line'] !== $valueLine)) { - $error = 'Each line in an array declaration must end in a comma'; - $fix = $phpcsFile->addFixableError($error, $index['value'], 'NoComma'); - - if (true === $fix) { - // Find the end of the line and put a comma there. - for ($i = ($index['value'] + 1); $i < $arrayEnd; $i++) { - if ($tokens[$i]['line'] > $valueLine) { - break; - } - } - - $phpcsFile->fixer->addContentBefore(($i - 1), ','); - } - } - - // Check that there is no space before the comma. - if (false !== $nextComma && T_WHITESPACE === $tokens[($nextComma - 1)]['code']) { - $content = $tokens[($nextComma - 2)]['content']; - $spaceLength = $tokens[($nextComma - 1)]['length']; - $error = 'Expected 0 spaces between "%s" and comma; %s found'; - $data = [$content, $spaceLength]; - - $fix = $phpcsFile->addFixableError($error, $nextComma, 'SpaceBeforeComma', $data); - if (true === $fix) { - $phpcsFile->fixer->replaceToken(($nextComma - 1), ''); - } - } } } } diff --git a/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.inc b/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.inc index ac0f03a..64cc16e 100644 --- a/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.inc +++ b/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.inc @@ -70,9 +70,41 @@ class TestClass 'short_name' => 'big' ); + $array1 = [ + [ + '', + ] + , + [ + '', + ], + ]; + + $array2 = array( + [ + '', + ],[ + '', + ], + ); + + $array3 = array( + 'a' => [''] + ,[''], + 'b' => [''] + ,[''], + ); + + $array4 = array( + [''] + ,'a' => [''], + [''] + ,'b' => [''] + ); + $utf8 = array( - '/[áàâãªäå]/u' => 'a', - '/[ÁÀÂÃÄÅ]/u' => 'A', + '/[áàâãªäå]/u' => 'a' + ,'/[ÁÀÂÃÄÅ]/u' => 'A', '/[ÍÌÎÏ]/u' => 'I', '/[íìîï]/u' => 'i', '/[éèêë]/u' => 'e', diff --git a/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.inc.fixed b/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.inc.fixed index d68bb46..daa6fb9 100644 --- a/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.inc.fixed +++ b/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.inc.fixed @@ -70,6 +70,38 @@ class TestClass 'short_name' => 'big', ); + $array1 = [ + [ + '', + ], + [ + '', + ], + ]; + + $array2 = array( + [ + '', + ], + [ + '', + ], + ); + + $array3 = array( + 'a' => [''], + [''], + 'b' => [''], + [''], + ); + + $array4 = array( + [''], + 'a' => [''], + [''], + 'b' => [''], + ); + $utf8 = array( '/[áàâãªäå]/u' => 'a', '/[ÁÀÂÃÄÅ]/u' => 'A', diff --git a/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.php b/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.php index 96d088a..6982d08 100644 --- a/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.php +++ b/SymfonyCustom/Tests/Arrays/ArrayDeclarationUnitTest.php @@ -22,31 +22,38 @@ class ArrayDeclarationUnitTest extends AbstractSniffUnitTest public function getErrorList() { return [ - 7 => 2, - 9 => 1, - 22 => 1, - 23 => 1, - 24 => 1, - 25 => 1, - 31 => 1, - 35 => 1, - 36 => 1, - 41 => 1, - 46 => 1, - 47 => 1, - 50 => 1, - 51 => 1, - 53 => 1, - 58 => 1, - 61 => 1, - 62 => 1, - 63 => 1, - 64 => 1, - 65 => 1, - 66 => 2, - 67 => 1, - 68 => 1, - 70 => 1, + 7 => 2, + 9 => 1, + 22 => 1, + 23 => 1, + 24 => 2, + 25 => 1, + 31 => 1, + 35 => 1, + 36 => 1, + 41 => 1, + 46 => 1, + 47 => 1, + 50 => 1, + 51 => 1, + 53 => 1, + 58 => 1, + 61 => 1, + 62 => 1, + 63 => 1, + 64 => 1, + 65 => 1, + 66 => 2, + 67 => 1, + 68 => 1, + 70 => 1, + 77 => 1, + 86 => 1, + 93 => 3, + 95 => 3, + 100 => 3, + 102 => 4, + 107 => 2, ]; } From 23e542f39ae347ffeee27b7fcbe94b1db4097d4f Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Wed, 1 May 2019 14:29:41 +0200 Subject: [PATCH 2/2] :lipstick: Lint --- .../Sniffs/Commenting/DocCommentForbiddenTagsSniff.php | 2 +- .../Sniffs/Commenting/DocCommentGroupSameTypeSniff.php | 2 +- SymfonyCustom/Sniffs/Formatting/BlankLineBeforeReturnSniff.php | 2 +- SymfonyCustom/Sniffs/WhiteSpace/DocCommentTagSpacingSniff.php | 2 +- SymfonyCustom/Sniffs/WhiteSpace/EmptyLinesSniff.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/SymfonyCustom/Sniffs/Commenting/DocCommentForbiddenTagsSniff.php b/SymfonyCustom/Sniffs/Commenting/DocCommentForbiddenTagsSniff.php index b58664d..88e0b13 100644 --- a/SymfonyCustom/Sniffs/Commenting/DocCommentForbiddenTagsSniff.php +++ b/SymfonyCustom/Sniffs/Commenting/DocCommentForbiddenTagsSniff.php @@ -28,7 +28,7 @@ class DocCommentForbiddenTagsSniff implements Sniff public function register() { return [ - T_DOC_COMMENT_TAG + T_DOC_COMMENT_TAG, ]; } diff --git a/SymfonyCustom/Sniffs/Commenting/DocCommentGroupSameTypeSniff.php b/SymfonyCustom/Sniffs/Commenting/DocCommentGroupSameTypeSniff.php index ce32cfa..1120b0a 100644 --- a/SymfonyCustom/Sniffs/Commenting/DocCommentGroupSameTypeSniff.php +++ b/SymfonyCustom/Sniffs/Commenting/DocCommentGroupSameTypeSniff.php @@ -56,7 +56,7 @@ class DocCommentGroupSameTypeSniff implements Sniff public function register() { return [ - T_DOC_COMMENT_OPEN_TAG + T_DOC_COMMENT_OPEN_TAG, ]; } diff --git a/SymfonyCustom/Sniffs/Formatting/BlankLineBeforeReturnSniff.php b/SymfonyCustom/Sniffs/Formatting/BlankLineBeforeReturnSniff.php index 15b2257..faf7e74 100644 --- a/SymfonyCustom/Sniffs/Formatting/BlankLineBeforeReturnSniff.php +++ b/SymfonyCustom/Sniffs/Formatting/BlankLineBeforeReturnSniff.php @@ -21,7 +21,7 @@ class BlankLineBeforeReturnSniff implements Sniff public function register() { return [ - T_RETURN + T_RETURN, ]; } diff --git a/SymfonyCustom/Sniffs/WhiteSpace/DocCommentTagSpacingSniff.php b/SymfonyCustom/Sniffs/WhiteSpace/DocCommentTagSpacingSniff.php index dc4655e..a954c9c 100644 --- a/SymfonyCustom/Sniffs/WhiteSpace/DocCommentTagSpacingSniff.php +++ b/SymfonyCustom/Sniffs/WhiteSpace/DocCommentTagSpacingSniff.php @@ -56,7 +56,7 @@ class DocCommentTagSpacingSniff implements Sniff public function register() { return [ - T_DOC_COMMENT_TAG + T_DOC_COMMENT_TAG, ]; } diff --git a/SymfonyCustom/Sniffs/WhiteSpace/EmptyLinesSniff.php b/SymfonyCustom/Sniffs/WhiteSpace/EmptyLinesSniff.php index 256a7cc..7de600b 100644 --- a/SymfonyCustom/Sniffs/WhiteSpace/EmptyLinesSniff.php +++ b/SymfonyCustom/Sniffs/WhiteSpace/EmptyLinesSniff.php @@ -18,7 +18,7 @@ class EmptyLinesSniff implements Sniff public function register() { return [ - T_WHITESPACE + T_WHITESPACE, ]; }