From 6c313906023b840e8db048437bf7d7cd9dede621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ch=C3=A1bek?= Date: Fri, 10 Jan 2020 11:30:38 +0100 Subject: [PATCH 1/2] Try to remove whitespace when fixing single line array Also refactor whitespace a little --- .../Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php | 12 ++++++++---- .../Arrays/ArrayDeclarationUnitTest.1.inc.fixed | 4 ++-- .../Tests/Arrays/ArrayDeclarationUnitTest.2.inc | 2 +- .../Arrays/ArrayDeclarationUnitTest.2.inc.fixed | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php b/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php index 96c895023f..68c779b70f 100644 --- a/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php +++ b/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php @@ -243,7 +243,13 @@ public function processSingleLineArray($phpcsFile, $stackPtr, $arrayStart, $arra if ($fix === true) { $phpcsFile->fixer->beginChangeset(); $phpcsFile->fixer->addNewline($arrayStart); - $phpcsFile->fixer->addNewlineBefore($arrayEnd); + + if ($tokens[($arrayEnd - 1)]['code'] === T_WHITESPACE) { + $phpcsFile->fixer->replaceToken(($arrayEnd - 1), $phpcsFile->eolChar); + } else { + $phpcsFile->fixer->addNewlineBefore($arrayEnd); + } + $phpcsFile->fixer->endChangeset(); } @@ -394,9 +400,7 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array continue; }//end if - if ($tokens[$nextToken]['code'] !== T_DOUBLE_ARROW - && $tokens[$nextToken]['code'] !== T_COMMA - ) { + if ($tokens[$nextToken]['code'] !== T_DOUBLE_ARROW && $tokens[$nextToken]['code'] !== T_COMMA) { continue; } diff --git a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed index de5325c2e1..b21bff3f0b 100644 --- a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed @@ -370,12 +370,12 @@ $a = array // comment ( 'a', - 'b', + 'b', ); $a = array /* comment */ ( 'a', - 'b', + 'b', ); $x = array('a' => false); diff --git a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc index 942b9b9de1..c60bdcbca8 100644 --- a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc +++ b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc @@ -179,7 +179,7 @@ $listItems[$aliasPath] = [ 'itemContent' => implode('
', $aliases) ]; -$x = +$x = [ $x, $y, diff --git a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed index d563408d88..d01d0de093 100644 --- a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed +++ b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed @@ -186,7 +186,7 @@ $listItems[$aliasPath] = [ 'itemContent' => implode('
', $aliases), ]; -$x = +$x = [ $x, $y, From 5fae8d99e36c611b1bee90142761885283bd97b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ch=C3=A1bek?= Date: Fri, 10 Jan 2020 16:00:46 +0100 Subject: [PATCH 2/2] Fix multiline array newline detection for multiline values --- .../Sniffs/Arrays/ArrayDeclarationSniff.php | 131 +++++++++--------- .../Arrays/ArrayDeclarationUnitTest.1.inc | 18 +++ .../ArrayDeclarationUnitTest.1.inc.fixed | 26 ++++ .../Arrays/ArrayDeclarationUnitTest.2.inc | 18 +++ .../ArrayDeclarationUnitTest.2.inc.fixed | 26 ++++ .../Tests/Arrays/ArrayDeclarationUnitTest.php | 18 ++- 6 files changed, 166 insertions(+), 71 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php b/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php index 68c779b70f..de5ee8535d 100644 --- a/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php +++ b/src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php @@ -441,7 +441,6 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array && $tokens[$prev]['code'] !== T_END_NOWDOC) || $tokens[($nextToken - 1)]['line'] === $tokens[$nextToken]['line'] ) { - $content = $tokens[($nextToken - 2)]['content']; if ($tokens[($nextToken - 1)]['content'] === $phpcsFile->eolChar) { $spaceLength = 'newline'; } else { @@ -611,8 +610,7 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array $phpcsFile->recordMetric($stackPtr, 'Array end comma', 'yes'); } - $lastValueLine = false; - foreach ($indices as $value) { + foreach ($indices as $valuePosition => $value) { if (empty($value['value']) === true) { // Array was malformed and we couldn't figure out // the array value correctly, so we have to ignore it. @@ -620,20 +618,32 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array continue; } - if ($lastValueLine !== false && $tokens[$value['value']]['line'] === $lastValueLine) { + $valuePointer = $value['value']; + + $previous = $phpcsFile->findPrevious([T_WHITESPACE, T_COMMA], ($valuePointer - 1), ($arrayStart + 1), true); + if ($previous === false) { + $previous = $stackPtr; + } + + $previousIsWhitespace = $tokens[($valuePointer - 1)]['code'] === T_WHITESPACE; + if ($tokens[$previous]['line'] === $tokens[$valuePointer]['line']) { $error = 'Each value in a multi-line array must be on a new line'; - $fix = $phpcsFile->addFixableError($error, $value['value'], 'ValueNoNewline'); + if ($valuePosition === 0) { + $error = 'The first value in a multi-value array must be on a new line'; + } + + $fix = $phpcsFile->addFixableError($error, $valuePointer, 'ValueNoNewline'); if ($fix === true) { - if ($tokens[($value['value'] - 1)]['code'] === T_WHITESPACE) { - $phpcsFile->fixer->replaceToken(($value['value'] - 1), ''); + if ($previousIsWhitespace === true) { + $phpcsFile->fixer->replaceToken(($valuePointer - 1), $phpcsFile->eolChar); + } else { + $phpcsFile->fixer->addNewlineBefore($valuePointer); } - - $phpcsFile->fixer->addNewlineBefore($value['value']); } - } else if ($tokens[($value['value'] - 1)]['code'] === T_WHITESPACE) { + } else if ($previousIsWhitespace === true) { $expected = $keywordStart; - $first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $value['value'], true); + $first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $valuePointer, true); $found = ($tokens[$first]['column'] - 1); if ($found !== $expected) { $error = 'Array value not aligned correctly; expected %s spaces but found %s'; @@ -642,18 +652,16 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array $found, ]; - $fix = $phpcsFile->addFixableError($error, $value['value'], 'ValueNotAligned', $data); + $fix = $phpcsFile->addFixableError($error, $valuePointer, 'ValueNotAligned', $data); if ($fix === true) { if ($found === 0) { - $phpcsFile->fixer->addContent(($value['value'] - 1), str_repeat(' ', $expected)); + $phpcsFile->fixer->addContent(($valuePointer - 1), str_repeat(' ', $expected)); } else { - $phpcsFile->fixer->replaceToken(($value['value'] - 1), str_repeat(' ', $expected)); + $phpcsFile->fixer->replaceToken(($valuePointer - 1), str_repeat(' ', $expected)); } } } }//end if - - $lastValueLine = $tokens[$value['value']]['line']; }//end foreach }//end if @@ -684,82 +692,68 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array to be moved back one space however, then both errors would be fixed. */ - $numValues = count($indices); - - $indicesStart = ($keywordStart + 1); - $indexLine = $tokens[$stackPtr]['line']; - $lastIndexLine = null; - foreach ($indices as $index) { - if ($index['value'] === false) { + $indicesStart = ($keywordStart + 1); + foreach ($indices as $valuePosition => $index) { + $valuePointer = $index['value']; + if ($valuePointer === false) { // Syntax error or live coding. continue; } if (isset($index['index']) === false) { // Array value only. - if ($tokens[$index['value']]['line'] === $tokens[$stackPtr]['line'] && $numValues > 1) { - $error = 'The first value in a multi-value array must be on a new line'; - $fix = $phpcsFile->addFixableError($error, $stackPtr, 'FirstValueNoNewline'); - if ($fix === true) { - $phpcsFile->fixer->addNewlineBefore($index['value']); - } - } - continue; } - $lastIndexLine = $indexLine; - $indexLine = $tokens[$index['index']]['line']; + $indexPointer = $index['index']; + $indexLine = $tokens[$indexPointer]['line']; - if ($indexLine === $tokens[$stackPtr]['line']) { - $error = 'The first index in a multi-value array must be on a new line'; - $fix = $phpcsFile->addFixableError($error, $index['index'], 'FirstIndexNoNewline'); - if ($fix === true) { - $phpcsFile->fixer->addNewlineBefore($index['index']); - } - - continue; + $previous = $phpcsFile->findPrevious([T_WHITESPACE, T_COMMA], ($indexPointer - 1), ($arrayStart + 1), true); + if ($previous === false) { + $previous = $stackPtr; } - if ($indexLine === $lastIndexLine) { + if ($tokens[$previous]['line'] === $indexLine) { $error = 'Each index in a multi-line array must be on a new line'; - $fix = $phpcsFile->addFixableError($error, $index['index'], 'IndexNoNewline'); + if ($valuePosition === 0) { + $error = 'The first index in a multi-value array must be on a new line'; + } + + $fix = $phpcsFile->addFixableError($error, $indexPointer, 'IndexNoNewline'); if ($fix === true) { - if ($tokens[($index['index'] - 1)]['code'] === T_WHITESPACE) { - $phpcsFile->fixer->replaceToken(($index['index'] - 1), ''); + if ($tokens[($indexPointer - 1)]['code'] === T_WHITESPACE) { + $phpcsFile->fixer->replaceToken(($indexPointer - 1), $phpcsFile->eolChar); + } else { + $phpcsFile->fixer->addNewlineBefore($indexPointer); } - - $phpcsFile->fixer->addNewlineBefore($index['index']); } continue; } - if ($tokens[$index['index']]['column'] !== $indicesStart - && ($index['index'] - 1) !== $arrayStart - ) { + if ($tokens[$indexPointer]['column'] !== $indicesStart && ($indexPointer - 1) !== $arrayStart) { $expected = ($indicesStart - 1); - $found = ($tokens[$index['index']]['column'] - 1); + $found = ($tokens[$indexPointer]['column'] - 1); $error = 'Array key not aligned correctly; expected %s spaces but found %s'; $data = [ $expected, $found, ]; - $fix = $phpcsFile->addFixableError($error, $index['index'], 'KeyNotAligned', $data); + $fix = $phpcsFile->addFixableError($error, $indexPointer, 'KeyNotAligned', $data); if ($fix === true) { - if ($found === 0 || $tokens[($index['index'] - 1)]['code'] !== T_WHITESPACE) { - $phpcsFile->fixer->addContent(($index['index'] - 1), str_repeat(' ', $expected)); + if ($found === 0 || $tokens[($indexPointer - 1)]['code'] !== T_WHITESPACE) { + $phpcsFile->fixer->addContent(($indexPointer - 1), str_repeat(' ', $expected)); } else { - $phpcsFile->fixer->replaceToken(($index['index'] - 1), str_repeat(' ', $expected)); + $phpcsFile->fixer->replaceToken(($indexPointer - 1), str_repeat(' ', $expected)); } } } - $arrowStart = ($tokens[$index['index']]['column'] + $maxLength + 1); + $arrowStart = ($tokens[$indexPointer]['column'] + $maxLength + 1); if ($tokens[$index['arrow']]['column'] !== $arrowStart) { - $expected = ($arrowStart - ($index['index_length'] + $tokens[$index['index']]['column'])); - $found = ($tokens[$index['arrow']]['column'] - ($index['index_length'] + $tokens[$index['index']]['column'])); + $expected = ($arrowStart - ($index['index_length'] + $tokens[$indexPointer]['column'])); + $found = ($tokens[$index['arrow']]['column'] - ($index['index_length'] + $tokens[$indexPointer]['column'])); $error = 'Array double arrow not aligned correctly; expected %s space(s) but found %s'; $data = [ $expected, @@ -779,9 +773,9 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array } $valueStart = ($arrowStart + 3); - if ($tokens[$index['value']]['column'] !== $valueStart) { + if ($tokens[$valuePointer]['column'] !== $valueStart) { $expected = ($valueStart - ($tokens[$index['arrow']]['length'] + $tokens[$index['arrow']]['column'])); - $found = ($tokens[$index['value']]['column'] - ($tokens[$index['arrow']]['length'] + $tokens[$index['arrow']]['column'])); + $found = ($tokens[$valuePointer]['column'] - ($tokens[$index['arrow']]['length'] + $tokens[$index['arrow']]['column'])); if ($found < 0) { $found = 'newline'; } @@ -795,25 +789,24 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array $fix = $phpcsFile->addFixableError($error, $index['arrow'], 'ValueNotAligned', $data); if ($fix === true) { if ($found === 'newline') { - $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($index['value'] - 1), null, true); + $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($valuePointer - 1), null, true); $phpcsFile->fixer->beginChangeset(); - for ($i = ($prev + 1); $i < $index['value']; $i++) { + for ($i = ($prev + 1); $i < $valuePointer; $i++) { $phpcsFile->fixer->replaceToken($i, ''); } - $phpcsFile->fixer->replaceToken(($index['value'] - 1), str_repeat(' ', $expected)); + $phpcsFile->fixer->replaceToken(($valuePointer - 1), str_repeat(' ', $expected)); $phpcsFile->fixer->endChangeset(); } else if ($found === 0) { - $phpcsFile->fixer->addContent(($index['value'] - 1), str_repeat(' ', $expected)); + $phpcsFile->fixer->addContent(($valuePointer - 1), str_repeat(' ', $expected)); } else { - $phpcsFile->fixer->replaceToken(($index['value'] - 1), str_repeat(' ', $expected)); + $phpcsFile->fixer->replaceToken(($valuePointer - 1), str_repeat(' ', $expected)); } } }//end if // Check each line ends in a comma. - $valueStart = $index['value']; - $valueLine = $tokens[$index['value']]['line']; + $valueStart = $valuePointer; $nextComma = false; $end = $phpcsFile->findEndOfStatement($valueStart); @@ -837,11 +830,11 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array if ($nextComma === false || ($tokens[$nextComma]['line'] !== $valueLine)) { $error = 'Each line in an array declaration must end in a comma'; - $fix = $phpcsFile->addFixableError($error, $index['value'], 'NoComma'); + $fix = $phpcsFile->addFixableError($error, $valuePointer, 'NoComma'); if ($fix === true) { // Find the end of the line and put a comma there. - for ($i = ($index['value'] + 1); $i <= $arrayEnd; $i++) { + for ($i = ($valuePointer + 1); $i <= $arrayEnd; $i++) { if ($tokens[$i]['line'] > $valueLine) { break; } diff --git a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc index 8671c5da16..e6fdbdc1f8 100644 --- a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc +++ b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc @@ -394,6 +394,18 @@ HERE , ); +array( + lorem( + 1 + ), 2, +); + +array( + 1 => lorem( + 1 + ), 2 => 2, +); + $foo = array( 'тип' => 'авто', 'цвет' => 'синий', @@ -429,6 +441,12 @@ $foo = array( $foo->fn => 'value', ); +array($a, $b, +$c); + +array('a' => $a, 'b' => $b, +'c' => $c); + // Intentional syntax error. $a = array( 'a' => diff --git a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed index b21bff3f0b..336ab0af9f 100644 --- a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.1.inc.fixed @@ -422,6 +422,20 @@ HERE , ); +array( + lorem( + 1 + ), + 2, +); + +array( + 1 => lorem( + 1 + ), + 2 => 2, +); + $foo = array( 'тип' => 'авто', 'цвет' => 'синий', @@ -457,6 +471,18 @@ $foo = array( $foo->fn => 'value', ); +array( + $a, + $b, + $c, +); + +array( + 'a' => $a, + 'b' => $b, + 'c' => $c, +); + // Intentional syntax error. $a = array( 'a' => diff --git a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc index c60bdcbca8..60ff35d64c 100644 --- a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc +++ b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc @@ -383,6 +383,18 @@ HERE , ]; +[ + lorem( + 1 + ), 2, +]; + +[ + 1 => lorem( + 1 + ), 2 => 2, +]; + $foo = [ 'тип' => 'авто', 'цвет' => 'синий', @@ -418,6 +430,12 @@ $foo = [ $foo->fn => 'value', ]; +[$a, $b, +$c]; + +['a' => $a, 'b' => $b, +'c' => $c]; + // Intentional syntax error. $a = [ 'a' => diff --git a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed index d01d0de093..1d17fa0c4d 100644 --- a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed +++ b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed @@ -409,6 +409,20 @@ HERE , ]; +[ + lorem( + 1 + ), + 2, +]; + +[ + 1 => lorem( + 1 + ), + 2 => 2, +]; + $foo = [ 'тип' => 'авто', 'цвет' => 'синий', @@ -444,6 +458,18 @@ $foo = [ $foo->fn => 'value', ]; +[ + $a, + $b, + $c, +]; + +[ + 'a' => $a, + 'b' => $b, + 'c' => $c, +]; + // Intentional syntax error. $a = [ 'a' => diff --git a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.php b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.php index 9c0bf7bd50..818b1b5562 100644 --- a/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.php +++ b/src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.php @@ -79,6 +79,7 @@ public function getErrorList($testFile='') 148 => 1, 151 => 1, 157 => 1, + 173 => 1, 174 => 3, 179 => 1, 182 => 1, @@ -113,7 +114,13 @@ public function getErrorList($testFile='') 370 => 1, 383 => 1, 394 => 1, - 429 => 1, + 400 => 1, + 406 => 1, + 441 => 1, + 444 => 2, + 445 => 2, + 447 => 2, + 448 => 3, ]; case 'ArrayDeclarationUnitTest.2.inc': return [ @@ -160,6 +167,7 @@ public function getErrorList($testFile='') 148 => 1, 151 => 1, 157 => 1, + 173 => 1, 174 => 3, 179 => 1, 190 => 1, @@ -189,7 +197,13 @@ public function getErrorList($testFile='') 358 => 1, 372 => 1, 383 => 1, - 418 => 1, + 389 => 1, + 395 => 1, + 430 => 1, + 433 => 2, + 434 => 2, + 436 => 2, + 437 => 3, ]; default: return [];