codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.38k stars 1.9k forks source link

Discussion: Coding Standard #4382

Closed kenjis closed 3 years ago

kenjis commented 3 years ago

CodeIgniter4 code does not follow the coding standard.

When I checked CI4 code under system, phpcs reported 6,336 errors .

$ vendor/bin/phpcs --standard=./vendor/codeigniter4/codeigniter4-standard/CodeIgniter4 --encoding=utf-8 system/ | grep ' | ERROR ' | wc -l
    6336

This means that the coding standard does not match the current situation.

Since we have the coding standard, I think we should remove unnecessary rules and bring errors and warnings to zero.

What are your thoughts?

paulbalandan commented 3 years ago

I have proposed a few PR before changing our coding standard from codesniffer to cs-fixer, because our current coding standard is not updated and pretty unmaintained now. I have started before a big overhaul but resulted in several diffs so I opted for small batches.

paulbalandan commented 3 years ago

Now that cs fixer supports php8, I'll try to revive my old PR to replace our cs.

paulbalandan commented 3 years ago

I have talked with Louis before on slack and he suggested opening a new repo instead of replacing the current repo.

kenjis commented 3 years ago

Do you mean we will stop maintaining the current coding standard and move to php-cs-fixer? What is the reason?

WinterSilence commented 3 years ago

@paulbalandan @kenjis you can merge configs by symplify/easy-coding-standard

kenjis commented 3 years ago

@WinterSilence Thanks! Since we have already the coding standard, if there is something wrong with it, it is better not throw it all away. easy-coding-standard might be a better solution?

WinterSilence commented 3 years ago

@kenjis ESC is aggregator for CsFixer and CodeSniffer rules + additional rules for them. package contains validator and normalizer. It's easy solution for use that's packages together. You can, step by step, replace CodeSniffer rules to CsFixer aliases.

sfadschm commented 3 years ago

When I checked CI4 code under system, phpcs reported 6,336 errors.

I know CI4 does not follow the standard everywhere, but just for my understanding: Did you exclude the ThirdParty folder when running the scan?

kenjis commented 3 years ago

It seems there is no error in system/ThirdParty/.

$ vendor/bin/phpcs --standard=./vendor/codeigniter4/codeigniter4-standard/CodeIgniter4 --encoding=utf-8 system/ThirdParty/
$

I ran composer update, and now 4,699 errors.

$ vendor/bin/phpcs --standard=./vendor/codeigniter4/codeigniter4-standard/CodeIgniter4 --encoding=utf-8 system/  | grep ' | ERROR ' | wc -l
    4699
WinterSilence commented 3 years ago

