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

[doctrine] fix repository crash when variable classname is used (#280) #283

Merged
merged 1 commit into from
Oct 29, 2022
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
14 changes: 10 additions & 4 deletions src/Handler/DoctrineRepositoryHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
use Psalm\SymfonyPsalmPlugin\Issue\RepositoryStringShortcut;
use Psalm\Type\Atomic\TNamedObject;
use Psalm\Type\Union;
use ReflectionClass;
use ReflectionException;

class DoctrineRepositoryHandler implements AfterMethodCallAnalysisInterface, AfterClassLikeVisitInterface
{
Expand All @@ -41,13 +43,17 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
$statements_source->getSuppressedIssues()
);
} elseif ($entityName instanceof Expr\ClassConstFetch) {
/** @psalm-var class-string $className */
/** @psalm-var class-string|null $className */
$className = $entityName->class->getAttribute('resolvedName');

if (null === $className) {
return;
}

try {
$reflectionClass = new \ReflectionClass($className);
$reflectionClass = new ReflectionClass($className);

if (method_exists(\ReflectionClass::class, 'getAttributes')) {
if (\PHP_VERSION_ID >= 80000 && method_exists(ReflectionClass::class, 'getAttributes')) {
$entityAttributes = $reflectionClass->getAttributes(EntityAnnotation::class);

foreach ($entityAttributes as $entityAttribute) {
Expand All @@ -70,7 +76,7 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
$event->setReturnTypeCandidate(new Union([new TNamedObject($entityAnnotation->repositoryClass)]));
}
}
} catch (\ReflectionException $e) {
} catch (ReflectionException $e) {
}
}
}
Expand Down
53 changes: 53 additions & 0 deletions tests/acceptance/acceptance/doctrine/RepositoryClass.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
@symfony-common
Feature: RepositoryClass

Background:
Given I have issue handlers "UndefinedClass,UnusedVariable" suppressed
And I have Symfony plugin enabled
And I have the following code preamble
"""
<?php
namespace RepositoryClass;

use Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\Foo;
use Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\FooRepository;
use Doctrine\ORM\EntityManagerInterface;
"""

Scenario: The plugin can find correct repository class from entity
Given I have the following code
"""
class SomeService
{
public function __construct(EntityManagerInterface $entityManager)
{
/** @psalm-trace $repository */
$repository = $entityManager->getRepository(Foo::class);
}
}
"""
When I run Psalm
Then I see these errors
| Type | Message |
| Trace | $repository: Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\FooRepository |
And I see no other errors

Scenario: Passing variable class does not crash the plugin
Given I have the following code
"""
class SomeService
{
public function __construct(EntityManagerInterface $entityManager)
{
$entity = 'Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\Foo';
/** @psalm-trace $repository */
$repository = $entityManager->getRepository($entity::class);
}
}
"""
When I run Psalm
Then I see these errors
| Type | Message |
| Trace | $repository: Doctrine\ORM\EntityRepository<object> |
| MixedArgument | Argument 1 of Doctrine\ORM\EntityManagerInterface::getRepository cannot be mixed, expecting class-string |
And I see no other errors
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,21 @@ Feature: RepositoryStringShortcut
I need Psalm to check preferred repository syntax

Background:
Given I have issue handlers "UndefinedClass,UnusedVariable" suppressed
Given I have issue handlers "ArgumentTypeCoercion,MixedArgument,UndefinedClass" suppressed
And I have Symfony plugin enabled
And I have the following code preamble
"""
<?php
namespace Doctrine\ORM;
interface EntityManagerInterface
{
/**
* @param string $className
*
* @return void
*/
public function getRepository($className);
}

namespace RepositoryStringShortcut;

use Doctrine\ORM\EntityManagerInterface;
use Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine\Foo;
"""

Scenario: Asserting using 'AppBundle:Entity' syntax raises issue
Given I have the following code
"""
use Doctrine\ORM\EntityManagerInterface;
class SomeService
{
public function __construct(EntityManagerInterface $entityManager)
Expand All @@ -36,7 +30,7 @@ Feature: RepositoryStringShortcut
"""
When I run Psalm
Then I see these errors
| Type | Message |
| Type | Message |
| RepositoryStringShortcut | Use Entity::class syntax instead |
And I see no other errors

Expand All @@ -47,7 +41,7 @@ Feature: RepositoryStringShortcut
{
public function __construct(EntityManagerInterface $entityManager)
{
$entityManager->getRepository(Entity::class);
$entityManager->getRepository(Foo::class);
}
}
"""
Expand All @@ -61,7 +55,7 @@ Feature: RepositoryStringShortcut
{
public function __construct(EntityManagerInterface $entityManager)
{
$className = 'App\Entity\EntityA';
$className = Foo::class;
$entityManager->getRepository($className);
}
}
Expand Down
14 changes: 14 additions & 0 deletions tests/fixture/Doctrine/Foo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine;

use Doctrine\ORM\Mapping\Entity;

/**
* @Entity(repositoryClass=FooRepository::class)
*/
class Foo
{
}
11 changes: 11 additions & 0 deletions tests/fixture/Doctrine/FooRepository.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace Psalm\SymfonyPsalmPlugin\Tests\Fixture\Doctrine;

use Doctrine\ORM\EntityRepository;

class FooRepository extends EntityRepository
{
}