From 29b7411d37562708e6364c62dbf4fd8cc6e51f33 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 9 May 2024 11:23:19 -0300 Subject: [PATCH 1/3] Generic/ArrayIndent: remove T_COMMA from list of tokens to ignore This commit removes T_COMMA from a list of tokens to ignore when searching for the non-empty token before the start of the array. T_COMMA was added to this list in 1697fac to fix a bug. See https://github.com/squizlabs/PHP_CodeSniffer/issues/3100 for more details. Back when this fix was introduced, it was necessary to ignore commas due to a bug in findStartOfStatement() that would cause this method to return the wrong token when passed the last token in a statement. This bug has been fixed afterwards in ef80e53 and ignoring commas is not necessary anymore. Now, the call to findStartOfStatement() in https://github.com/squizlabs/PHP_CodeSniffer/blob/4cf6badaf0c177acaf295e2178f4383e2ea71b20/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php#L67 correctly finds the start of the statement when passed the comma that marks the end of the statement. A test was added 1697fac. It would fail if running an older version of PHPCS without ignoring commas, but now it passes: https://github.com/squizlabs/PHP_CodeSniffer/pull/3101/files#diff-f786a9f6c41b82204dc89de2c02437c0e44401d580b02fcbde6278ea03f25693R83-R90 --- src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php b/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php index 87da8f3cf9..94490d5563 100644 --- a/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php +++ b/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php @@ -62,7 +62,6 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array // Determine how far indented the entire array declaration should be. $ignore = Tokens::$emptyTokens; $ignore[] = T_DOUBLE_ARROW; - $ignore[] = T_COMMA; $prev = $phpcsFile->findPrevious($ignore, ($stackPtr - 1), null, true); $start = $phpcsFile->findStartOfStatement($prev); $first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $start, true); From d74b8469854f7ce4dcccc6f102c9083ba5c0c833 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 8 May 2024 11:35:29 -0300 Subject: [PATCH 2/3] Generic/ArrayIndent: improve code coverage --- src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc | 3 +++ .../Generic/Tests/Arrays/ArrayIndentUnitTest.inc.fixed | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc b/src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc index 6f418ca1f3..06ebca78f8 100644 --- a/src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc +++ b/src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc @@ -149,3 +149,6 @@ $var = [ ]; // phpcs:set Generic.Arrays.ArrayIndent indent 4 + +$array = [1, +]; diff --git a/src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc.fixed b/src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc.fixed index 1ea8dd1e6f..03f508dbe5 100644 --- a/src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc.fixed +++ b/src/Standards/Generic/Tests/Arrays/ArrayIndentUnitTest.inc.fixed @@ -150,3 +150,6 @@ $var = [ ]; // phpcs:set Generic.Arrays.ArrayIndent indent 4 + +$array = [1, +]; From 3a2230cfd1cb6077c014205391a9cf9c3ef72991 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 9 May 2024 16:41:25 -0300 Subject: [PATCH 3/3] Generic/ArrayIndent: avoid extra calls to ArrayIndent::processMultiLineArray() This commit fixes a small inefficiency when fixing the `CloseBraceNotNewLine` error. Previously, the sniff would add the newline and the number of spaces used for an array element (which is more than what should be used for the array closing brace). Then on a subsequent call to processMultiLineArray(), it would detect the wrong number of spaces for the closing brace and it would fix it. Now, the sniff will use the correct number of spaces when adding the newline. --- .../Generic/Sniffs/Arrays/ArrayIndentSniff.php | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php b/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php index 94490d5563..b6e3d37d70 100644 --- a/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php +++ b/src/Standards/Generic/Sniffs/Arrays/ArrayIndentSniff.php @@ -151,7 +151,7 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array $error = 'Closing brace of array declaration must be on a new line'; $fix = $phpcsFile->addFixableError($error, $arrayEnd, 'CloseBraceNotNewLine'); if ($fix === true) { - $padding = $phpcsFile->eolChar.str_repeat(' ', $expectedIndent); + $padding = $phpcsFile->eolChar.str_repeat(' ', $startIndent); $phpcsFile->fixer->addContentBefore($arrayEnd, $padding); } @@ -159,20 +159,19 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array } // The close brace must be indented one stop less. - $expectedIndent -= $this->indent; - $foundIndent = ($tokens[$arrayEnd]['column'] - 1); - if ($foundIndent === $expectedIndent) { + $foundIndent = ($tokens[$arrayEnd]['column'] - 1); + if ($foundIndent === $startIndent) { return; } $pluralizeSpace = 's'; - if ($expectedIndent === 1) { + if ($startIndent === 1) { $pluralizeSpace = ''; } $error = 'Array close brace not indented correctly; expected %s space%s but found %s'; $data = [ - $expectedIndent, + $startIndent, $pluralizeSpace, $foundIndent, ]; @@ -181,7 +180,7 @@ public function processMultiLineArray($phpcsFile, $stackPtr, $arrayStart, $array return; } - $padding = str_repeat(' ', $expectedIndent); + $padding = str_repeat(' ', $startIndent); if ($foundIndent === 0) { $phpcsFile->fixer->addContentBefore($arrayEnd, $padding); } else {