Browse Source

DX: Make `TypeExpression` API more explicit about composite types (#8214)

Greg Korba 5 months ago
parent
commit
4ab3b39d82

+ 1 - 1
dev-tools/phpstan/baseline.php

@@ -543,7 +543,7 @@ $ignoreErrors[] = [
 ];
 ];
 $ignoreErrors[] = [
 $ignoreErrors[] = [
 	// identifier: assign.propertyType
 	// identifier: assign.propertyType
-	'message' => '#^Property PhpCsFixer\\\\DocBlock\\\\TypeExpression\\:\\:\\$typesGlue \\(\'&\'\\|\'\\|\'\\) does not accept non\\-empty\\-string\\.$#',
+	'message' => '#^Property PhpCsFixer\\\\DocBlock\\\\TypeExpression\\:\\:\\$typesGlue \\(\'&\'\\|\'\\|\'\\|null\\) does not accept non\\-empty\\-string\\.$#',
 	'count' => 1,
 	'count' => 1,
 	'path' => __DIR__ . '/../../src/DocBlock/TypeExpression.php',
 	'path' => __DIR__ . '/../../src/DocBlock/TypeExpression.php',
 ];
 ];

+ 1 - 1
phpstan.dist.neon

@@ -54,7 +54,7 @@ parameters:
         -
         -
             message: '#^Method PhpCsFixer\\Tests\\.+::provide.+Cases\(\) return type has no value type specified in iterable type iterable\.$#'
             message: '#^Method PhpCsFixer\\Tests\\.+::provide.+Cases\(\) return type has no value type specified in iterable type iterable\.$#'
             path: tests
             path: tests
-            count: 266
+            count: 265
     tipsOfTheDay: false
     tipsOfTheDay: false
     tmpDir: dev-tools/phpstan/cache
     tmpDir: dev-tools/phpstan/cache
 
 

+ 1 - 1
src/AbstractPhpdocToTypeDeclarationFixer.php

@@ -230,7 +230,7 @@ abstract class AbstractPhpdocToTypeDeclarationFixer extends AbstractFixer implem
             return null;
             return null;
         }
         }
 
 
-        if (!$typesExpression->isUnionType() || '|' !== $typesExpression->getTypesGlue()) {
+        if (!$typesExpression->isUnionType()) {
             return null;
             return null;
         }
         }
 
 

+ 1 - 1
src/AbstractPhpdocTypesFixer.php

