Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: TrailingCommaInMultilineFixer - do not add trailing comma when there is no break line after last element #6677

Merged
merged 1 commit into from Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -42,7 +42,7 @@ public function getDefinition(): FixerDefinitionInterface
/**
* {@inheritdoc}
*
* Must run before BinaryOperatorSpacesFixer, MethodArgumentSpaceFixer, TrailingCommaInMultilineFixer.
* Must run before BinaryOperatorSpacesFixer, MethodArgumentSpaceFixer.
*/
public function getPriority(): int
{
Expand Down
16 changes: 6 additions & 10 deletions src/Fixer/ControlStructure/TrailingCommaInMultilineFixer.php
Expand Up @@ -86,16 +86,6 @@ public function getDefinition(): FixerDefinitionInterface
);
}

/**
* {@inheritdoc}
*
* Must run after NoMultilineWhitespaceAroundDoubleArrowFixer.
*/
public function getPriority(): int
{
return 0;
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(','));
Expand Down
1 change: 0 additions & 1 deletion tests/AutoReview/FixerFactoryTest.php
Expand Up @@ -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',
Expand Down
19 changes: 12 additions & 7 deletions tests/Fixer/ControlStructure/TrailingCommaInMultilineFixerTest.php
Expand Up @@ -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);"],
Expand Down Expand Up @@ -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]],
],
[
Expand All @@ -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]],
],
[
Expand Down Expand Up @@ -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']],
];
}
}

This file was deleted.