Repository: VincentLanglet/Twig-CS-Fixer (php)

Issue:
Trailing comma rule
Hello,

First of all, thank you for this project, it's very useful, we use it on many projects in our company!

### Rule(s) related to or rule(s) missing

Add a rule for the trailing comma character.

Since Twig has allowed the trailing comma (see https://github.com/twigphp/Twig/pull/3363), perhaps the default should be to keep it, but allow it to be deleted. What do you think?

### Expected behavior

```twig
{% set data = {
    title: 'Custom title'
} %}
```

### Actual behavior

```twig
{% set data = {
    title: 'Custom title',
} %}
```

Pull request description:
Introduce Trailing Comma MultiLine rule
Closes #238

Patch:
diff --git a/docs/configuration.md b/docs/configuration.md
index a62e795..475419e 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -11,6 +11,7 @@ By default, the twig-cs-fixer standard is enabled with the twig coding standard
 - `IncludeFunctionRule`: ensures that include function is used instead of function tag.
 - `IndentRule`: ensures that files are not indented with tabs.
 - `SingleQuoteRule`: ensures that strings use single quotes when possible.
+- `TrailingCommaMultiLineRule`: ensures that multi-line arrays, objects and argument lists have a trailing comma.
 - `TrailingCommaSingleLineRule`: ensures that single-line arrays, objects and argument lists do not have a trailing comma.
 - `TrailingSpaceRule`: ensures that files have no trailing spaces.
 
diff --git a/docs/rules.md b/docs/rules.md
index 94b5980..e2b1915 100644
--- a/docs/rules.md
+++ b/docs/rules.md
@@ -15,6 +15,7 @@
 - **OperatorSpacingRule**: ensures there is one space before and after an operator except for `..`.
 - **PunctuationSpacingRule**: ensures there is no space before and after a punctuation except for `:` and `,` (configurable).
 - **SingleQuoteRule**: ensures that strings use single quotes when possible (configurable).
+- **TrailingCommaMultiLineRule**: ensures that multi-line arrays, objects and argument lists have a trailing comma (configurable).
 - **TrailingCommaSingleLineRule**: ensures that single-line arrays, objects and argument lists do not have a trailing comma.
 - **TrailingSpaceRule**: ensures that files have no trailing spaces.
 
@@ -60,6 +61,7 @@ new TwigCsFixer\Rules\Whitespace\IndentRule(3);
 - IncludeFunctionRule
 - IndentRule
 - SingleQuoteRule
+- TrailingCommaMultiLineRule
 - TrailingCommaSingleLineRule
 - TrailingSpaceRule
 
diff --git a/src/Rules/Punctuation/TrailingCommaMultiLineRule.php b/src/Rules/Punctuation/TrailingCommaMultiLineRule.php
new file mode 100644
index 0000000..040afad
--- /dev/null
+++ b/src/Rules/Punctuation/TrailingCommaMultiLineRule.php
@@ -0,0 +1,65 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TwigCsFixer\Rules\Punctuation;
+
+use TwigCsFixer\Rules\AbstractFixableRule;
+use TwigCsFixer\Rules\ConfigurableRuleInterface;
+use TwigCsFixer\Token\Token;
+use Webmozart\Assert\Assert;
+
+/**
+ * Ensures that multi-line arrays, objects and argument lists have a trailing comma.
+ */
+final class TrailingCommaMultiLineRule extends AbstractFixableRule implements ConfigurableRuleInterface
+{
+    public function __construct(private bool $useTrailingComma = true)
+    {
+    }
+
+    public function getConfiguration(): array
+    {
+        return [
+            'comma' => $this->useTrailingComma,
+        ];
+    }
+
+    protected function process(int $tokenPosition, array $tokens): void
+    {
+        $token = $tokens[$tokenPosition];
+        if (!$this->isTokenMatching($token, Token::PUNCTUATION_TYPE, [')', '}', ']'])) {
+            return;
+        }
+
+        $previousPosition = $this->findPrevious(Token::EMPTY_TOKENS, $tokens, $tokenPosition - 1, true);
+        Assert::notFalse($previousPosition, 'A closer cannot be the first token.');
+
+        if ($tokens[$previousPosition]->getLine() === $token->getLine()) {
+            // The closer is on the same line as the last element.
+            return;
+        }
+
+        $isMatchingComma = $this->isTokenMatching($tokens[$previousPosition], Token::PUNCTUATION_TYPE, ',');
+        if ($this->useTrailingComma === $isMatchingComma) {
+            return;
+        }
+
+        $fixer = $this->addFixableError(
+            $this->useTrailingComma
+                ? 'Multi-line arrays, objects and parameters lists should have trailing comma.'
+                : 'Multi-line arrays, objects and parameters lists should not have trailing comma.',
+            $token
+        );
+
+        if (null === $fixer) {
+            return;
+        }
+
+        if ($this->useTrailingComma) {
+            $fixer->addContent($previousPosition, ',');
+        } else {
+            $fixer->replaceToken($previousPosition, '');
+        }
+    }
+}
diff --git a/src/Rules/Punctuation/TrailingCommaSingleLineRule.php b/src/Rules/Punctuation/TrailingCommaSingleLineRule.php
index 8d94f92..ef8c154 100644
--- a/src/Rules/Punctuation/TrailingCommaSingleLineRule.php
+++ b/src/Rules/Punctuation/TrailingCommaSingleLineRule.php
@@ -20,17 +20,14 @@ final class TrailingCommaSingleLineRule extends AbstractFixableRule
             return;
         }
 