@@ -91,7 +91,7 @@ abstract class AbstractPhpdocTypesFixer extends AbstractFixer
         }
         }
 
 
         $newTypeExpression = $typeExpression->mapTypes(function (TypeExpression $type) {
         $newTypeExpression = $typeExpression->mapTypes(function (TypeExpression $type) {
-            if (!$type->isUnionType()) {
+            if (!$type->isCompositeType()) {
                 $value = $this->normalize($type->toString());
                 $value = $this->normalize($type->toString());
 
 
                 return new TypeExpression($value, null, []);
                 return new TypeExpression($value, null, []);

+ 7 - 1
src/DocBlock/Annotation.php

@@ -215,7 +215,13 @@ final class Annotation
     public function setTypes(array $types): void
     public function setTypes(array $types): void
     {
     {
         $origTypesContent = $this->getTypesContent();
         $origTypesContent = $this->getTypesContent();
-        $newTypesContent = implode($this->getTypeExpression()->getTypesGlue(), $types);
+        $newTypesContent = implode(
+            // Fallback to union type is provided for backward compatibility (previously glue was set to `|` by default even when type was not composite)
+            // @TODO Better handling for cases where type is fixed (original type is not composite, but was made composite during fix)
+            $this->getTypeExpression()->getTypesGlue() ?? '|',
+            $types
+        );
+
         if ($origTypesContent === $newTypesContent) {
         if ($origTypesContent === $newTypesContent) {
             return;
             return;
         }
         }

+ 23 - 11
src/DocBlock/TypeExpression.php

@@ -211,10 +211,10 @@ final class TypeExpression
 
 
     private string $value;
     private string $value;
 
 
-    private bool $isUnionType;
+    private bool $isCompositeType;
 
 
-    /** @var '&'|'|' */
-    private string $typesGlue;
+    /** @var null|'&'|'|' */
+    private ?string $typesGlue = null;
 
 
     /** @var list<array{start_index: int, expression: self}> */
     /** @var list<array{start_index: int, expression: self}> */
     private array $innerTypeExpressions = [];
     private array $innerTypeExpressions = [];
@@ -246,7 +246,7 @@ final class TypeExpression
      */
      */
     public function getTypes(): array
     public function getTypes(): array
     {
     {
-        if ($this->isUnionType) {
+        if ($this->isCompositeType) {
             return array_map(
             return array_map(
                 static fn (array $type) => $type['expression']->toString(),
                 static fn (array $type) => $type['expression']->toString(),
                 $this->innerTypeExpressions,
                 $this->innerTypeExpressions,
@@ -256,15 +256,28 @@ final class TypeExpression
         return [$this->value];
         return [$this->value];
     }
     }
 
 
+    /**
+     * Determines if type expression is a composite type (union or intersection).
+     */
+    public function isCompositeType(): bool
+    {
+        return $this->isCompositeType;
+    }
+
     public function isUnionType(): bool
     public function isUnionType(): bool
     {
     {
-        return $this->isUnionType;
+        return $this->isCompositeType && '|' === $this->typesGlue;
+    }
+
+    public function isIntersectionType(): bool
+    {
+        return $this->isCompositeType && '&' === $this->typesGlue;
     }
     }
 
 
     /**
     /**
-     * @return '&'|'|'
+     * @return null|'&'|'|'
      */
      */
-    public function getTypesGlue(): string
+    public function getTypesGlue(): ?string
     {
     {
         return $this->typesGlue;
         return $this->typesGlue;
     }
     }
@@ -324,7 +337,7 @@ final class TypeExpression
     public function sortTypes(\Closure $compareCallback): self
     public function sortTypes(\Closure $compareCallback): self
     {
     {
         return $this->mapTypes(function (self $type) use ($compareCallback): self {
         return $this->mapTypes(function (self $type) use ($compareCallback): self {
-            if ($type->isUnionType) {
+            if ($type->isCompositeType) {
                 $innerTypeExpressions = Utils::stableSort(
                 $innerTypeExpressions = Utils::stableSort(
                     $type->innerTypeExpressions,
                     $type->innerTypeExpressions,
                     static fn (array $v): self => $v['expression'],
                     static fn (array $v): self => $v['expression'],
@@ -445,7 +458,7 @@ final class TypeExpression
                 $seenGlues = array_filter($seenGlues);
                 $seenGlues = array_filter($seenGlues);
                 \assert([] !== $seenGlues);
                 \assert([] !== $seenGlues);
 
 
-                $this->isUnionType = true;
+                $this->isCompositeType = true;
                 $this->typesGlue = array_key_first($seenGlues);
                 $this->typesGlue = array_key_first($seenGlues);
 
 
                 if (1 === \count($seenGlues)) {
                 if (1 === \count($seenGlues)) {
@@ -482,8 +495,7 @@ final class TypeExpression
             }
             }
         }
         }
 
 
-        $this->isUnionType = false;
-        $this->typesGlue = '|';
+        $this->isCompositeType = false;
 
 
         if ('' !== $matches['nullable'][0]) {
         if ('' !== $matches['nullable'][0]) {
             $this->innerTypeExpressions[] = [
             $this->innerTypeExpressions[] = [

+ 60 - 6
tests/DocBlock/TypeExpressionTest.php

@@ -47,7 +47,7 @@ final class TypeExpressionTest extends TestCase
             null,
             null,
             []
             []
         );
         );
-        if (!$expression->isUnionType() || '|' === $expression->getTypesGlue()) {
+        if (!$expression->isCompositeType() || $expression->isUnionType()) {
             self::assertSame(
             self::assertSame(
                 [$unionTestNs.'\A', ...$expectedTypes, $unionTestNs.'\Z'],
                 [$unionTestNs.'\A', ...$expectedTypes, $unionTestNs.'\Z'],
                 [...$unionExpression->getTypes()]
                 [...$unionExpression->getTypes()]
@@ -55,6 +55,9 @@ final class TypeExpressionTest extends TestCase
         }
         }
     }
     }
 
 
+    /**
+     * @return iterable<int|string, array{0: string, 1?: null|list<string>}>
+     */
     public static function provideGetTypesCases(): iterable
     public static function provideGetTypesCases(): iterable
     {
     {
         yield ['int'];
         yield ['int'];
@@ -451,30 +454,57 @@ final class TypeExpressionTest extends TestCase
     /**
     /**
      * @dataProvider provideGetTypesGlueCases
      * @dataProvider provideGetTypesGlueCases
      */
      */
-    public function testGetTypesGlue(string $expectedTypesGlue, string $typesExpression): void
+    public function testGetTypesGlue(?string $expectedTypesGlue, string $typesExpression): void
     {
     {
         $expression = new TypeExpression($typesExpression, null, []);
         $expression = new TypeExpression($typesExpression, null, []);
         self::assertSame($expectedTypesGlue, $expression->getTypesGlue());
         self::assertSame($expectedTypesGlue, $expression->getTypesGlue());
     }
     }
 
 
     /**
     /**
-     * @return iterable<array{0: '&'|'|', 1: string}>
+     * @return iterable<array{0: null|'&'|'|', 1: string}>
      */
      */
     public static function provideGetTypesGlueCases(): iterable
     public static function provideGetTypesGlueCases(): iterable
     {
     {
-        yield ['|', 'string']; // for backward behaviour
+        yield [null, 'string'];
 
 
         yield ['|', 'bool|string'];
         yield ['|', 'bool|string'];
 
 
         yield ['&', 'Foo&Bar'];
         yield ['&', 'Foo&Bar'];
     }
     }
 
 
+    /**
+     * @dataProvider provideIsCompositeTypeCases
+     */
+    public function testIsCompositeType(bool $expectedIsCompositeType, string $typeExpression): void
+    {
+        $expression = new TypeExpression($typeExpression, null, []);
+
+        self::assertSame($expectedIsCompositeType, $expression->isCompositeType());
+    }
+
+    /**
+     * @return iterable<array{0: bool, 1: string}>
+     */
+    public static function provideIsCompositeTypeCases(): iterable
+    {
+        yield [false, 'string'];
+
+        yield [false, 'iterable<Foo>'];
+
+        yield [true, 'iterable&stringable'];
+
+        yield [true, 'bool|string'];
+
+        yield [true, 'Foo|(Bar&Baz)'];
+    }
+
     /**
     /**
      * @dataProvider provideIsUnionTypeCases
      * @dataProvider provideIsUnionTypeCases
      */
      */
-    public function testIsUnionType(bool $expectedIsUnionType, string $typesExpression): void
+    public function testIsUnionType(bool $expectedIsUnionType, string $typeExpression): void
     {
     {
-        $expression = new TypeExpression($typesExpression, null, []);
+        $expression = new TypeExpression($typeExpression, null, []);
+
         self::assertSame($expectedIsUnionType, $expression->isUnionType());
         self::assertSame($expectedIsUnionType, $expression->isUnionType());
     }
     }
 
 
@@ -485,6 +515,8 @@ final class TypeExpressionTest extends TestCase
     {
     {
         yield [false, 'string'];
         yield [false, 'string'];
 
 
+        yield [false, 'iterable&stringable'];
+
         yield [true, 'bool|string'];
         yield [true, 'bool|string'];
 
 
         yield [true, 'int|string|null'];
         yield [true, 'int|string|null'];
@@ -496,10 +528,32 @@ final class TypeExpressionTest extends TestCase
         yield [false, '?int'];
         yield [false, '?int'];
 
 
         yield [true, 'Foo|Bar'];
         yield [true, 'Foo|Bar'];
+    }
+
+    /**
+     * @dataProvider provideIsIntersectionTypeCases
+     */
+    public function testIsIntersectionType(bool $expectedIsIntersectionType, string $typeExpression): void
+    {
+        $expression = new TypeExpression($typeExpression, null, []);
+
+        self::assertSame($expectedIsIntersectionType, $expression->isIntersectionType());
+    }
+
+    /**
+     * @return iterable<array{0: bool, 1: string}>
+     */
+    public static function provideIsIntersectionTypeCases(): iterable
+    {
+        yield [false, 'string'];
+
+        yield [false, 'string|int'];
 
 
         yield [true, 'Foo&Bar'];
         yield [true, 'Foo&Bar'];
 
 
         yield [true, 'Foo&Bar&?Baz'];
         yield [true, 'Foo&Bar&?Baz'];
+
+        yield [true, '\iterable&\Stringable'];
     }
     }
 
 
     /**
     /**