Skip to content

Commit

Permalink
Ensure that simple union types like A|null yield a nullable `Reflec…
Browse files Browse the repository at this point in the history
…tionNamedType`

Fixes #901

## Context

Currently, when running reflection on an `A|null` type, BetterReflection produces a `Roave\BetterReflection\Reflection\ReflectionUnionType`:

```php
var_dump(
    get_class(
        (new DefaultReflector(new StringSourceLocator(
            <<<'PHP'
<?php
interface A {}
final class AClass {
    private A|null $typed;
}
PHP
            ,
            (new BetterReflection())->astLocator()
        )))
            ->reflectClass('AClass')
            ->getProperty('typed')
            ->getType()
    )
);
```

produces

```
string(53) "Roave\BetterReflection\Reflection\ReflectionUnionType"
```

In PHP-SRC, this behavior is different: https://3v4l.org/gMA4T#v8.1rc3

```php
<?php

interface A {}
interface B {}

class Implementation
{
    function foo(A|null $param) { throw new Exception(); }
    function bar(A|B|null $param) { throw new Exception(); }
}

var_dump((new ReflectionParameter([Implementation::class, 'foo'], 0))->getType());
var_dump((new ReflectionParameter([Implementation::class, 'bar'], 0))->getType());
```

produces:

```
object(ReflectionNamedType)#2 (0) {
}
object(ReflectionUnionType)#1 (0) {
}
```

This means that a `UnionType` AST node composed of just `null` plus another type should be converted into
a `ReflectionNamedType`, for the sake of compatibility with upstream (this patch does that).

This is ugly, but will (for now) avoid some bad issues in downstream handling (presently blocking Roave/BackwardCompatibilityCheck#324 )
  • Loading branch information
Ocramius committed Dec 5, 2021
1 parent c6c9b85 commit c861f48
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 0 deletions.
13 changes: 13 additions & 0 deletions src/Reflection/ReflectionType.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
use PhpParser\Node\UnionType;
use Roave\BetterReflection\Reflector\Reflector;

use function array_filter;
use function array_values;
use function count;

abstract class ReflectionType
{
protected function __construct(
Expand Down Expand Up @@ -43,6 +47,15 @@ public static function createFromNode(
return new ReflectionIntersectionType($reflector, $owner, $type);
}

$nonNullTypes = array_values(array_filter(
$type->types,
static fn (Identifier|Name $type): bool => $type->toString() !== 'null',
));

if (count($nonNullTypes) === 1) {
return self::createFromNode($reflector, $owner, $nonNullTypes[0], true);
}

return new ReflectionUnionType($reflector, $owner, $type, $allowsNull);
}

Expand Down
12 changes: 12 additions & 0 deletions test/unit/Reflection/ReflectionTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ public function dataProvider(): array
[new Node\NullableType(new Node\Identifier('string')), false, ReflectionNamedType::class, true],
[new Node\IntersectionType([new Node\Name('A'), new Node\Name('B')]), false, ReflectionIntersectionType::class, false],
[new Node\UnionType([new Node\Name('A'), new Node\Name('B')]), false, ReflectionUnionType::class, false],
'Union types composed of just `null` and a type are simplified into a ReflectionNamedType' => [
new Node\UnionType([new Node\Name('A'), new Node\Name('null')]),
false,
ReflectionNamedType::class,
true,
],
'Union types composed of `null` and more than one type are kept as ReflectionUnionType' => [
new Node\UnionType([new Node\Name('A'), new Node\Name('B'), new Node\Name('null')]),
false,
ReflectionUnionType::class,
false,
],
];
}

Expand Down

0 comments on commit c861f48

Please sign in to comment.