ErickSkrauch/align_multiline_parameters inserted a space between the …

…type and the param name in the wrong position
erickskrauch · Jan 7, 2024 · 0601ddc · 0601ddc
1 parent e4f93c2
commit 0601ddc
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Fixed
 - Bug #6: `ErickSkrauch/align_multiline_parameters` not working correctly with unions and intersections.
+- `ErickSkrauch/align_multiline_parameters` inserted a space between the type and the param name in the wrong position when there were no whitespace between them.
 
 ## [1.2.1] - 2023-11-16
 ### Fixed

diff --git a/src/FunctionNotation/AlignMultilineParametersFixer.php b/src/FunctionNotation/AlignMultilineParametersFixer.php
@@ -12,6 +12,7 @@
 use PhpCsFixer\FixerDefinition\CodeSample;
 use PhpCsFixer\FixerDefinition\FixerDefinition;
 use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
+use PhpCsFixer\Tokenizer\Analyzer\Analysis\TypeAnalysis;
 use PhpCsFixer\Tokenizer\Analyzer\FunctionsAnalyzer;
 use PhpCsFixer\Tokenizer\Analyzer\WhitespacesAnalyzer;
 use PhpCsFixer\Tokenizer\CT;
@@ -127,7 +128,7 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
                 $typeAnalysis = $argument->getTypeAnalysis();
                 if ($typeAnalysis) {
                     $hasAtLeastOneTypedArgument = true;
-                    $typeLength = $this->getFullTypeLength($tokens, $typeAnalysis->getStartIndex());
+                    $typeLength = $this->getFullTypeLength($tokens, $typeAnalysis);
                     if ($typeLength > $longestType) {
                         $longestType = $typeLength;
                     }
@@ -140,13 +141,31 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
             }
 
             $argsIndent = WhitespacesAnalyzer::detectIndent($tokens, $i) . $this->whitespacesConfig->getIndent();
-            foreach ($arguments as $argument) {
+            // Since we perform insertion of new tokens in this loop, if we go sequentially,
+            // at each new iteration the token indices will shift due to the addition of new whitespaces.
+            // If we go from the end, this problem will not occur.
+            foreach (array_reverse($arguments) as $argument) {
+                if ($this->configuration[self::C_DEFAULTS] !== null) {
+                    // Can't use $argument->hasDefault() because it's null when it's default for a type (e.g. 0 for int)
+                    $equalToken = $tokens[$tokens->getNextMeaningfulToken($argument->getNameIndex())];
+                    if ($equalToken->getContent() === '=') {
+                        $nameLen = mb_strlen($argument->getName());
+                        $whitespaceIndex = $argument->getNameIndex() + 1;
+                        if ($this->configuration[self::C_DEFAULTS] === true) {
+                            $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, str_repeat(' ', $longestVariableName - $nameLen + 1));
+                        } else {
+                            $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, ' ');
+                        }
+                    }
+                }
+
                 if ($this->configuration[self::C_VARIABLES] !== null) {
                     $whitespaceIndex = $argument->getNameIndex() - 1;
                     if ($this->configuration[self::C_VARIABLES] === true) {
                         $typeLen = 0;
-                        if ($argument->getTypeAnalysis() !== null) {
-                            $typeLen = $this->getFullTypeLength($tokens, $argument->getTypeAnalysis()->getStartIndex());
+                        $typeAnalysis = $argument->getTypeAnalysis();
+                        if ($typeAnalysis !== null) {
+                            $typeLen = $this->getFullTypeLength($tokens, $typeAnalysis);
                         }
 
                         $appendix = str_repeat(' ', $longestType - $typeLen + (int)$hasAtLeastOneTypedArgument);
@@ -163,22 +182,7 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
                         }
                     }
 
-                    $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, $whitespaceToken);
-                }
-
-                if ($this->configuration[self::C_DEFAULTS] !== null) {
-                    // Can't use $argument->hasDefault() because it's null when it's default for a type (e.g. 0 for int)
-                    /** @var \PhpCsFixer\Tokenizer\Token $equalToken */
-                    $equalToken = $tokens[$tokens->getNextMeaningfulToken($argument->getNameIndex())];
-                    if ($equalToken->getContent() === '=') {
-                        $nameLen = mb_strlen($argument->getName());
-                        $whitespaceIndex = $argument->getNameIndex() + 1;
-                        if ($this->configuration[self::C_DEFAULTS] === true) {
-                            $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, str_repeat(' ', $longestVariableName - $nameLen + 1));
-                        } else {
-                            $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 0, ' ');
-                        }
-                    }
+                    $tokens->ensureWhitespaceAtIndex($whitespaceIndex, 1, $whitespaceToken);
                 }
             }
         }
@@ -187,25 +191,22 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
     /**
      * TODO: The declaration might be split across multiple lines.
      *       In such case we need to find the longest line and return it as the full type length
-     *
-     * @param int $typeIndex points to the beginning of the type
      */
-    private function getFullTypeLength(Tokens $tokens, int $typeIndex): int {
+    private function getFullTypeLength(Tokens $tokens, TypeAnalysis $typeAnalysis): int {
         $typeLength = 0;
-        $varNameTokenIndex = $tokens->getNextTokenOfKind($typeIndex, [[T_VARIABLE]]);
-        for ($i = $typeIndex; $i < $varNameTokenIndex - 1; $i++) { // -1 to avoid whitespace between param name and type
+        for ($i = $typeAnalysis->getStartIndex(); $i <= $typeAnalysis->getEndIndex(); $i++) {
             $typeLength += mb_strlen($tokens[$i]->getContent());
         }
 
-        $possiblyReadonlyToken = $tokens[$typeIndex - 2];
+        $possiblyReadonlyToken = $tokens[$typeAnalysis->getStartIndex() - 2];
         if ($possiblyReadonlyToken->isGivenKind($this->parameterModifiers)) {
-            $whitespaceToken = $tokens[$typeIndex - 1];
+            $whitespaceToken = $tokens[$typeAnalysis->getStartIndex() - 1];
             $typeLength += strlen($possiblyReadonlyToken->getContent() . $whitespaceToken->getContent());
         }
 
-        $possiblyPromotionToken = $tokens[$typeIndex - 4];
+        $possiblyPromotionToken = $tokens[$typeAnalysis->getStartIndex() - 4];
         if ($possiblyPromotionToken->isGivenKind($this->parameterModifiers)) {
-            $whitespaceToken = $tokens[$typeIndex - 3];
+            $whitespaceToken = $tokens[$typeAnalysis->getStartIndex() - 3];
             $typeLength += strlen($possiblyPromotionToken->getContent() . $whitespaceToken->getContent());
         }
 

diff --git a/tests/FunctionNotation/AlignMultilineParametersFixerTest.php b/tests/FunctionNotation/AlignMultilineParametersFixerTest.php
@@ -253,6 +253,27 @@ public function provideNullCases(): iterable {
         }
     }
 
+    public function testNoWhitespace(): void {
+        $this->fixer->configure([
+            AlignMultilineParametersFixer::C_VARIABLES => true,
+            AlignMultilineParametersFixer::C_DEFAULTS => true,
+        ]);
+        $this->doTest(
+            '<?php
+            function test(
+                string $string = "string",
+                int    $int    = 0
+            ): void {}
+            ',
+            '<?php
+            function test(
+                string$string = "string",
+                int$int = 0
+            ): void {}
+            ',
+        );
+    }
+
     /**
      * @dataProvider provide80TrueCases
      * @requires PHP 8.0