bug: TrailingCommaInMultilineFixer - do not add trailing comma when t…

…here is no break line after last element (#6677)
PHP-CS-Fixer · Nov 4, 2022 · bb94db0 · bb94db0
1 parent 3969f39
commit bb94db0
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 32 deletions.
diff --git a/src/Fixer/ArrayNotation/NoMultilineWhitespaceAroundDoubleArrowFixer.php b/src/Fixer/ArrayNotation/NoMultilineWhitespaceAroundDoubleArrowFixer.php
@@ -42,7 +42,7 @@ public function getDefinition(): FixerDefinitionInterface
     /**
      * {@inheritdoc}
      *
-     * Must run before BinaryOperatorSpacesFixer, MethodArgumentSpaceFixer, TrailingCommaInMultilineFixer.
+     * Must run before BinaryOperatorSpacesFixer, MethodArgumentSpaceFixer.
      */
     public function getPriority(): int
     {

diff --git a/src/Fixer/ControlStructure/TrailingCommaInMultilineFixer.php b/src/Fixer/ControlStructure/TrailingCommaInMultilineFixer.php
@@ -86,16 +86,6 @@ public function getDefinition(): FixerDefinitionInterface
         );
     }
 
-    /**
-     * {@inheritdoc}
-     *
-     * Must run after NoMultilineWhitespaceAroundDoubleArrowFixer.
-     */
-    public function getPriority(): int
-    {
-        return 0;
-    }
-
     /**
      * {@inheritdoc}
      */
@@ -201,6 +191,9 @@ private function fixBlock(Tokens $tokens, int $startIndex): void
         $endIndex = $tokens->findBlockEnd($blockType['type'], $startIndex);
 
         $beforeEndIndex = $tokens->getPrevMeaningfulToken($endIndex);
+        if (!$tokens->isPartialCodeMultiline($beforeEndIndex, $endIndex)) {
+            return;
+        }
         $beforeEndToken = $tokens[$beforeEndIndex];
 
         // if there is some item between braces then add `,` after it
@@ -242,6 +235,9 @@ private function fixMatch(Tokens $tokens, int $index): void
         }
 
         $previousIndex = $tokens->getPrevMeaningfulToken($closeIndex);
+        if (!$tokens->isPartialCodeMultiline($previousIndex, $closeIndex)) {
+            return;
+        }
 
         if (!$tokens[$previousIndex]->equals(',')) {
             $tokens->insertAt($previousIndex + 1, new Token(','));

diff --git a/tests/AutoReview/FixerFactoryTest.php b/tests/AutoReview/FixerFactoryTest.php
@@ -560,7 +560,6 @@ private static function getFixersPriorityGraph(): array
             'no_multiline_whitespace_around_double_arrow' => [
                 'binary_operator_spaces',
                 'method_argument_space',
-                'trailing_comma_in_multiline',
             ],
             'no_php4_constructor' => [
                 'ordered_class_elements',

diff --git a/tests/Fixer/ControlStructure/TrailingCommaInMultilineFixerTest.php b/tests/Fixer/ControlStructure/TrailingCommaInMultilineFixerTest.php
@@ -80,19 +80,15 @@ public static function provideFixCases(): array
             ["<?php \$x = array(\narray('foo'),\n);", "<?php \$x = array(\narray('foo')\n);"],
             ["<?php \$x = array(\n /* He */ \n);"],
             [
-                "<?php \$x = array('a', 'b', 'c',\n  'd', 'q', 'z', );",
                 "<?php \$x = array('a', 'b', 'c',\n  'd', 'q', 'z');",
             ],
             [
-                "<?php \$x = array('a', 'b', 'c',\n'd', 'q', 'z', );",
                 "<?php \$x = array('a', 'b', 'c',\n'd', 'q', 'z');",
             ],
             [
-                "<?php \$x = array('a', 'b', 'c',\n'd', 'q', 'z', );",
                 "<?php \$x = array('a', 'b', 'c',\n'd', 'q', 'z' );",
             ],
             [
-                "<?php \$x = array('a', 'b', 'c',\n'd', 'q', 'z',\t);",
                 "<?php \$x = array('a', 'b', 'c',\n'd', 'q', 'z'\t);",
             ],
             ["<?php \$x = array(\n<<<EOT\noet\nEOT\n);"],
@@ -430,8 +426,8 @@ function a()
                 ['elements' => [TrailingCommaInMultilineFixer::ELEMENTS_ARRAYS]],
             ],
             [
-                "<?php \$var = foo('a', 'b', 'c',\n  'd', 'q', 'z', );",
                 "<?php \$var = foo('a', 'b', 'c',\n  'd', 'q', 'z');",
+                null,
                 ['elements' => [TrailingCommaInMultilineFixer::ELEMENTS_ARGUMENTS]],
             ],
             [
@@ -440,13 +436,13 @@ function a()
                 ['elements' => [TrailingCommaInMultilineFixer::ELEMENTS_ARGUMENTS]],
             ],
             [
-                "<?php \$var = \$foonction('a', 'b', 'c',\n  'd', 'q', 'z', );",
                 "<?php \$var = \$foonction('a', 'b', 'c',\n  'd', 'q', 'z');",
+                null,
                 ['elements' => [TrailingCommaInMultilineFixer::ELEMENTS_ARGUMENTS]],
             ],
             [
-                "<?php \$var = \$fMap[100]('a', 'b', 'c',\n  'd', 'q', 'z', );",
                 "<?php \$var = \$fMap[100]('a', 'b', 'c',\n  'd', 'q', 'z');",
+                null,
                 ['elements' => [TrailingCommaInMultilineFixer::ELEMENTS_ARGUMENTS]],
             ],
             [
@@ -653,5 +649,14 @@ public static function provideFix80Cases(): iterable
             ',
             ['elements' => ['match']],
         ];
+
+        yield 'match with last comma in the same line as closing brace' => [
+            '<?php
+$x = match ($a) { 1 => 0,
+                  2 => 1 };
+            ',
+            null,
+            ['elements' => ['match']],
+        ];
     }
 }
diff --git a/...ion/priority/no_multiline_whitespace_around_double_arrow,trailing_comma_in_multiline.test b/...ion/priority/no_multiline_whitespace_around_double_arrow,trailing_comma_in_multiline.test