-        $relatedToken = $token->getRelatedToken();
-        Assert::notNull($relatedToken, 'A closer must have a related token.');
+        $previousPosition = $this->findPrevious(Token::EMPTY_TOKENS, $tokens, $tokenPosition - 1, true);
+        Assert::notFalse($previousPosition, 'A closer cannot be the first token.');
 
-        if ($relatedToken->getLine() !== $token->getLine()) {
-            // Multiline.
+        if ($tokens[$previousPosition]->getLine() !== $token->getLine()) {
+            // The closer is on a different line than the last element.
             return;
         }
 
-        $previousPosition = $this->findPrevious(Token::EMPTY_TOKENS, $tokens, $tokenPosition - 1, true);
-        Assert::notFalse($previousPosition, 'A closer cannot be the first token.');
-
         if (!$this->isTokenMatching($tokens[$previousPosition], Token::PUNCTUATION_TYPE, ',')) {
             return;
         }
diff --git a/src/Standard/TwigCsFixer.php b/src/Standard/TwigCsFixer.php
index cd96e38..a4cc6b3 100644
--- a/src/Standard/TwigCsFixer.php
+++ b/src/Standard/TwigCsFixer.php
@@ -6,6 +6,7 @@ namespace TwigCsFixer\Standard;
 
 use TwigCsFixer\Rules\Delimiter\BlockNameSpacingRule;
 use TwigCsFixer\Rules\Function\IncludeFunctionRule;
+use TwigCsFixer\Rules\Punctuation\TrailingCommaMultiLineRule;
 use TwigCsFixer\Rules\Punctuation\TrailingCommaSingleLineRule;
 use TwigCsFixer\Rules\String\HashQuoteRule;
 use TwigCsFixer\Rules\String\SingleQuoteRule;
@@ -30,6 +31,7 @@ final class TwigCsFixer implements StandardInterface
             new IncludeFunctionRule(),
             new IndentRule(),
             new SingleQuoteRule(),
+            new TrailingCommaMultiLineRule(),
             new TrailingCommaSingleLineRule(),
             new TrailingSpaceRule(),
         ];


