Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing proxy methods #348

Merged
merged 1 commit into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ parameters:
count: 1
path: src/Persistence/Reflection/EnumReflectionProperty.php

-
message: "#^Method Doctrine\\\\Persistence\\\\Reflection\\\\EnumReflectionProperty\\:\\:getDeclaringClass\\(\\) return type with generic class ReflectionClass does not specify its types\\: T$#"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be solved by making EnumReflectionProperty generic on the class name, as done in the stubs for ReflectionProperty in Psalm and phpstan instead of adding it in the baseline IMO.

Copy link
Member Author

@greg0ire greg0ire Mar 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but I'm not sure how I can make the type flow from the constructor to there 🤔 I think I'm supposed to leverage ReflectionProperty's template types, if there are some, but I don't know where to find their definitions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, apparently, Psalm makes ReflectionProperty generic but phpstan does not. So I'm not sure solving this one is actually possible.

count: 1
path: src/Persistence/Reflection/EnumReflectionProperty.php

-
message: "#^Method Doctrine\\\\Persistence\\\\Reflection\\\\EnumReflectionProperty\\:\\:getType\\(\\) should return ReflectionNamedType\\|ReflectionUnionType\\|null but returns ReflectionType\\|null\\.$#"
count: 1
path: src/Persistence/Reflection/EnumReflectionProperty.php

-
message: "#^Method Doctrine\\\\Persistence\\\\Reflection\\\\EnumReflectionProperty\\:\\:toEnum\\(\\) should return array\\<BackedEnum\\>\\|BackedEnum but returns array\\<BackedEnum\\|int\\|string\\>\\.$#"
count: 1
Expand Down
40 changes: 40 additions & 0 deletions src/Persistence/Reflection/EnumReflectionProperty.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
namespace Doctrine\Persistence\Reflection;

use BackedEnum;
use ReflectionClass;
use ReflectionProperty;
use ReflectionType;
use ReturnTypeWillChange;

use function array_map;
Expand All @@ -30,6 +32,44 @@
$this->enumType = $enumType;
}

/**
* {@inheritDoc}
*
* @psalm-external-mutation-free
*/
public function getDeclaringClass(): ReflectionClass
{

Check warning on line 41 in src/Persistence/Reflection/EnumReflectionProperty.php

View check run for this annotation

Codecov / codecov/patch

src/Persistence/Reflection/EnumReflectionProperty.php#L41

Added line #L41 was not covered by tests
return $this->originalReflectionProperty->getDeclaringClass();
}

Check warning on line 43 in src/Persistence/Reflection/EnumReflectionProperty.php

View check run for this annotation

Codecov / codecov/patch

src/Persistence/Reflection/EnumReflectionProperty.php#L43

Added line #L43 was not covered by tests

/**
* {@inheritDoc}
*
* @psalm-external-mutation-free
*/
public function getName(): string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should be adding the @psalm-immutable annotation (which is not the same than @psalm-pure that you tried)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it locally, to no avail, then noticed I still had Psalm 4, apparently because Psalm 5 is not compatible with php 8.3 yet, so I just tried pushing, and it doesn't work with Psalm 5 either.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just @psalm-external-mutation-free seems sufficient.

{

Check warning on line 51 in src/Persistence/Reflection/EnumReflectionProperty.php

View check run for this annotation

Codecov / codecov/patch

src/Persistence/Reflection/EnumReflectionProperty.php#L51

Added line #L51 was not covered by tests
return $this->originalReflectionProperty->getName();
}

Check warning on line 53 in src/Persistence/Reflection/EnumReflectionProperty.php

View check run for this annotation

Codecov / codecov/patch

src/Persistence/Reflection/EnumReflectionProperty.php#L53

Added line #L53 was not covered by tests

/**
* {@inheritDoc}
*
* @psalm-external-mutation-free
*/
public function getType(): ?ReflectionType
{

Check warning on line 61 in src/Persistence/Reflection/EnumReflectionProperty.php

View check run for this annotation

Codecov / codecov/patch

src/Persistence/Reflection/EnumReflectionProperty.php#L61

Added line #L61 was not covered by tests
return $this->originalReflectionProperty->getType();
}

Check warning on line 63 in src/Persistence/Reflection/EnumReflectionProperty.php

View check run for this annotation

Codecov / codecov/patch

src/Persistence/Reflection/EnumReflectionProperty.php#L63

Added line #L63 was not covered by tests

/**
* {@inheritDoc}
*/
public function getAttributes(?string $name = null, int $flags = 0): array
{

Check warning on line 69 in src/Persistence/Reflection/EnumReflectionProperty.php

View check run for this annotation

Codecov / codecov/patch

src/Persistence/Reflection/EnumReflectionProperty.php#L69

Added line #L69 was not covered by tests
return $this->originalReflectionProperty->getAttributes($name, $flags);
}

Check warning on line 71 in src/Persistence/Reflection/EnumReflectionProperty.php

View check run for this annotation

Codecov / codecov/patch

src/Persistence/Reflection/EnumReflectionProperty.php#L71

Added line #L71 was not covered by tests

/**
* {@inheritDoc}
*
Expand Down
43 changes: 40 additions & 3 deletions tests_php81/Persistence/Reflection/EnumReflectionPropertyTest.php
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests_PHP81\Persistence\Reflection;

use Attribute;
use Doctrine\Persistence\Reflection\EnumReflectionProperty;
use PHPUnit\Framework\TestCase;
use ReflectionNamedType;
use ReflectionProperty;
use ValueError;

Expand All @@ -20,6 +24,33 @@ public function testGetValue(): void
self::assertNull($reflProperty->getValue($object));
}

public function testGetDeclaringClass(): void
{
$reflProperty = new EnumReflectionProperty(new ReflectionProperty(TypedEnumClass::class, 'suit'), Suit::class);
self::assertSame(TypedEnumClass::class, $reflProperty->getDeclaringClass()->getName());
}

public function testGetName(): void
{
$reflProperty = new EnumReflectionProperty(new ReflectionProperty(TypedEnumClass::class, 'suit'), Suit::class);
self::assertSame('suit', $reflProperty->getName());
}

public function testGetType(): void
{
$reflProperty = new EnumReflectionProperty(new ReflectionProperty(TypedEnumClass::class, 'suit'), Suit::class);
$type = $reflProperty->getType();
self::assertInstanceOf(ReflectionNamedType::class, $type);
self::assertSame(Suit::class, $type->getName());
}

public function testGetAttributes(): void
{
$reflProperty = new EnumReflectionProperty(new ReflectionProperty(TypedEnumClass::class, 'suit'), Suit::class);
self::assertCount(1, $reflProperty->getAttributes());
self::assertSame(MyAttribute::class, $reflProperty->getAttributes()[0]->getName());
}

public function testSetValidValue(): void
{
$object = new TypedEnumClass();
Expand Down Expand Up @@ -84,17 +115,23 @@ public function testSetEnumArray(): void
}
}

#[Attribute(Attribute::TARGET_PROPERTY)]
class MyAttribute
{
}

class TypedEnumClass
{
#[MyAttribute]
public ?Suit $suit = null;

public ?array $suits = null;
}

enum Suit: string
{
case Hearts = 'H';
case Hearts = 'H';
case Diamonds = 'D';
case Clubs = 'C';
case Spades = 'S';
case Clubs = 'C';
case Spades = 'S';
}
Loading