PhpdocTypesOrderFixer - Improve nested types support

src/DocBlock/TypeExpression.php

@@ -17,6 +17,7 @@ namespace PhpCsFixer\DocBlock;
 use PhpCsFixer\Preg;
 use PhpCsFixer\Tokenizer\Analyzer\Analysis\NamespaceAnalysis;
 use PhpCsFixer\Tokenizer\Analyzer\Analysis\NamespaceUseAnalysis;
+use PhpCsFixer\Utils;
 
 /**
  * @internal
@@ -31,39 +32,47 @@ final class TypeExpression
     public const REGEX_TYPES = '
     (?<types> # several types separated by `|` or `&`
         (?<type> # single type
-            \?? # optionally nullable
+            (?<nullable>\??)
             (?:
                 (?<object_like_array>
-                    array\h*\{
-                        (?<object_like_array_key>
-                            \h*[^?:\h]+\h*\??\h*:\h*(?&types)
+                    (?<object_like_array_start>array\h*\{)
+                        (?<object_like_array_keys>
+                            (?<object_like_array_key>
+                                \h*[^?:\h]+\h*\??\h*:\h*(?&types)
+                            )
+                            (?:\h*,(?&object_like_array_key))*
                         )
-                        (?:\h*,(?&object_like_array_key))*
                     \h*\}
                 )
                 |
                 (?<callable> # callable syntax, e.g. `callable(string): bool`
-                    (?:callable|Closure)\h*\(\h*
-                        (?&types)
-                        (?:
-                            \h*,\h*
+                    (?<callable_start>(?:callable|Closure)\h*\(\h*)
+                        (?<callable_arguments>
                             (?&types)
-                        )*
+                            (?:
+                                \h*,\h*
+                                (?&types)
+                            )*
+                        )?
                     \h*\)
                     (?:
                         \h*\:\h*
-                        (?&types)
+                        (?<callable_return>(?&types))
                     )?
                 )
                 |
                 (?<generic> # generic syntax, e.g.: `array<int, \Foo\Bar>`
-                    (?&name)+
-                    \h*<\h*
-                        (?&types)
-                        (?:
-                            \h*,\h*
+                    (?<generic_start>
+                        (?&name)+
+                        \h*<\h*
+                    )
+                        (?<generic_types>
                             (?&types)
-                        )*
+                            (?:
+                                \h*,\h*
+                                (?&types)
+                            )*
+                        )
                     \h*>
                 )
                 |
@@ -97,9 +106,19 @@ final class TypeExpression
     ';
 
     /**
-     * @var string[]
+     * @var string
+     */
+    private $value;
+
+    /**
+     * @var bool
+     */
+    private $isUnionType = false;
+
+    /**
+     * @var list<array{start_index: int, expression: self}>
      */
-    private $types = [];
+    private $innerTypeExpressions = [];
 
     /**
      * @var null|NamespaceAnalysis
@@ -116,23 +135,16 @@ final class TypeExpression
      */
     public function __construct(string $value, ?NamespaceAnalysis $namespace, array $namespaceUses)
     {
-        while ('' !== $value) {
-            Preg::match(
-                '{^'.self::REGEX_TYPES.'$}x',
-                $value,
-                $matches
-            );
-
-            $this->types[] = $matches['type'];
-            $value = Preg::replace(
-                '/^'.preg_quote($matches['type'], '/').'(\h*[|&]\h*)?/',
-                '',
-                $value
-            );
-        }
-
+        $this->value = $value;
         $this->namespace = $namespace;
         $this->namespaceUses = $namespaceUses;
+
+        $this->parse();
+    }
+
+    public function toString(): string
+    {
+        return $this->value;
     }
 
     /**
@@ -140,7 +152,46 @@ final class TypeExpression
      */
     public function getTypes(): array
     {
-        return $this->types;
+        if ($this->isUnionType) {
+            return array_map(
+                static function (array $type) { return $type['expression']->toString(); },
+                $this->innerTypeExpressions
+            );
+        }
+
+        return [$this->value];
+    }
+
+    /**
+     * @param callable(self $a, self $b): int $compareCallback
+     */
+    public function sortUnionTypes(callable $compareCallback): void
+    {
+        foreach (array_reverse($this->innerTypeExpressions) as [
+            'start_index' => $startIndex,
+            'expression' => $inner,
+        ]) {
+            $initialValueLength = \strlen($inner->toString());
+
+            $inner->sortUnionTypes($compareCallback);
+
+            $this->value = substr_replace(
+                $this->value,
+                $inner->toString(),
+                $startIndex,
+                $initialValueLength
+            );
+        }
+
+        if ($this->isUnionType) {
+            $this->innerTypeExpressions = Utils::stableSort(
+                $this->innerTypeExpressions,
+                static function (array $type): self { return $type['expression']; },
+                $compareCallback
+            );
+
+            $this->value = implode('|', $this->getTypes());
+        }
     }
 
     public function getCommonType(): ?string
