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

NoTrailingCommaInSinglelineFunctionCallFixer - Introduction #6304

Merged
merged 1 commit into from Feb 21, 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
11 changes: 11 additions & 0 deletions doc/list.rst
Expand Up @@ -741,6 +741,10 @@ List of Available Rules
| Spacing to use before open parenthesis for closures.
| Allowed values: ``'none'``, ``'one'``
| Default value: ``'one'``
- | ``trailing_comma_single_line``
| Whether trailing commas are allowed in single line signatures.
| Allowed types: ``bool``
| Default value: ``false``


Part of rule sets `@PSR12 <./ruleSets/PSR12.rst>`_ `@PSR2 <./ruleSets/PSR2.rst>`_ `@PhpCsFixer <./ruleSets/PhpCsFixer.rst>`_ `@Symfony <./ruleSets/Symfony.rst>`_
Expand Down Expand Up @@ -1538,6 +1542,13 @@ List of Available Rules
Part of rule sets `@PhpCsFixer <./ruleSets/PhpCsFixer.rst>`_ `@Symfony <./ruleSets/Symfony.rst>`_

`Source PhpCsFixer\\Fixer\\ArrayNotation\\NoTrailingCommaInSinglelineArrayFixer <./../src/Fixer/ArrayNotation/NoTrailingCommaInSinglelineArrayFixer.php>`_
- `no_trailing_comma_in_singleline_function_call <./rules/function_notation/no_trailing_comma_in_singleline_function_call.rst>`_

When making a method or function call on a single line there MUST NOT be a trailing comma after the last argument.

Part of rule sets `@PhpCsFixer <./ruleSets/PhpCsFixer.rst>`_ `@Symfony <./ruleSets/Symfony.rst>`_

`Source PhpCsFixer\\Fixer\\FunctionNotation\\NoTrailingCommaInSinglelineFunctionCallFixer <./../src/Fixer/FunctionNotation/NoTrailingCommaInSinglelineFunctionCallFixer.php>`_
- `no_trailing_whitespace <./rules/whitespace/no_trailing_whitespace.rst>`_

Remove trailing whitespace at the end of non-blank lines.
Expand Down
1 change: 1 addition & 0 deletions doc/ruleSets/Symfony.rst
Expand Up @@ -70,6 +70,7 @@ Rules
``['allow_mixed' => true, 'allow_unused_params' => true]``
- `no_trailing_comma_in_list_call <./../rules/control_structure/no_trailing_comma_in_list_call.rst>`_
- `no_trailing_comma_in_singleline_array <./../rules/array_notation/no_trailing_comma_in_singleline_array.rst>`_
- `no_trailing_comma_in_singleline_function_call <./../rules/function_notation/no_trailing_comma_in_singleline_function_call.rst>`_
- `no_unneeded_control_parentheses <./../rules/control_structure/no_unneeded_control_parentheses.rst>`_
config:
``['statements' => ['break', 'clone', 'continue', 'echo_print', 'return', 'switch_case', 'yield', 'yield_from']]``
Expand Down
9 changes: 9 additions & 0 deletions doc/rules/function_notation/function_declaration.rst
Expand Up @@ -16,6 +16,15 @@ Allowed values: ``'none'``, ``'one'``

Default value: ``'one'``

``trailing_comma_single_line``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Whether trailing commas are allowed in single line signatures.

Allowed types: ``bool``

Default value: ``false``

Examples
--------

Expand Down
@@ -0,0 +1,31 @@
======================================================
Rule ``no_trailing_comma_in_singleline_function_call``
======================================================

When making a method or function call on a single line there MUST NOT be a
trailing comma after the last argument.

Examples
--------

Example #1
~~~~~~~~~~

.. code-block:: diff

--- Original
+++ New
<?php
-foo($a,);
+foo($a);

Rule sets
---------

The rule is part of the following rule sets:

@PhpCsFixer
Using the `@PhpCsFixer <./../../ruleSets/PhpCsFixer.rst>`_ rule set will enable the ``no_trailing_comma_in_singleline_function_call`` rule.

@Symfony
Using the `@Symfony <./../../ruleSets/Symfony.rst>`_ rule set will enable the ``no_trailing_comma_in_singleline_function_call`` rule.
3 changes: 3 additions & 0 deletions doc/rules/index.rst
Expand Up @@ -340,6 +340,9 @@ Function Notation
- `no_spaces_after_function_name <./function_notation/no_spaces_after_function_name.rst>`_

When making a method or function call, there MUST NOT be a space between the method or function name and the opening parenthesis.
- `no_trailing_comma_in_singleline_function_call <./function_notation/no_trailing_comma_in_singleline_function_call.rst>`_

When making a method or function call on a single line there MUST NOT be a trailing comma after the last argument.
- `no_unreachable_default_argument_value <./function_notation/no_unreachable_default_argument_value.rst>`_ *(risky)*

In function arguments there must not be arguments with default values before non-default ones.
Expand Down
38 changes: 32 additions & 6 deletions src/Fixer/FunctionNotation/FunctionDeclarationFixer.php
Expand Up @@ -47,10 +47,7 @@ final class FunctionDeclarationFixer extends AbstractFixer implements Configurab

private const SUPPORTED_SPACINGS = [self::SPACING_NONE, self::SPACING_ONE];

/**
* @var string
*/
private $singleLineWhitespaceOptions = " \t";
private string $singleLineWhitespaceOptions = " \t";

/**
* {@inheritdoc}
Expand Down Expand Up @@ -134,6 +131,17 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
}

$endParenthesisIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $startParenthesisIndex);

if (false === $this->configuration['trailing_comma_single_line']
&& !$tokens->isPartialCodeMultiline($index, $endParenthesisIndex)
) {
$commaIndex = $tokens->getPrevMeaningfulToken($endParenthesisIndex);

if ($tokens[$commaIndex]->equals(',')) {
$tokens->clearTokenAndMergeSurroundingWhitespace($commaIndex);
}
}

$startBraceIndex = $tokens->getNextTokenOfKind($endParenthesisIndex, [';', '{', [T_DOUBLE_ARROW]]);

// fix single-line whitespace before { or =>
Expand All @@ -160,6 +168,16 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
$useStartParenthesisIndex = $tokens->getNextTokenOfKind($afterParenthesisIndex, ['(']);
$useEndParenthesisIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $useStartParenthesisIndex);

if (false === $this->configuration['trailing_comma_single_line']
&& !$tokens->isPartialCodeMultiline($index, $useEndParenthesisIndex)
) {
$commaIndex = $tokens->getPrevMeaningfulToken($useEndParenthesisIndex);

if ($tokens[$commaIndex]->equals(',')) {
$tokens->clearTokenAndMergeSurroundingWhitespace($commaIndex);
}
}

// remove single-line edge whitespaces inside use parentheses
$this->fixParenthesisInnerEdge($tokens, $useStartParenthesisIndex, $useEndParenthesisIndex);

Expand Down Expand Up @@ -211,14 +229,22 @@ protected function createConfigurationDefinition(): FixerConfigurationResolverIn
->setDefault(self::SPACING_ONE)
->setAllowedValues(self::SUPPORTED_SPACINGS)
->getOption(),
(new FixerOptionBuilder('trailing_comma_single_line', 'Whether trailing commas are allowed in single line signatures.'))
->setAllowedTypes(['bool'])
->setDefault(false)
->getOption(),
]);
}

private function fixParenthesisInnerEdge(Tokens $tokens, int $start, int $end): void
{
do {
--$end;
} while ($tokens->isEmptyAt($end));

// remove single-line whitespace before `)`
if ($tokens[$end - 1]->isWhitespace($this->singleLineWhitespaceOptions)) {
$tokens->clearAt($end - 1);
if ($tokens[$end]->isWhitespace($this->singleLineWhitespaceOptions)) {
$tokens->clearAt($end);
}

// remove single-line whitespace after `(`
Expand Down
@@ -0,0 +1,112 @@
<?php

declare(strict_types=1);

/*
* This file is part of PHP CS Fixer.
*
* (c) Fabien Potencier <fabien@symfony.com>
* Dariusz Rumiński <dariusz.ruminski@gmail.com>
*
* This source file is subject to the MIT license that is bundled
* with this source code in the file LICENSE.
*/