Test patch:
diff --git a/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.fixed.twig b/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.fixed.twig
new file mode 100644
index 0000000..b684482
--- /dev/null
+++ b/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.fixed.twig
@@ -0,0 +1,34 @@
+{{ [1,2] }}
+{{ [1,2,] }}
+{{ {a:1,b:2} }}
+{{ {a:1,b:2,} }}
+{{ foo(1,2) }}
+{{ foo(1,2,) }}
+
+{{ [
+    1,
+    2,
+] }}
+{{ [
+    1,
+    2,
+] }}
+{{ {
+    a:1,
+    b:2,
+} }}
+{{ {
+    a:1,
+    b:2,
+} }}
+{{ foo(
+    1,
+    2,
+) }}
+{{ foo(
+    1,
+    2,
+) }}
+
+{{ untouched_multiline(1,
+    2) }}
diff --git a/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.fixed2.twig b/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.fixed2.twig
new file mode 100644
index 0000000..0132822
--- /dev/null
+++ b/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.fixed2.twig
@@ -0,0 +1,34 @@
+{{ [1,2] }}
+{{ [1,2,] }}
+{{ {a:1,b:2} }}
+{{ {a:1,b:2,} }}
+{{ foo(1,2) }}
+{{ foo(1,2,) }}
+
+{{ [
+    1,
+    2
+] }}
+{{ [
+    1,
+    2
+] }}
+{{ {
+    a:1,
+    b:2
+} }}
+{{ {
+    a:1,
+    b:2
+} }}
+{{ foo(
+    1,
+    2
+) }}
+{{ foo(
+    1,
+    2
+) }}
+
+{{ untouched_multiline(1,
+    2) }}
diff --git a/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.php b/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.php
new file mode 100644
index 0000000..129a706
--- /dev/null
+++ b/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.php
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TwigCsFixer\Tests\Rules\Punctuation\TrailingCommaMultiLine;
+
+use TwigCsFixer\Rules\Punctuation\TrailingCommaMultiLineRule;
+use TwigCsFixer\Tests\Rules\AbstractRuleTestCase;
+
+final class TrailingCommaMultiLineRuleTest extends AbstractRuleTestCase
+{
+    public function testConfiguration(): void
+    {
+        static::assertSame(
+            ['comma' => true],
+            (new TrailingCommaMultiLineRule())->getConfiguration()
+        );
+        static::assertSame(
+            ['comma' => false],
+            (new TrailingCommaMultiLineRule(false))->getConfiguration()
+        );
+    }
+
+    public function testRule(): void
+    {
+        $this->checkRule(new TrailingCommaMultiLineRule(), [
+            'TrailingCommaMultiLine.Error:11:1' => 'Multi-line arrays, objects and parameters lists should have trailing comma.',
+            'TrailingCommaMultiLine.Error:19:1' => 'Multi-line arrays, objects and parameters lists should have trailing comma.',
+            'TrailingCommaMultiLine.Error:27:1' => 'Multi-line arrays, objects and parameters lists should have trailing comma.',
+        ]);
+    }
+
+    public function testRuleWithoutTrailingComma(): void
+    {
+        $this->checkRule(new TrailingCommaMultiLineRule(false), [
+            'TrailingCommaMultiLine.Error:15:1' => 'Multi-line arrays, objects and parameters lists should not have trailing comma.',
+            'TrailingCommaMultiLine.Error:23:1' => 'Multi-line arrays, objects and parameters lists should not have trailing comma.',
+            'TrailingCommaMultiLine.Error:31:1' => 'Multi-line arrays, objects and parameters lists should not have trailing comma.',
+        ], fixedFilePath: __DIR__.'/TrailingCommaMultiLineRuleTest.fixed2.twig');
+    }
+}
diff --git a/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.twig b/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.twig
new file mode 100644
index 0000000..24ebc87
--- /dev/null
+++ b/tests/Rules/Punctuation/TrailingCommaMultiLine/TrailingCommaMultiLineRuleTest.twig
@@ -0,0 +1,34 @@
+{{ [1,2] }}
+{{ [1,2,] }}
+{{ {a:1,b:2} }}
+{{ {a:1,b:2,} }}
+{{ foo(1,2) }}
+{{ foo(1,2,) }}
+
+{{ [
+    1,
+    2
+] }}
+{{ [
+    1,
+    2,
+] }}
+{{ {
+    a:1,
+    b:2
+} }}
+{{ {
+    a:1,
+    b:2,
+} }}
+{{ foo(
+    1,
+    2
+) }}
+{{ foo(
+    1,
+    2,
+) }}
+
+{{ untouched_multiline(1,
+    2) }}
diff --git a/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.fixed.twig b/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.fixed.twig
index 38f1c16..4e5fb5d 100644
--- a/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.fixed.twig
+++ b/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.fixed.twig
@@ -29,3 +29,6 @@
     1,
     2,
 ) }}
+
+{{ changed_multiline(1,
+    2) }}
diff --git a/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.php b/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.php
index 0636283..42295b7 100644
--- a/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.php
+++ b/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.php
@@ -15,6 +15,7 @@ final class TrailingCommaSingleLineRuleTest extends AbstractRuleTestCase
             'TrailingCommaSingleLine.Error:2:9' => 'Single-line arrays, objects and parameters lists should not have trailing comma.',
             'TrailingCommaSingleLine.Error:4:13' => 'Single-line arrays, objects and parameters lists should not have trailing comma.',
             'TrailingCommaSingleLine.Error:6:12' => 'Single-line arrays, objects and parameters lists should not have trailing comma.',
+            'TrailingCommaSingleLine.Error:34:7' => 'Single-line arrays, objects and parameters lists should not have trailing comma.',
         ]);
     }
 }
diff --git a/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.twig b/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.twig
index f390b97..d27c07c 100644
--- a/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.twig
+++ b/tests/Rules/Punctuation/TrailingCommaSingleLine/TrailingCommaSingleLineRuleTest.twig
@@ -29,3 +29,6 @@
     1,
     2,
 ) }}
+
+{{ changed_multiline(1,
+    2,) }}
diff --git a/tests/Standard/TwigCsFixerTest.php b/tests/Standard/TwigCsFixerTest.php
index 220c4f3..32e7f92 100644
--- a/tests/Standard/TwigCsFixerTest.php
+++ b/tests/Standard/TwigCsFixerTest.php
@@ -11,6 +11,7 @@ use TwigCsFixer\Rules\Function\IncludeFunctionRule;
 use TwigCsFixer\Rules\Operator\OperatorNameSpacingRule;
 use TwigCsFixer\Rules\Operator\OperatorSpacingRule;
 use TwigCsFixer\Rules\Punctuation\PunctuationSpacingRule;
+use TwigCsFixer\Rules\Punctuation\TrailingCommaMultiLineRule;
 use TwigCsFixer\Rules\Punctuation\TrailingCommaSingleLineRule;
 use TwigCsFixer\Rules\String\HashQuoteRule;
 use TwigCsFixer\Rules\String\SingleQuoteRule;
@@ -40,6 +41,7 @@ final class TwigCsFixerTest extends TestCase
             new IncludeFunctionRule(),
             new IndentRule(),
             new SingleQuoteRule(),
+            new TrailingCommaMultiLineRule(),
             new TrailingCommaSingleLineRule(),
             new TrailingSpaceRule(),
         ], $standard->getRules());