@@ -149,7 +200,7 @@ final class TypeExpression
 
         $mainType = null;
 
-        foreach ($this->types as $type) {
+        foreach ($this->getTypes() as $type) {
             if ('null' === $type) {
                 continue;
             }
@@ -180,7 +231,7 @@ final class TypeExpression
 
     public function allowsNull(): bool
     {
-        foreach ($this->types as $type) {
+        foreach ($this->getTypes() as $type) {
             if (\in_array($type, ['null', 'mixed'], true)) {
                 return true;
             }
@@ -189,6 +240,143 @@ final class TypeExpression
         return false;
     }
 
+    private function parse(): void
+    {
+        $value = $this->value;
+
+        Preg::match(
+            '{^'.self::REGEX_TYPES.'$}x',
+            $value,
+            $matches
+        );
+
+        if ([] === $matches) {
+            return;
+        }
+
+        $index = '' !== $matches['nullable'] ? 1 : 0;
+
+        if ($matches['type'] !== $matches['types']) {
+            $this->isUnionType = true;
+
+            while (true) {
+                $innerType = $matches['type'];
+
+                $newValue = Preg::replace(
+                    '/^'.preg_quote($innerType, '/').'(\h*[|&]\h*)?/',
+                    '',
+                    $value
+                );
+
+                $this->innerTypeExpressions[] = [
+                    'start_index' => $index,
+                    'expression' => $this->inner($innerType),
+                ];
+
+                if ('' === $newValue) {
+                    return;
+                }
+
+                $index += \strlen($value) - \strlen($newValue);
+                $value = $newValue;
+
+                Preg::match(
+                    '{^'.self::REGEX_TYPES.'$}x',
+                    $value,
+                    $matches
+                );
+            }
+        }
+
+        if ('' !== ($matches['generic'] ?? '')) {
+            $this->parseCommaSeparatedInnerTypes(
+                $index + \strlen($matches['generic_start']),
+                $matches['generic_types']
+            );
+
+            return;
+        }
+
+        if ('' !== ($matches['callable'] ?? '')) {
+            $this->parseCommaSeparatedInnerTypes(
+                $index + \strlen($matches['callable_start']),
+                $matches['callable_arguments']
+            );
+
+            $return = $matches['callable_return'] ?? null;
+            if (null !== $return) {
+                $this->innerTypeExpressions[] = [
+                    'start_index' => \strlen($this->value) - \strlen($matches['callable_return']),
+                    'expression' => $this->inner($matches['callable_return']),
+                ];
+            }
+
+            return;
+        }
+
+        if ('' !== ($matches['object_like_array'] ?? '')) {
+            $this->parseObjectLikeArrayKeys(
+                $index + \strlen($matches['object_like_array_start']),
+                $matches['object_like_array_keys']
+            );
+        }
+    }
+
+    private function parseCommaSeparatedInnerTypes(int $startIndex, string $value): void
+    {
+        while ('' !== $value) {
+            Preg::match(
+                '{^'.self::REGEX_TYPES.'\h*(?:,|$)}x',
+                $value,
+                $matches
+            );
+
+            $this->innerTypeExpressions[] = [
+                'start_index' => $startIndex,
+                'expression' => $this->inner($matches['types']),
+            ];
+
+            $newValue = Preg::replace(
+                '/^'.preg_quote($matches['types'], '/').'(\h*\,\h*)?/',
+                '',
+                $value
+            );
+
+            $startIndex += \strlen($value) - \strlen($newValue);
+            $value = $newValue;
+        }
+    }
+
+    private function parseObjectLikeArrayKeys(int $startIndex, string $value): void
+    {
+        while ('' !== $value) {
+            Preg::match(
+                '{(?<_start>^.+?:\h*)'.self::REGEX_TYPES.'\h*(?:,|$)}x',
+                $value,
+                $matches
+            );
+
+            $this->innerTypeExpressions[] = [
+                'start_index' => $startIndex + \strlen($matches['_start']),
+                'expression' => $this->inner($matches['types']),
+            ];
+
+            $newValue = Preg::replace(
+                '/^.+?:\h*'.preg_quote($matches['types'], '/').'(\h*\,\h*)?/',
+                '',
+                $value
+            );
+
+            $startIndex += \strlen($value) - \strlen($newValue);
+            $value = $newValue;
+        }
+    }
+
+    private function inner(string $value): self
+    {
+        return new self($value, $this->namespace, $this->namespaceUses);
+    }
+
     private function getParentType(string $type1, string $type2): ?string
     {
         $types = [

src/Fixer/Phpdoc/PhpdocTypesOrderFixer.php

@@ -17,6 +17,7 @@ namespace PhpCsFixer\Fixer\Phpdoc;
 use PhpCsFixer\AbstractFixer;
 use PhpCsFixer\DocBlock\Annotation;
 use PhpCsFixer\DocBlock\DocBlock;
+use PhpCsFixer\DocBlock\TypeExpression;
 use PhpCsFixer\Fixer\ConfigurableFixerInterface;
 use PhpCsFixer\FixerConfiguration\FixerConfigurationResolver;
 use PhpCsFixer\FixerConfiguration\FixerConfigurationResolverInterface;
@@ -27,7 +28,6 @@ use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
 use PhpCsFixer\Preg;
 use PhpCsFixer\Tokenizer\Token;
 use PhpCsFixer\Tokenizer\Tokens;
-use PhpCsFixer\Utils;
 
 final class PhpdocTypesOrderFixer extends AbstractFixer implements ConfigurableFixerInterface
 {
@@ -139,10 +139,12 @@ final class PhpdocTypesOrderFixer extends AbstractFixer implements ConfigurableF
             }
 
             foreach ($annotations as $annotation) {
-                $types = $annotation->getTypes();
-
                 // fix main types
-                $annotation->setTypes($this->sortTypes($types));
+                $annotation->setTypes(
+                    $this->sortTypes(
+                        $annotation->getTypeExpression()
+                    )
+                );
 
                 // fix @method parameters types
                 $line = $doc->getLine($annotation->getStart());
@@ -160,63 +162,45 @@ final class PhpdocTypesOrderFixer extends AbstractFixer implements ConfigurableF
     }
 
     /**
-     * @param string[] $types
-     *
      * @return string[]
      */
-    private function sortTypes(array $types): array
+    private function sortTypes(TypeExpression $typeExpression): array
     {
-        foreach ($types as $index => $type) {
-            $types[$index] = Preg::replaceCallback('/^([^<]+)<(?:([\w\|]+?|<?.*>)(,\s*))?(.*)>$/', function (array $matches) {
-                return $matches[1].'<'.$this->sortJoinedTypes($matches[2]).$matches[3].$this->sortJoinedTypes($matches[4]).'>';
-            }, $type);
-        }
-
-        if ('alpha' === $this->configuration['sort_algorithm']) {
-            $types = Utils::stableSort(
-                $types,
-                static function (string $type): string { return $type; },
-                static function (string $typeA, string $typeB): int {
-                    $regexp = '/^\\??\\\?/';
-
-                    return strcasecmp(
-                        Preg::replace($regexp, '', $typeA),
-                        Preg::replace($regexp, '', $typeB)
-                    );
+        $normalizeType = static function (string $type): string {
+            return Preg::replace('/^\\??\\\?/', '', $type);
+        };
+
+        $typeExpression->sortUnionTypes(
+            function (TypeExpression $a, TypeExpression $b) use ($normalizeType): int {
+                $a = $normalizeType($a->toString());
+                $b = $normalizeType($b->toString());
+                $lowerCaseA = strtolower($a);
+                $lowerCaseB = strtolower($b);
+
+                if ('none' !== $this->configuration['null_adjustment']) {
+                    if ('null' === $lowerCaseA && 'null' !== $lowerCaseB) {
+                        return 'always_last' === $this->configuration['null_adjustment'] ? 1 : -1;
+                    }
+                    if ('null' !== $lowerCaseA && 'null' === $lowerCaseB) {
+                        return 'always_last' === $this->configuration['null_adjustment'] ? -1 : 1;
+                    }
                 }
-            );
-        }
 
-        if ('none' !== $this->configuration['null_adjustment']) {
-            $nulls = [];
-            foreach ($types as $index => $type) {
-                if (Preg::match('/^\\\?null$/i', $type)) {
-                    $nulls[$index] = $type;
-                    unset($types[$index]);
+                if ('alpha' === $this->configuration['sort_algorithm']) {
+                    return strcasecmp($a, $b);
                 }
-            }
 
-            if (\count($nulls) > 0) {
-                if ('always_last' === $this->configuration['null_adjustment']) {
-                    array_push($types, ...$nulls);
-                } else {
-                    array_unshift($types, ...$nulls);
-                }
+                return 0;
             }
-        }
+        );
 
-        return $types;
+        return $typeExpression->getTypes();
     }
 
     private function sortJoinedTypes(string $types): string
     {
-        $types = array_filter(
-            Preg::split('/([^|<{\(]+(?:[<{].*[>}]|\(.+\)(?::.+)?)?)/', $types, -1, PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY),
-            static function (string $value): bool {
-                return '|' !== $value;
-            }
-        );
+        $typeExpression = new TypeExpression($types, null, []);
 
-        return implode('|', $this->sortTypes($types));
+        return implode('|', $this->sortTypes($typeExpression));
     }
 }

tests/DocBlock/TypeExpressionTest.php

@@ -179,4 +179,90 @@ final class TypeExpressionTest extends TestCase
         yield ['bool', false];
         yield ['string', false];
     }
+
+    /**
+     * @dataProvider provideSortUnionTypesCases
+     */
+    public function testSortUnionTypes(string $typesExpression, string $expectResult): void
+    {
+        $expression = new TypeExpression($typesExpression, null, []);
+
+        $expression->sortUnionTypes(static function (TypeExpression $a, TypeExpression $b): int {
+            return strcasecmp($a->toString(), $b->toString());
+        });
+
+        static::assertSame($expectResult, $expression->toString());
+    }
+
+    public function provideSortUnionTypesCases(): iterable
+    {
+        yield 'not a union type' => [
+            'int',
+            'int',
+        ];
+        yield 'simple' => [
+            'int|bool',
+            'bool|int',
+        ];
+        yield 'simple in generic' => [
+            'array<int|bool>',
+            'array<bool|int>',
+        ];
+        yield 'generic with multiple types' => [
+            'array<int|bool, string|float>',
+            'array<bool|int, float|string>',
+        ];
+        yield 'simple in array shape with int key' => [
+            'array{0: int|bool}',
+            'array{0: bool|int}',
+        ];
+        yield 'simple in array shape with string key' => [
+            'array{"foo": int|bool}',
+            'array{"foo": bool|int}',
+        ];
+        yield 'simple in array shape with multiple keys' => [
+            'array{0: int|bool, "foo": int|bool}',
+            'array{0: bool|int, "foo": bool|int}',
+        ];
+        yield 'simple in callable argument' => [
+            'callable(int|bool)',
+            'callable(bool|int)',
+        ];
+        yield 'callable with multiple arguments' => [
+            'callable(int|bool, null|array)',
+            'callable(bool|int, array|null)',
+        ];
+        yield 'simple in callable return type' => [
+            'callable(): string|float',
+            'callable(): float|string',
+        ];
+        yield 'simple in closure argument' => [
+            'Closure(int|bool)',
+            'Closure(bool|int)',
+        ];
+        yield 'closure with multiple arguments' => [
+            'Closure(int|bool, null|array)',
+            'Closure(bool|int, array|null)',
+        ];
+        yield 'simple in closure return type' => [
+            'Closure(): string|float',
+            'Closure(): float|string',
+        ];
+        yield 'with multiple nesting levels' => [
+            'array{0: Foo<int|bool>|Bar<callable(string|float|array<int|bool>): Foo|Bar>}',
+            'array{0: Bar<callable(array<bool|int>|float|string): Bar|Foo>|Foo<bool|int>}',
+        ];
+        yield 'nullable generic' => [
+            '?array<Foo|Bar>',
+            '?array<Bar|Foo>',
+        ];
+        yield 'nullable callable' => [
+            '?callable(Foo|Bar): Foo|Bar',
+            '?callable(Bar|Foo): Bar|Foo',
+        ];
+        yield 'nullable array shape' => [
+            '?array{0: Foo|Bar}',
+            '?array{0: Bar|Foo}',
+        ];
+    }
 }

tests/Fixer/Phpdoc/PhpdocTypesOrderFixerTest.php

@@ -162,6 +162,22 @@ final class PhpdocTypesOrderFixerTest extends AbstractFixerTestCase
             [
                 '<?php /** @param null|callable(array<string>): array<string, T> $callback */',
             ],
+            [
+                '<?php /** @return array<int, callable(array<string, null|string> , DateTime): bool> */',
+            ],
+            [
+                '<?php /** @return Closure(Iterator<TKey, T>): Generator<int, array<TKey, T>> */',
+            ],
+            [
+                '<?php /** @var Closure(Iterator<TKey, T>): Generator<int, array<TKey, T>> $pipe */',
+            ],
+            [
+                '<?php /** @return Generator<int, Promise<mixed>, mixed, Identity> */',
+            ],
+            [
+                '<?php /** @param null|callable(null|foo, null|bar): array<string, T> $callback */',
+                '<?php /** @param null|callable(foo|null, bar|null): array<string, T> $callback */',
+            ],
         ];
     }
 
@@ -278,6 +294,7 @@ final class PhpdocTypesOrderFixerTest extends AbstractFixerTestCase
                 '<?php /** @var Foo[]|null|Foo|Foo\Bar|Foo_Bar */',
             ],
             [
+                '<?php /** @return array<int, callable(array<string, string|null> , DateTime): bool> */',
                 '<?php /** @return array<int, callable(array<string, null|string> , DateTime): bool> */',
             ],
         ];
@@ -621,8 +638,12 @@ final class PhpdocTypesOrderFixerTest extends AbstractFixerTestCase
                 '<?php /** @var Foo[]|null|Foo|Foo\Bar|Foo_Bar */',
             ],
             [
+                '<?php /** @return array<int, callable(array<string, string|null> , DateTime): bool> */',
                 '<?php /** @return array<int, callable(array<string, null|string> , DateTime): bool> */',
             ],
+            [
+                '<?php /** @var ?Deferred<TestLocations> */',
+            ],
         ];
     }
 }