namespace PhpCsFixer\Fixer\FunctionNotation;

use PhpCsFixer\AbstractFixer;
use PhpCsFixer\FixerDefinition\CodeSample;
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
use PhpCsFixer\Tokenizer\Analyzer\AttributeAnalyzer;
use PhpCsFixer\Tokenizer\CT;
use PhpCsFixer\Tokenizer\Tokens;

final class NoTrailingCommaInSinglelineFunctionCallFixer extends AbstractFixer
{
/**
* {@inheritdoc}
*/
public function getDefinition(): FixerDefinitionInterface
{
return new FixerDefinition(
'When making a method or function call on a single line there MUST NOT be a trailing comma after the last argument.',
[new CodeSample("<?php\nfoo(\$a,);\n")]
);
}

/**
* {@inheritdoc}
*
* Must run before NoSpacesInsideParenthesisFixer.
*/
public function getPriority(): int
{
return 3;
}

/**
* {@inheritdoc}
*/
public function isCandidate(Tokens $tokens): bool
{
return $tokens->isAnyTokenKindsFound([T_STRING, T_VARIABLE, T_CLASS, T_UNSET, T_ISSET, T_LIST]);
}

/**
* {@inheritdoc}
*/
protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
{
for ($index = \count($tokens) - 1; $index > 0; --$index) {
if (!$tokens[$index]->equals(')')) {
continue;
}

$trailingCommaIndex = $tokens->getPrevMeaningfulToken($index);

if (!$tokens[$trailingCommaIndex]->equals(',')) {
continue;
}

$callIndex = $tokens->getPrevMeaningfulToken( // get before "parenthesis open index"
$tokens->findBlockStart(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $index)
);

if ($tokens[$callIndex]->isGivenKind([T_VARIABLE, T_CLASS, T_UNSET, T_ISSET, T_LIST])) {
$this->clearCommaIfNeeded($tokens, $callIndex, $index, $trailingCommaIndex);

continue;
}

if ($tokens[$callIndex]->isGivenKind(T_STRING)) {
if (!AttributeAnalyzer::isAttribute($tokens, $callIndex)) {
$this->clearCommaIfNeeded($tokens, $callIndex, $index, $trailingCommaIndex);
}

continue;
}

if ($tokens[$callIndex]->equalsAny([')', ']', [CT::T_DYNAMIC_VAR_BRACE_CLOSE], [CT::T_ARRAY_INDEX_CURLY_BRACE_CLOSE]])) {
$block = Tokens::detectBlockType($tokens[$callIndex]);
if (
Tokens::BLOCK_TYPE_ARRAY_INDEX_CURLY_BRACE === $block['type']
|| Tokens::BLOCK_TYPE_DYNAMIC_VAR_BRACE === $block['type']
|| Tokens::BLOCK_TYPE_INDEX_SQUARE_BRACE === $block['type']
|| Tokens::BLOCK_TYPE_PARENTHESIS_BRACE === $block['type']
) {
$this->clearCommaIfNeeded($tokens, $callIndex, $index, $trailingCommaIndex);

// continue; implicit
}
}
}
}

private function clearCommaIfNeeded(Tokens $tokens, int $startIndex, int $endIndex, int $commaIndex): void
{
if (!$tokens->isPartialCodeMultiline($startIndex, $endIndex)) {
$tokens->clearTokenAndMergeSurroundingWhitespace($commaIndex);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this way, not:

            $tokens->removeTrailingWhitespace($beforeEndIndex);
            $tokens->clearAt($beforeEndIndex);

as it's done in NoTrailingCommaInSinglelineArrayFixer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I follow, what would be the benefit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistency, better fixing - now code:

$a = [1, 2, 3, ];
foo(1, 2, 3, );

would be changed to:

$a = [1, 2, 3];
foo(1, 2, 3 );

I cannot imagine anyone want to have the comma removed and keep it's trailing whitespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check, I didn't even know removeTrailingWhitespace existed, I think your case is covered through single-responsibility of the rule combined with priorities, but if there is more room for improvement than lets PR it.

}
}
}
2 changes: 1 addition & 1 deletion src/Fixer/Whitespace/NoSpacesInsideParenthesisFixer.php
Expand Up @@ -51,7 +51,7 @@ function foo( \$bar, \$baz )
* {@inheritdoc}
*
* Must run before FunctionToConstantFixer, GetClassToClassKeywordFixer, StringLengthToEmptyFixer.
* Must run after CombineConsecutiveIssetsFixer, CombineNestedDirnameFixer, LambdaNotUsedImportFixer, ModernizeStrposFixer, NoUselessSprintfFixer, PowToExponentiationFixer.
* Must run after CombineConsecutiveIssetsFixer, CombineNestedDirnameFixer, LambdaNotUsedImportFixer, ModernizeStrposFixer, NoTrailingCommaInSinglelineFunctionCallFixer, NoUselessSprintfFixer, PowToExponentiationFixer.
*/
public function getPriority(): int
{
Expand Down
1 change: 1 addition & 0 deletions src/RuleSet/Sets/SymfonySet.php
Expand Up @@ -104,6 +104,7 @@ public function getRules(): array
],
'no_trailing_comma_in_list_call' => true,
'no_trailing_comma_in_singleline_array' => true,
'no_trailing_comma_in_singleline_function_call' => true,
'no_unneeded_control_parentheses' => [
'statements' => [
'break',
Expand Down
3 changes: 3 additions & 0 deletions tests/AutoReview/FixerFactoryTest.php
Expand Up @@ -561,6 +561,9 @@ private static function getFixersPriorityGraph(): array
'no_empty_phpdoc',
'void_return',
],
'no_trailing_comma_in_singleline_function_call' => [
'no_spaces_inside_parenthesis',
],
'no_unneeded_control_parentheses' => [
'no_trailing_whitespace',
],
Expand Down
55 changes: 52 additions & 3 deletions tests/Fixer/FunctionNotation/FunctionDeclarationFixerTest.php
Expand Up @@ -434,13 +434,39 @@ public function testFixPhp80(string $expected, ?string $input = null, array $con
public function provideFixPhp80Cases(): \Generator
{
yield [
'<?php function ($i,) {};',
'<?php function ($i) {};',
'<?php function( $i, ) {};',
];

yield [
'<?php function (
$a,
$b,
$c,
) {};',
'<?php function(
$a,
$b,
$c,
) {};',
];

yield [
'<?php function foo(
$a,
$b,
$c,
) {}',
'<?php function foo (
$a,
$b,
$c,
){}',
];

yield [
'<?php
$b = static function ($a,$b,) {
$b = static function ($a,$b) {
echo $a;
};
',
Expand All @@ -452,9 +478,32 @@ public function provideFixPhp80Cases(): \Generator
];

yield [
'<?php fn&($a,$b,) => null;',
'<?php fn&($a,$b) => null;',
'<?php fn &( $a,$b, ) => null;',
self::$configurationClosureSpacingNone,
];

yield [
'<?php
function ($a) use ($b) {};
function ($y) use (
$b,
$c,
) {};
',
'<?php
function ($a) use ($b , ) {};
function ($y) use (
$b,
$c,
) {};
',
];

yield [
'<?php function ($i,) {};',
'<?php function( $i, ) {};',
['trailing_comma_single_line' => true],
];
}
}