@kenjis https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-files-and-folders (:

kenjis commented 3 years ago

I looked into the issue https://github.com/codeigniter4/coding-standard/issues/41.

But I can't fix it, because I can't understand the code of CodeIgniter4\Sniffs\Arrays\ArrayDeclarationSniff: https://github.com/codeigniter4/coding-standard/issues/41#issuecomment-802434148

WinterSilence commented 3 years ago

@kenjis it's old version of PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff, without bug fixes from original sniff

kenjis commented 3 years ago
--- ArrayDeclarationSniff.php.orig  2021-03-20 09:41:26.000000000 +0900
+++ CodeIgniter4/Sniffs/Arrays/ArrayDeclarationSniff.php    2021-03-19 08:38:11.000000000 +0900
@@ -1,21 +1,47 @@
 <?php
 /**
- * Ensures that arrays conform to the array coding standard.
+ * Array Declaration
  *
- * @author    Greg Sherwood <gsherwood@squiz.net>
- * @copyright 2006-2015 Squiz Pty Ltd (ABN 77 084 670 600)
- * @license   https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
+ * @package   CodeIgniter4-Standard
+ * @author    Louis Linehan <louis.linehan@gmail.com>
+ * @copyright 2017 British Columbia Institute of Technology
+ * @license   https://github.com/bcit-ci/CodeIgniter4-Standard/blob/master/LICENSE MIT License
  */

-namespace PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays;
+namespace CodeIgniter4\Sniffs\Arrays;

-use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Util\Tokens;
+use CodeIgniter4\Util\Common;

+/**
+ * Array Declaration Sniff
+ *
+ * Check array declaration, indentation, alignment and that the last item
+ * has a trailing comma.
+ *
+ * @author Louis Linehan <louis.linehan@gmail.com>
+ */
 class ArrayDeclarationSniff implements Sniff
 {

+    /**
+     * The --tab-width CLI value that is being used.
+     *
+     * @var integer
+     */
+    private $tabWidth = null;
+
+    /**
+     * Indent unit.
+     *
+     * Either space or tab.
+     *
+     * @var string
+     */
+    private $indentUnit = 'space';
+

     /**
      * Returns an array of tokens this test wants to listen for.
@@ -43,55 +69,52 @@
      */
     public function process(File $phpcsFile, $stackPtr)
     {
+
+        if ($this->tabWidth === null) {
+            if (isset($phpcsFile->config->tabWidth) === false || $phpcsFile->config->tabWidth === 0) {
+                // We have no idea how wide tabs are, so assume 4 spaces for fixing.
+                // It shouldn't really matter because alignment and spacing sniffs
+                // elsewhere in the standard should fix things up.
+                $this->tabWidth = 4;
+            } else {
+                $this->tabWidth   = $phpcsFile->config->tabWidth;
+                $this->indentUnit = 'tab';
+            }
+        }
+
         $tokens = $phpcsFile->getTokens();

+        // First make sure all arrays use short array syntax, this makes fixing much easier.
         if ($tokens[$stackPtr]['code'] === T_ARRAY) {
-            $phpcsFile->recordMetric($stackPtr, 'Short array syntax used', 'no');
+            $error = 'Short array syntax must be used to define arrays';
+            $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'FoundLongArray');

-            // Array keyword should be lower case.
-            if ($tokens[$stackPtr]['content'] !== strtolower($tokens[$stackPtr]['content'])) {
-                if ($tokens[$stackPtr]['content'] === strtoupper($tokens[$stackPtr]['content'])) {
-                    $phpcsFile->recordMetric($stackPtr, 'Array keyword case', 'upper');
+            if ($fix === true) {
+                $tokens = $phpcsFile->getTokens();
+                $opener = $tokens[$stackPtr]['parenthesis_opener'];
+                $closer = $tokens[$stackPtr]['parenthesis_closer'];
+
+                $phpcsFile->fixer->beginChangeset();
+
+                if ($opener === null) {
+                    $phpcsFile->fixer->replaceToken($stackPtr, '[]');
                 } else {
-                    $phpcsFile->recordMetric($stackPtr, 'Array keyword case', 'mixed');
+                    $phpcsFile->fixer->replaceToken($stackPtr, '');
+                    $phpcsFile->fixer->replaceToken($opener, '[');
+                    $phpcsFile->fixer->replaceToken($closer, ']');
                 }

-                $error = 'Array keyword should be lower case; expected "array" but found "%s"';
-                $data  = [$tokens[$stackPtr]['content']];
-                $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'NotLowerCase', $data);
-                if ($fix === true) {
-                    $phpcsFile->fixer->replaceToken($stackPtr, 'array');
-                }
-            } else {
-                $phpcsFile->recordMetric($stackPtr, 'Array keyword case', 'lower');
+                $phpcsFile->fixer->endChangeset();
             }
+        }//end if

+        if ($tokens[$stackPtr]['code'] === T_ARRAY) {
             $arrayStart = $tokens[$stackPtr]['parenthesis_opener'];
             if (isset($tokens[$arrayStart]['parenthesis_closer']) === false) {
                 return;
             }

             $arrayEnd = $tokens[$arrayStart]['parenthesis_closer'];
-
-            if ($arrayStart !== ($stackPtr + 1)) {
-                $error = 'There must be no space between the "array" keyword and the opening parenthesis';
-
-                $next = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), $arrayStart, true);
-                if (isset(Tokens::$commentTokens[$tokens[$next]['code']]) === true) {
-                    // We don't have anywhere to put the comment, so don't attempt to fix it.
-                    $phpcsFile->addError($error, $stackPtr, 'SpaceAfterKeyword');
-                } else {
-                    $fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpaceAfterKeyword');
-                    if ($fix === true) {
-                        $phpcsFile->fixer->beginChangeset();
-                        for ($i = ($stackPtr + 1); $i < $arrayStart; $i++) {
-                            $phpcsFile->fixer->replaceToken($i, '');
-                        }
-
-                        $phpcsFile->fixer->endChangeset();
-                    }
-                }
-            }
         } else {
             $phpcsFile->recordMetric($stackPtr, 'Short array syntax used', 'yes');
             $arrayStart = $stackPtr;
@@ -229,27 +252,17 @@
         }//end while

         if ($valueCount > 0) {
-            $nestedParenthesis = false;
-            if (isset($tokens[$stackPtr]['nested_parenthesis']) === true) {
-                $nested            = $tokens[$stackPtr]['nested_parenthesis'];
-                $nestedParenthesis = array_pop($nested);
-            }
+            $conditionCheck = $phpcsFile->findPrevious([T_OPEN_PARENTHESIS, T_SEMICOLON], ($stackPtr - 1), null, false);

-            if ($nestedParenthesis === false
-                || $tokens[$nestedParenthesis]['line'] !== $tokens[$stackPtr]['line']
+            if ($conditionCheck === false
+                || $tokens[$conditionCheck]['line'] !== $tokens[$stackPtr]['line']
             ) {
                 $error = 'Array with multiple values cannot be declared on a single line';
                 $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'SingleLineNotAllowed');
                 if ($fix === true) {
                     $phpcsFile->fixer->beginChangeset();
                     $phpcsFile->fixer->addNewline($arrayStart);
-
-                    if ($tokens[($arrayEnd - 1)]['code'] === T_WHITESPACE) {
-                        $phpcsFile->fixer->replaceToken(($arrayEnd - 1), $phpcsFile->eolChar);
-                    } else {
                         $phpcsFile->fixer->addNewlineBefore($arrayEnd);
-                    }
-
                     $phpcsFile->fixer->endChangeset();
                 }

@@ -320,25 +333,135 @@
         $tokens       = $phpcsFile->getTokens();
         $keywordStart = $tokens[$stackPtr]['column'];

-        // Check the closing bracket is on a new line.
+        $prevNonWhitespaceToken = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true);
+
+        // Find where this array should be indented from.
+        switch ($tokens[$prevNonWhitespaceToken]['code']) {
+        case T_EQUAL:
+        case T_OPEN_PARENTHESIS:
+            // It's "=", "(" or "return".
+            $starts = [
+                T_VARIABLE,
+                T_VAR,
+                T_PUBLIC,
+                T_PRIVATE,
+                T_PROTECTED,
+                T_ARRAY_CAST,
+                T_UNSET_CAST,
+                T_OBJECT_CAST,
+                T_STATIC,
+                T_CONST,
+                T_RETURN,
+                T_OBJECT_OPERATOR,
+            ];
+
+            $firstOnLine = $phpcsFile->findFirstOnLine($starts, $prevNonWhitespaceToken);
+            $indentStart = $firstOnLine;
+            break;
+        case T_DOUBLE_ARROW:
+            // It's an array in an array "=> []".
+            $indentStart = $phpcsFile->findPrevious(T_WHITESPACE, ($prevNonWhitespaceToken - 1), null, true);
+            break;
+        case T_RETURN:
+            $indentStart = $prevNonWhitespaceToken;
+            break;
+        case T_COMMENT:
+        case T_OPEN_SHORT_ARRAY:
+            // It's an array in an array "[[]]" or the end of an array line "[],".
+            $indentStart = $stackPtr;
+            break;
+        case T_COMMA:
+            // The end of an array line "[],".
+            // Argument in a function "$item->save($data, [...], ...)".
+            $starts = [
+                T_VARIABLE,
+                T_VAR,
+                T_PUBLIC,
+                T_PRIVATE,
+                T_PROTECTED,
+                T_ARRAY_CAST,
+                T_UNSET_CAST,
+                T_OBJECT_CAST,
+                T_STATIC,
+                T_CONST,
+                T_RETURN,
+                T_OBJECT_OPERATOR,
+                T_CLOSE_SHORT_ARRAY,
+                T_CONSTANT_ENCAPSED_STRING,
+            ];
+
+            $firstOnLine = $phpcsFile->findFirstOnLine($starts, $prevNonWhitespaceToken);
+            $indentStart = $firstOnLine;
+            break;
+        default:
+            // Nothing expected preceded this so leave ptr where it is and
+            // it should get picked in a future pass.
+            $indentStart = $stackPtr;
+        }//end switch
+
+        // If this is the first argument in a function ensure the bracket to be right after the parenthesis. eg "array_combine([".
+        if ($tokens[$prevNonWhitespaceToken]['code'] === T_OPEN_PARENTHESIS && $tokens[$stackPtr]['code'] === T_OPEN_SHORT_ARRAY) {
+            if ($tokens[$stackPtr]['line'] > $tokens[$prevNonWhitespaceToken]['line']) {
+                $error = 'Array openening bracket should be after function open parenthesis "(["';
+                $data  = [];
+                $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'ShortArrayOpenWrongLine', $data);
+                if ($fix === true) {
+                    $phpcsFile->fixer->beginChangeset();
+                    for ($i = ($prevNonWhitespaceToken + 1); $i < $stackPtr; $i++) {
+                        $phpcsFile->fixer->replaceToken($i, '');
+                    }
+
+                    $phpcsFile->fixer->endChangeset();
+                }
+            }
+        }
+
+        // Get content before closing array bracket/brace.
         $lastContent = $phpcsFile->findPrevious(T_WHITESPACE, ($arrayEnd - 1), $arrayStart, true);
+
+        // Check for ) after last Array end.
+        $afterCloser = $phpcsFile->findNext(T_WHITESPACE, ($arrayEnd + 1), null, true);
+        if ($tokens[$afterCloser]['code'] === T_CLOSE_PARENTHESIS) {
+            if ($tokens[$afterCloser]['column'] !== ($tokens[$arrayEnd]['column'] + 1)) {
+                $error = 'Closing parenthesis should be after array closing bracket "])"';
+                $data  = [];
+                $fix   = $phpcsFile->addFixableError($error, $afterCloser, 'CloseBracketAfterArrayBracket');
+                if ($fix === true) {
+                    $phpcsFile->fixer->beginChangeset();
+                    for ($i = ($arrayEnd + 1); $i < $afterCloser; $i++) {
+                        $phpcsFile->fixer->replaceToken($i, '');
+                    }
+
+                    $phpcsFile->fixer->endChangeset();
+                }
+            }
+        }
+
+        // Check the closing bracket is on a new line.
         if ($tokens[$lastContent]['line'] === $tokens[$arrayEnd]['line']) {
             $error = 'Closing parenthesis of array declaration must be on a new line';
-            $fix   = $phpcsFile->addFixableError($error, $arrayEnd, 'CloseBraceNewLine');
+            $fix   = $phpcsFile->addFixableError($error, $arrayEnd, 'CloseArrayBraceNewLine');
             if ($fix === true) {
                 $phpcsFile->fixer->addNewlineBefore($arrayEnd);
             }
-        } else if ($tokens[$arrayEnd]['column'] !== $keywordStart) {
-            // Check the closing bracket is lined up under the "a" in array.
-            $expected = ($keywordStart - 1);
+        } else if ($tokens[$arrayEnd]['column'] !== $tokens[$indentStart]['column']) {
+            // Check the closing bracket with the array declarer or if an
+            // array in an array the previous array key.
+            if ($tokens[$indentStart]['column'] > 1) {
+                $expected = ($tokens[$indentStart]['column'] - 1);
+            } else {
+                $expected = $this->tabWidth;
+            }
+
             $found    = ($tokens[$arrayEnd]['column'] - 1);
-            $error    = 'Closing parenthesis not aligned correctly; expected %s space(s) but found %s';
+            $error = 'Closing parenthesis not aligned correctly; expected %s %s but found %s';
             $data     = [
-                $expected,
-                $found,
+                ($expected / $this->tabWidth),
+                Common::pluralize($this->indentUnit, ($expected / $this->tabWidth)),
+                ($found / $this->tabWidth),
             ];

-            $fix = $phpcsFile->addFixableError($error, $arrayEnd, 'CloseBraceNotAligned', $data);
+            $fix = $phpcsFile->addFixableError($error, $arrayEnd, 'CloseArrayBraceNotAligned', $data);
             if ($fix === true) {
                 if ($found === 0) {
                     $phpcsFile->fixer->addContent(($arrayEnd - 1), str_repeat(' ', $expected));
@@ -373,7 +496,6 @@
             if ($tokens[$nextToken]['code'] === T_ARRAY
                 || $tokens[$nextToken]['code'] === T_OPEN_SHORT_ARRAY
                 || $tokens[$nextToken]['code'] === T_CLOSURE
-                || $tokens[$nextToken]['code'] === T_FN
             ) {
                 // Let subsequent calls of this test handle nested arrays.
                 if ($tokens[$lastToken]['code'] !== T_DOUBLE_ARROW) {
@@ -386,7 +508,6 @@
                 } else if ($tokens[$nextToken]['code'] === T_OPEN_SHORT_ARRAY) {
                     $nextToken = $tokens[$nextToken]['bracket_closer'];
                 } else {
-                    // T_CLOSURE.
                     $nextToken = $tokens[$nextToken]['scope_closer'];
                 }

@@ -400,7 +521,9 @@
                 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;
             }

@@ -436,31 +559,24 @@

                 if ($keyUsed === false) {
                     if ($tokens[($nextToken - 1)]['code'] === T_WHITESPACE) {
-                        $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($nextToken - 1), null, true);
-                        if (($tokens[$prev]['code'] !== T_END_HEREDOC
-                                && $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 {
                                 $spaceLength = $tokens[($nextToken - 1)]['length'];
                             }

-                            $error = 'Expected 0 spaces before comma; %s found';
-                            $data  = [$spaceLength];
+                        $error = 'Expected 0 spaces between "%s" and comma; %s found';
+                        $data  = [
+                            $content,
+                            $spaceLength,
+                        ];

-                            // The error is only fixable if there is only whitespace between the tokens.
-                            if ($prev === $phpcsFile->findPrevious(T_WHITESPACE, ($nextToken - 1), null, true)) {
                                 $fix = $phpcsFile->addFixableError($error, $nextToken, 'SpaceBeforeComma', $data);
                                 if ($fix === true) {
                                     $phpcsFile->fixer->replaceToken(($nextToken - 1), '');
                                 }
-                            } else {
-                                $phpcsFile->addError($error, $nextToken, 'SpaceBeforeComma', $data);
-                            }
                         }
-                    }//end if

                     $valueContent = $phpcsFile->findNext(
                         Tokens::$emptyTokens,
@@ -494,19 +610,14 @@
                 if ($indexStart === $indexEnd) {
                     $currentEntry['index']         = $indexEnd;
                     $currentEntry['index_content'] = $tokens[$indexEnd]['content'];
-                    $currentEntry['index_length']  = $tokens[$indexEnd]['length'];
                 } else {
                     $currentEntry['index']         = $indexStart;
-                    $currentEntry['index_content'] = '';
-                    $currentEntry['index_length']  = 0;
-                    for ($i = $indexStart; $i <= $indexEnd; $i++) {
-                        $currentEntry['index_content'] .= $tokens[$i]['content'];
-                        $currentEntry['index_length']  += $tokens[$i]['length'];
-                    }
+                    $currentEntry['index_content'] = $phpcsFile->getTokensAsString($indexStart, ($indexEnd - $indexStart + 1));
                 }

-                if ($maxLength < $currentEntry['index_length']) {
-                    $maxLength = $currentEntry['index_length'];
+                $indexLength = mb_strlen($currentEntry['index_content']);
+                if ($maxLength < $indexLength) {
+                    $maxLength = $indexLength;
                 }

                 // Find the value of this index.
@@ -517,70 +628,15 @@
                     true
                 );

-                $currentEntry['value'] = $nextContent;
-                $indices[] = $currentEntry;
-                $lastToken = $nextToken;
-            }//end if
-        }//end for
-
-        // Check for multi-line arrays that should be single-line.
-        $singleValue = false;
-
-        if (empty($indices) === true) {
-            $singleValue = true;
-        } else if (count($indices) === 1 && $tokens[$lastToken]['code'] === T_COMMA) {
-            // There may be another array value without a comma.
-            $exclude     = Tokens::$emptyTokens;
-            $exclude[]   = T_COMMA;
-            $nextContent = $phpcsFile->findNext($exclude, ($indices[0]['value'] + 1), $arrayEnd, true);
             if ($nextContent === false) {
-                $singleValue = true;
-            }
-        }
-
-        if ($singleValue === true) {
-            // Before we complain, make sure the single value isn't a here/nowdoc.
-            $next = $phpcsFile->findNext(Tokens::$heredocTokens, ($arrayStart + 1), ($arrayEnd - 1));
-            if ($next === false) {
-                // Array cannot be empty, so this is a multi-line array with
-                // a single value. It should be defined on single line.
-                $error     = 'Multi-line array contains a single value; use single-line array instead';
-                $errorCode = 'MultiLineNotAllowed';
-
-                $find    = Tokens::$phpcsCommentTokens;
-                $find[]  = T_COMMENT;
-                $comment = $phpcsFile->findNext($find, ($arrayStart + 1), $arrayEnd);
-                if ($comment === false) {
-                    $fix = $phpcsFile->addFixableError($error, $stackPtr, $errorCode);
-                } else {
-                    $fix = false;
-                    $phpcsFile->addError($error, $stackPtr, $errorCode);
-                }
-
-                if ($fix === true) {
-                    $phpcsFile->fixer->beginChangeset();
-                    for ($i = ($arrayStart + 1); $i < $arrayEnd; $i++) {
-                        if ($tokens[$i]['code'] !== T_WHITESPACE) {
                             break;
                         }

-                        $phpcsFile->fixer->replaceToken($i, '');
-                    }
-
-                    for ($i = ($arrayEnd - 1); $i > $arrayStart; $i--) {
-                        if ($tokens[$i]['code'] !== T_WHITESPACE) {
-                            break;
-                        }
-
-                        $phpcsFile->fixer->replaceToken($i, '');
-                    }
-
-                    $phpcsFile->fixer->endChangeset();
-                }
-
-                return;
-            }//end if
+                $currentEntry['value'] = $nextContent;
+                $indices[] = $currentEntry;
+                $lastToken = $nextToken;
         }//end if
+        }//end for

         /*
             This section checks for arrays that don't specify keys.
@@ -615,7 +671,8 @@
                 $phpcsFile->recordMetric($stackPtr, 'Array end comma', 'yes');
             }

-            foreach ($indices as $valuePosition => $value) {
+            $lastValueLine = false;
+            foreach ($indices as $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.
@@ -623,62 +680,47 @@
                     continue;
                 }

-                $valuePointer = $value['value'];
-
-                $ignoreTokens  = [
-                    T_WHITESPACE => T_WHITESPACE,
-                    T_COMMA      => T_COMMA,
-                ];
-                $ignoreTokens += Tokens::$castTokens;
-
-                if ($tokens[$valuePointer]['code'] === T_CLOSURE
-                    || $tokens[$valuePointer]['code'] === T_FN
-                ) {
-                    $ignoreTokens += [T_STATIC => T_STATIC];
-                }
-
-                $previous = $phpcsFile->findPrevious($ignoreTokens, ($valuePointer - 1), ($arrayStart + 1), true);
-                if ($previous === false) {
-                    $previous = $stackPtr;
-                }
-
-                $previousIsWhitespace = $tokens[($valuePointer - 1)]['code'] === T_WHITESPACE;
-                if ($tokens[$previous]['line'] === $tokens[$valuePointer]['line']) {
+                if ($lastValueLine !== false && $tokens[$value['value']]['line'] === $lastValueLine) {
                     $error = 'Each value in a multi-line array must be on a new line';
-                    if ($valuePosition === 0) {
-                        $error = 'The first value in a multi-value array must be on a new line';
+                    $fix   = $phpcsFile->addFixableError($error, $value['value'], 'ValueNoNewline');
+                    if ($fix === true) {
+                        if ($tokens[($value['value'] - 1)]['code'] === T_WHITESPACE) {
+                            $phpcsFile->fixer->replaceToken(($value['value'] - 1), '');
                     }

-                    $fix = $phpcsFile->addFixableError($error, $valuePointer, 'ValueNoNewline');
-                    if ($fix === true) {
-                        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) {
+                    // Expected indent.
+                    if ($tokens[$indentStart]['column'] > 1) {
+                        $expected = ($tokens[$indentStart]['column'] + $this->tabWidth - 1);
+                    } else {
+                        $expected = $this->tabWidth;
                     }
-                } else if ($previousIsWhitespace === true) {
-                    $expected = $keywordStart;

-                    $first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $valuePointer, true);
+                    $first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $value['value'], true);
                     $found = ($tokens[$first]['column'] - 1);
+
                     if ($found !== $expected) {
-                        $error = 'Array value not aligned correctly; expected %s spaces but found %s';
+                        $error = 'Array value not aligned correctly; expected %s %s but found %s';
                         $data  = [
-                            $expected,
-                            $found,
+                            ($expected / $this->tabWidth),
+                            Common::pluralize($this->indentUnit, ($expected / $this->tabWidth)),
+                            ($found / $this->tabWidth),
                         ];

-                        $fix = $phpcsFile->addFixableError($error, $first, 'ValueNotAligned', $data);
+                        $fix = $phpcsFile->addFixableError($error, $value['value'], 'ValueNotAligned', $data);
                         if ($fix === true) {
                             if ($found === 0) {
-                                $phpcsFile->fixer->addContent(($first - 1), str_repeat(' ', $expected));
+                                $phpcsFile->fixer->addContent(($value['value'] - 1), str_repeat(' ', $expected));
                             } else {
-                                $phpcsFile->fixer->replaceToken(($first - 1), str_repeat(' ', $expected));
+                                $phpcsFile->fixer->replaceToken(($value['value'] - 1), str_repeat(' ', $expected));
                             }
                         }
                     }
                 }//end if
+
+                $lastValueLine = $tokens[$value['value']]['line'];
             }//end foreach
         }//end if

@@ -709,71 +751,91 @@
             to be moved back one space however, then both errors would be fixed.
         */

-        $indicesStart = ($keywordStart + 1);
-        foreach ($indices as $valuePosition => $index) {
-            $valuePointer = $index['value'];
-            if ($valuePointer === false) {
-                // Syntax error or live coding.
-                continue;
+        $numValues = count($indices);
+
+        // Expected indent.
+        if ($tokens[$indentStart]['column'] > 1) {
+            $indicesStart = ($tokens[$indentStart]['column'] + $this->tabWidth);
+        } else {
+            $indicesStart = $this->tabWidth;
             }

+        $arrowStart    = ($indicesStart + $maxLength + 1);
+        $valueStart    = ($arrowStart + 3);
+        $indexLine     = $tokens[$stackPtr]['line'];
+        $lastIndexLine = null;
+        foreach ($indices as $index) {
             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;
             }

-            $indexPointer = $index['index'];
-            $indexLine    = $tokens[$indexPointer]['line'];
+            $lastIndexLine = $indexLine;
+            $indexLine     = $tokens[$index['index']]['line'];

-            $previous = $phpcsFile->findPrevious([T_WHITESPACE, T_COMMA], ($indexPointer - 1), ($arrayStart + 1), true);
-            if ($previous === false) {
-                $previous = $stackPtr;
+            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']);
             }

-            if ($tokens[$previous]['line'] === $indexLine) {
-                $error = 'Each index in a multi-line array must be on a new line';
-                if ($valuePosition === 0) {
-                    $error = 'The first index in a multi-value array must be on a new line';
+                continue;
                 }

-                $fix = $phpcsFile->addFixableError($error, $indexPointer, 'IndexNoNewline');
+            if ($indexLine === $lastIndexLine) {
+                $error = 'Each index in a multi-line array must be on a new line';
+                $fix   = $phpcsFile->addFixableError($error, $index['index'], 'IndexNoNewline');
                 if ($fix === true) {
-                    if ($tokens[($indexPointer - 1)]['code'] === T_WHITESPACE) {
-                        $phpcsFile->fixer->replaceToken(($indexPointer - 1), $phpcsFile->eolChar);
-                    } else {
-                        $phpcsFile->fixer->addNewlineBefore($indexPointer);
+                    if ($tokens[($index['index'] - 1)]['code'] === T_WHITESPACE) {
+                        $phpcsFile->fixer->replaceToken(($index['index'] - 1), '');
                     }
+
+                    $phpcsFile->fixer->addNewlineBefore($index['index']);
                 }

                 continue;
             }

-            if ($tokens[$indexPointer]['column'] !== $indicesStart && ($indexPointer - 1) !== $arrayStart) {
+            if ($tokens[$index['index']]['column'] !== $indicesStart) {
                 $expected = ($indicesStart - 1);
-                $found    = ($tokens[$indexPointer]['column'] - 1);
-                $error    = 'Array key not aligned correctly; expected %s spaces but found %s';
+
+                $found = ($tokens[$index['index']]['column'] - 1);
+                $error = 'Array key not aligned correctly; expected %s %s but found %s';
                 $data     = [
-                    $expected,
-                    $found,
+                    ($expected / $this->tabWidth),
+                    Common::pluralize($this->indentUnit, ($expected / $this->tabWidth)),
+                    ($found / $this->tabWidth),
                 ];

-                $fix = $phpcsFile->addFixableError($error, $indexPointer, 'KeyNotAligned', $data);
+                $fix = $phpcsFile->addFixableError($error, $index['index'], 'KeyNotAligned', $data);
                 if ($fix === true) {
-                    if ($found === 0 || $tokens[($indexPointer - 1)]['code'] !== T_WHITESPACE) {
-                        $phpcsFile->fixer->addContent(($indexPointer - 1), str_repeat(' ', $expected));
+                    if ($found === 0) {
+                        $phpcsFile->fixer->addContent(($index['index'] - 1), str_repeat(' ', $expected));
                     } else {
-                        $phpcsFile->fixer->replaceToken(($indexPointer - 1), str_repeat(' ', $expected));
-                    }
+                        $phpcsFile->fixer->replaceToken(($index['index'] - 1), str_repeat(' ', $expected));
                 }
             }

-            $arrowStart = ($tokens[$indexPointer]['column'] + $maxLength + 1);
+                continue;
+            }//end if
+
             if ($tokens[$index['arrow']]['column'] !== $arrowStart) {
-                $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';
+                $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']));
+                $error = 'Array double arrow not aligned correctly; expected %s %s but found %s';
                 $data     = [
                     $expected,
+                    Common::pluralize('space', $expected),
                     $found,
                 ];

@@ -787,91 +849,101 @@
                 }

                 continue;
-            }
+            }//end if

-            $valueStart = ($arrowStart + 3);
-            if ($tokens[$valuePointer]['column'] !== $valueStart) {
+            if ($tokens[$index['value']]['column'] !== $valueStart) {
                 $expected = ($valueStart - ($tokens[$index['arrow']]['length'] + $tokens[$index['arrow']]['column']));
-                $found    = ($tokens[$valuePointer]['column'] - ($tokens[$index['arrow']]['length'] + $tokens[$index['arrow']]['column']));
+                $found    = ($tokens[$index['value']]['column'] - ($tokens[$index['arrow']]['length'] + $tokens[$index['arrow']]['column']));
                 if ($found < 0) {
                     $found = 'newline';
                 }

-                $error = 'Array value not aligned correctly; expected %s space(s) but found %s';
+                $error = 'Array value not aligned correctly; expected %s %s but found %s';
                 $data  = [
                     $expected,
+                    Common::pluralize('space', $expected),
                     $found,
                 ];

                 $fix = $phpcsFile->addFixableError($error, $index['arrow'], 'ValueNotAligned', $data);
                 if ($fix === true) {
                     if ($found === 'newline') {
-                        $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($valuePointer - 1), null, true);
+                        $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($index['value'] - 1), null, true);
                         $phpcsFile->fixer->beginChangeset();
-                        for ($i = ($prev + 1); $i < $valuePointer; $i++) {
+                        for ($i = ($prev + 1); $i < $index['value']; $i++) {
                             $phpcsFile->fixer->replaceToken($i, '');
                         }

-                        $phpcsFile->fixer->replaceToken(($valuePointer - 1), str_repeat(' ', $expected));
+                        $phpcsFile->fixer->replaceToken(($index['value'] - 1), str_repeat(' ', $expected));
                         $phpcsFile->fixer->endChangeset();
                     } else if ($found === 0) {
-                        $phpcsFile->fixer->addContent(($valuePointer - 1), str_repeat(' ', $expected));
+                        $phpcsFile->fixer->addContent(($index['value'] - 1), str_repeat(' ', $expected));
                     } else {
-                        $phpcsFile->fixer->replaceToken(($valuePointer - 1), str_repeat(' ', $expected));
+                        $phpcsFile->fixer->replaceToken(($index['value'] - 1), str_repeat(' ', $expected));
                     }
                 }
             }//end if

             // Check each line ends in a comma.
-            $valueStart = $valuePointer;
+            $valueLine = $tokens[$index['value']]['line'];
             $nextComma  = false;
+            for ($i = $index['value']; $i < $arrayEnd; $i++) {
+                // Skip bracketed statements, like function calls.
+                if ($tokens[$i]['code'] === T_OPEN_PARENTHESIS) {
+                    $i         = $tokens[$i]['parenthesis_closer'];
+                    $valueLine = $tokens[$i]['line'];
+                    continue;
+                }

-            $end = $phpcsFile->findEndOfStatement($valueStart);
-            if ($end === false) {
-                $valueEnd = $valueStart;
-            } else if ($tokens[$end]['code'] === T_COMMA) {
-                $valueEnd  = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($end - 1), $valueStart, true);
-                $nextComma = $end;
-            } else {
-                $valueEnd = $end;
-                $next     = $phpcsFile->findNext(Tokens::$emptyTokens, ($end + 1), $arrayEnd, true);
-                if ($next !== false && $tokens[$next]['code'] === T_COMMA) {
-                    $nextComma = $next;
+                if ($tokens[$i]['code'] === T_ARRAY) {
+                    $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;
             }

-            $valueLine = $tokens[$valueEnd]['line'];
-            if ($tokens[$valueEnd]['code'] === T_END_HEREDOC || $tokens[$valueEnd]['code'] === T_END_NOWDOC) {
-                $valueLine++;
+                if ($tokens[$i]['code'] === T_OPEN_SHORT_ARRAY) {
+                    $i         = $tokens[$i]['bracket_closer'];
+                    $valueLine = $tokens[$i]['line'];
+                    continue;
             }

+                if ($tokens[$i]['code'] === T_CLOSURE) {
+                    $i         = $tokens[$i]['scope_closer'];
+                    $valueLine = $tokens[$i]['line'];
+                    continue;
+                }
+
+                if ($tokens[$i]['code'] === T_COMMA) {
+                    $nextComma = $i;
+                    break;
+                }
+            }//end for
+
             if ($nextComma === false || ($tokens[$nextComma]['line'] !== $valueLine)) {
                 $error = 'Each line in an array declaration must end in a comma';
-                $fix   = $phpcsFile->addFixableError($error, $valuePointer, 'NoComma');
-
+                $fix   = $phpcsFile->addFixableError($error, $index['value'], 'NoComma');
                 if ($fix === true) {
                     // Find the end of the line and put a comma there.
-                    for ($i = ($valuePointer + 1); $i <= $arrayEnd; $i++) {
+                    for ($i = ($index['value'] + 1); $i < $arrayEnd; $i++) {
                         if ($tokens[$i]['line'] > $valueLine) {
                             break;
                         }
                     }

-                    $phpcsFile->fixer->beginChangeset();
                     $phpcsFile->fixer->addContentBefore(($i - 1), ',');
-                    if ($nextComma !== false) {
-                        $phpcsFile->fixer->replaceToken($nextComma, '');
-                    }
-
-                    $phpcsFile->fixer->endChangeset();
                 }
             }//end if

             // Check that there is no space before the comma.
             if ($nextComma !== false && $tokens[($nextComma - 1)]['code'] === T_WHITESPACE) {
-                // Here/nowdoc closing tags must have the comma on the next line.
-                $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($nextComma - 1), null, true);
-                if ($tokens[$prev]['code'] !== T_END_HEREDOC && $tokens[$prev]['code'] !== T_END_NOWDOC) {
                     $content     = $tokens[($nextComma - 2)]['content'];
                     $spaceLength = $tokens[($nextComma - 1)]['length'];
                     $error       = 'Expected 0 spaces between "%s" and comma; %s found';
@@ -885,7 +957,6 @@
                         $phpcsFile->fixer->replaceToken(($nextComma - 1), '');
                     }
                 }
-            }
         }//end foreach

     }//end processMultiLineArray()
kenjis commented 3 years ago

@WinterSilence Thanks for the info. It seems it derived from the old version.

Anyway, to read it is too difficult to me...

WinterSilence commented 3 years ago

@kenjis what you want change in PHP_CodeSniffer\Standards\Squiz\Sniffs\Arrays\ArrayDeclarationSniff?

kenjis commented 3 years ago

@WinterSilence I want fix the bug: https://github.com/codeigniter4/coding-standard/issues/41

paulbalandan commented 3 years ago

Closing as we have substantially cleaned the framework of coding standards violations.