Fixes #5. Reimplement type length calculation to cover all possible c…

…ases with DNF types
erickskrauch · Jan 7, 2024 · 28ddf03 · 28ddf03
1 parent 6b0e22f
commit 28ddf03
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Fixed
+- Bug #5: `ErickSkrauch/align_multiline_parameters` not working correctly with unions and intersections.
 
 ## [1.2.1] - 2023-11-16
 ### Fixed

diff --git a/src/FunctionNotation/AlignMultilineParametersFixer.php b/src/FunctionNotation/AlignMultilineParametersFixer.php
@@ -183,33 +183,27 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens): void {
         }
     }
 
-    private function getFullTypeLength(Tokens $tokens, ?int $typeIndex): int {
-        /** @var \PhpCsFixer\Tokenizer\Token $typeToken */
-        $typeToken = $tokens[$typeIndex];
-        $typeLength = strlen($typeToken->getContent());
-
-        if ($typeToken->isGivenKind(CT::T_NULLABLE_TYPE)) {
-            $possiblyWhitespace = $tokens[$typeIndex + 1];
-            if ($possiblyWhitespace->isWhitespace()) {
-                $typeLength += strlen($possiblyWhitespace->getContent());
-            }
-
-            $realTypeToken = $tokens[$tokens->getNextMeaningfulToken($typeIndex)];
-            $typeLength += strlen($realTypeToken->getContent());
+    /**
+     * 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 {
+        $typeLength = 0;
+        $varNameTokenIndex = $tokens->getNextTokenOfKind($typeIndex, [[T_VARIABLE]]);
+        for ($i = $typeIndex; $i < $varNameTokenIndex - 1; $i++) { // -1 to avoid whitespace between param name and type
+            $typeLength += strlen($tokens[$i]->getContent());
         }
 
-        /** @var \PhpCsFixer\Tokenizer\Token $possiblyReadonlyToken */
         $possiblyReadonlyToken = $tokens[$typeIndex - 2];
         if ($possiblyReadonlyToken->isGivenKind($this->parameterModifiers)) {
-            /** @var \PhpCsFixer\Tokenizer\Token $whitespaceToken */
             $whitespaceToken = $tokens[$typeIndex - 1];
             $typeLength += strlen($possiblyReadonlyToken->getContent() . $whitespaceToken->getContent());
         }
 
-        /** @var \PhpCsFixer\Tokenizer\Token $possiblyPromotionToken */
         $possiblyPromotionToken = $tokens[$typeIndex - 4];
         if ($possiblyPromotionToken->isGivenKind($this->parameterModifiers)) {
-            /** @var \PhpCsFixer\Tokenizer\Token $whitespaceToken */
             $whitespaceToken = $tokens[$typeIndex - 3];
             $typeLength += strlen($possiblyPromotionToken->getContent() . $whitespaceToken->getContent());
         }

diff --git a/tests/FunctionNotation/AlignMultilineParametersFixerTest.php b/tests/FunctionNotation/AlignMultilineParametersFixerTest.php
@@ -344,6 +344,24 @@ public function __construct(
             }
             ',
         ];
+        yield 'constructor with union types, mixed whitespace' => [
+            '<?php
+            class Test {
+                public function __construct(
+                    string                               $string = "string",
+                    protected readonly int | string|null $int    = 0
+                ) {}
+            }
+            ',
+            '<?php
+            class Test {
+                public function __construct(
+                    string $string = "string",
+                    protected readonly int | string|null $int = 0
+                ) {}
+            }
+            ',
+        ];
     }
 
     /**
@@ -368,6 +386,61 @@ public function provideFalse81Cases(): iterable {
         }
     }
 
+    /**
+     * @dataProvider provide82TrueCases
+     * @requires PHP 8.2
+     */
+    public function test82BothTrue(string $expected, ?string $input = null): void {
+        $this->fixer->configure([
+            AlignMultilineParametersFixer::C_VARIABLES => true,
+            AlignMultilineParametersFixer::C_DEFAULTS => true,
+        ]);
+        $this->doTest($expected, $input);
+    }
+
+    public function provide82TrueCases(): iterable {
+        yield 'constructor with union types, mixed whitespace' => [
+            '<?php
+            class Test {
+                public function __construct(
+                    string                                                       $string = "string",
+                    protected readonly int | (JsonSerializable& Stringable)|null $int    = 0
+                ) {}
+            }
+            ',
+            '<?php
+            class Test {
+                public function __construct(
+                    string $string = "string",
+                    protected readonly int | (JsonSerializable& Stringable)|null $int = 0
+                ) {}
+            }
+            ',
+        ];
+    }
+
+    /**
+     * @dataProvider provideFalse82Cases
+     * @requires PHP 8.2
+     */
+    public function test82BothFalse(string $expected, ?string $input = null): void {
+        $this->fixer->configure([
+            AlignMultilineParametersFixer::C_VARIABLES => false,
+            AlignMultilineParametersFixer::C_DEFAULTS => false,
+        ]);
+        $this->doTest($expected, $input);
+    }
+
+    public function provideFalse82Cases(): iterable {
+        foreach ($this->provide82TrueCases() as $key => $case) {
+            if (isset($case[1])) {
+                yield $key => [$case[1], $case[0]];
+            } else {
+                yield $key => $case;
+            }
+        }
+    }
+
     protected function createFixer(): AbstractFixer {
         return new AlignMultilineParametersFixer();
     }