Cover edge cases for ErickSkrauch/align_multiline_parameters in align…

…ed mode
erickskrauch · Jan 15, 2024 · d6fd445 · d6fd445
1 parent e29d67a
commit d6fd445
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 13 deletions.
diff --git a/src/FunctionNotation/AlignMultilineParametersFixer.php b/src/FunctionNotation/AlignMultilineParametersFixer.php
@@ -104,6 +104,12 @@ protected function createConfigurationDefinition(): FixerConfigurationResolverIn
         ]);
     }
 
+    /**
+     * TODO: This fixer can be reimplemented using a simpler idea:
+     *       we need to find the position of the farthest token $ and =, and then align all other tokens to them.
+     *       The current implementation strongly relies on the fact, that the code will be well written and some
+     *       additional fixer like TypeDeclarationSpacesFixer will be used as well
+     */
     protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
         // There is nothing to do
         if ($this->configuration[self::C_VARIABLES] === null && $this->configuration[self::C_DEFAULTS] === null) {
@@ -133,8 +139,6 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
             $longestType = 0;
             $longestVariableName = 0;
             $hasAtLeastOneTypedArgument = false;
-            /** @var bool|null $isVariadicArgTypeLong */
-            $isVariadicArgTypeLong = null;
             /** @var list<DeclarationAnalysis> $analysedArguments */
             $analysedArguments = [];
             foreach ($arguments as $argument) {
@@ -153,7 +157,16 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
                 }
 
                 if ($declarationAnalysis['isVariadic']) {
-                    $isVariadicArgTypeLong = $longestType === $declarationAnalysis['typeLength'];
+                    // We want to do the alignment on the $ symbol.
+                    // If none of the arguments has a type or the longest type is too short,
+                    // the variadic argument will not be able to take the correct position.
+                    // So we extend the type length so that all variables are aligned in a column by the $ symbol
+                    $longestTypeDelta = $longestType - $declarationAnalysis['typeLength'];
+                    if ($longestType === $declarationAnalysis['typeLength']) {
+                        $longestType += 3;
+                    } elseif ($longestTypeDelta < 3) { // Ensure there is enough space for "..."
+                        $longestType += 3 - $longestTypeDelta - (int)($declarationAnalysis['typeLength'] === 0);
+                    }
                 }
 
                 $analysedArguments[] = $declarationAnalysis;
@@ -186,16 +199,8 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
 
                     if ($this->configuration[self::C_VARIABLES] === true) {
                         $alignLength = $longestType - $argument['typeLength'] + (int)$hasAtLeastOneTypedArgument;
-                        if ($isVariadicArgTypeLong !== null) {
-                            if ($isVariadicArgTypeLong) {
-                                if (!$argument['isVariadic']) {
-                                    $alignLength += 3;
-                                }
-                            } else {
-                                if ($argument['isVariadic']) {
-                                    $alignLength -= 3;
-                                }
-                            }
+                        if ($argument['isVariadic']) {
+                            $alignLength -= 3; // 3 is the length of "..."
                         }
 
                         $appendix = str_repeat(' ', $alignLength);

diff --git a/tests/FunctionNotation/AlignMultilineParametersFixerTest.php b/tests/FunctionNotation/AlignMultilineParametersFixerTest.php
@@ -232,6 +232,21 @@ function test(
             ',
         ];
 
+        yield 'variadic parameter (short, edge case)' => [
+            '<?php
+            function test(
+                float  $float,
+                int ...$params
+            ): void {}
+            ',
+            '<?php
+            function test(
+                float $float,
+                int ...$params
+            ): void {}
+            ',
+        ];
+
         yield 'variadic parameter (long)' => [
             '<?php
             function test(
@@ -261,6 +276,40 @@ function test(
             ): void {}
             ',
         ];
+
+        yield 'variadic parameter (untyped, edge case 1)' => [
+            '<?php
+            interface A {}
+            function test(
+                A  $a,
+                ...$params
+            ): void {}
+            ',
+            '<?php
+            interface A {}
+            function test(
+                A $a,
+                ...$params
+            ): void {}
+            ',
+        ];
+
+        yield 'variadic parameter (untyped, edge case 2)' => [
+            '<?php
+            interface A {}
+            function test(
+                   $a,
+                ...$params
+            ): void {}
+            ',
+            '<?php
+            interface A {}
+            function test(
+                $a,
+                ...$params
+            ): void {}
+            ',
+        ];
     }
 
     /**