From f15850693074458e1f1c9418154bf1da136b4d63 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 21 Oct 2020 18:46:41 -0700 Subject: [PATCH 1/6] Guard against crashes for entities which implement interfaces This prevents a call to Doctrine's metadata factory with an interface, which throws an exception since the class in question is not an entity. --- src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php b/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php index eb2e8a51..a149a7cb 100644 --- a/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php +++ b/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php @@ -59,6 +59,10 @@ public function processNode(Node $node, Scope $scope): array return []; } $entityClass = $entityClassType->getClassName(); + + if (interface_exists($entityClass)) { + return []; + } $methodNameIdentifier = $node->name; if (!$methodNameIdentifier instanceof Node\Identifier) { From 3a73e69255353f93d891ac8a0a773f033232cf45 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 22 Oct 2020 09:53:19 -0700 Subject: [PATCH 2/6] Fix indentation --- src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php b/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php index a149a7cb..d2f7d4f6 100644 --- a/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php +++ b/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php @@ -59,7 +59,7 @@ public function processNode(Node $node, Scope $scope): array return []; } $entityClass = $entityClassType->getClassName(); - + if (interface_exists($entityClass)) { return []; } From 627f76e55c7c55b768382773ca3fc865217f678b Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 22 Oct 2020 10:10:01 -0700 Subject: [PATCH 3/6] Reproduce structure --- .../data/MyEntityImplementingInterface.php | 25 +++++++++++++++++++ .../Doctrine/ORM/data/MyExtendedInterface.php | 8 ++++++ tests/Rules/Doctrine/ORM/data/MyInterface.php | 11 ++++++++ 3 files changed, 44 insertions(+) create mode 100644 tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php create mode 100644 tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php create mode 100644 tests/Rules/Doctrine/ORM/data/MyInterface.php diff --git a/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php b/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php new file mode 100644 index 00000000..8768d753 --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php @@ -0,0 +1,25 @@ + Date: Thu, 22 Oct 2020 10:40:15 -0700 Subject: [PATCH 4/6] Implement the tests --- .../ORM/EntityImplementingInterfaceTest.php | 38 +++++++++++++++++++ .../data/MyEntityImplementingInterface.php | 4 +- tests/Rules/Doctrine/ORM/data/MyInterface.php | 5 +-- .../ORM/data/repository-findBy-interface.php | 25 ++++++++++++ 4 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 tests/Rules/Doctrine/ORM/EntityImplementingInterfaceTest.php create mode 100644 tests/Rules/Doctrine/ORM/data/repository-findBy-interface.php diff --git a/tests/Rules/Doctrine/ORM/EntityImplementingInterfaceTest.php b/tests/Rules/Doctrine/ORM/EntityImplementingInterfaceTest.php new file mode 100644 index 00000000..ba0afad1 --- /dev/null +++ b/tests/Rules/Doctrine/ORM/EntityImplementingInterfaceTest.php @@ -0,0 +1,38 @@ + + */ +class EntityImplementingInterfaceTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new RepositoryMethodCallRule(new ObjectMetadataResolver($this->createReflectionProvider(), __DIR__ . '/entity-manager.php', null)); + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/magic-repository.neon']; + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/repository-findBy-interface.php'], [ + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntityImplementingInterface does not have a field named $nonexistent.', + 22, + ], + ]); + } + +} diff --git a/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php b/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php index 8768d753..133aff13 100644 --- a/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php +++ b/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php @@ -7,7 +7,7 @@ /** * @ORM\Entity() */ -class MyEntity implements MyExtendedInterface +class MyEntityImplementingInterface implements MyExtendedInterface { /** * @ORM\Id() @@ -18,7 +18,7 @@ class MyEntity implements MyExtendedInterface */ private $id; - public function requiredMethod() + public function requiredMethod(): bool { return true; } diff --git a/tests/Rules/Doctrine/ORM/data/MyInterface.php b/tests/Rules/Doctrine/ORM/data/MyInterface.php index 99eb3151..cc616a43 100644 --- a/tests/Rules/Doctrine/ORM/data/MyInterface.php +++ b/tests/Rules/Doctrine/ORM/data/MyInterface.php @@ -4,8 +4,5 @@ interface MyInterface { - /** - * @return bool - */ - public function requiredMethod(); + public function requiredMethod(): bool; } diff --git a/tests/Rules/Doctrine/ORM/data/repository-findBy-interface.php b/tests/Rules/Doctrine/ORM/data/repository-findBy-interface.php new file mode 100644 index 00000000..831e4b08 --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/repository-findBy-interface.php @@ -0,0 +1,25 @@ +entityManager = $entityManager; + } + + public function doFindBy(): void + { + $entityRepository = $this->entityManager->getRepository(MyEntityImplementingInterface::class); + $entityRepository->findBy(['id' => 1]); + $entityRepository->findBy(['nonexistent' => 'test']); + } + +} From 7927fdf2e497169eb9bec9a7aeddedb42a74ee8e Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 22 Oct 2020 10:42:41 -0700 Subject: [PATCH 5/6] Method on child interface which more accurately simulates original --- .../Doctrine/ORM/data/MyEntityImplementingInterface.php | 5 +++++ tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php b/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php index 133aff13..0b79f6c1 100644 --- a/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php +++ b/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php @@ -22,4 +22,9 @@ public function requiredMethod(): bool { return true; } + + public function getName(): string + { + return 'my name'; + } } diff --git a/tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php b/tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php index 14deb55a..1da76a1f 100644 --- a/tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php +++ b/tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php @@ -4,5 +4,5 @@ interface MyExtendedInterface extends MyInterface { - // No new methods + public function getName(): string; } From af52af39680858f472b390582142bdb3f8f92ee9 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Thu, 22 Oct 2020 13:11:04 -0700 Subject: [PATCH 6/6] Reproduce the actual, fundamental issue --- .../ORM/data/MyEntityImplementingInterface.php | 13 +++++++------ .../Doctrine/ORM/data/MyExtendedInterface.php | 8 -------- tests/Rules/Doctrine/ORM/data/MyInterface.php | 2 +- .../ORM/data/repository-findBy-interface.php | 15 +++++++++++++-- 4 files changed, 21 insertions(+), 17 deletions(-) delete mode 100644 tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php diff --git a/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php b/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php index 0b79f6c1..ad3c5bb2 100644 --- a/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php +++ b/tests/Rules/Doctrine/ORM/data/MyEntityImplementingInterface.php @@ -7,7 +7,7 @@ /** * @ORM\Entity() */ -class MyEntityImplementingInterface implements MyExtendedInterface +class MyEntityImplementingInterface implements MyInterface { /** * @ORM\Id() @@ -18,13 +18,14 @@ class MyEntityImplementingInterface implements MyExtendedInterface */ private $id; - public function requiredMethod(): bool - { - return true; - } + /** + * @ORM\Column() + * @var string + */ + private $name; public function getName(): string { - return 'my name'; + return $this->name; } } diff --git a/tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php b/tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php deleted file mode 100644 index 1da76a1f..00000000 --- a/tests/Rules/Doctrine/ORM/data/MyExtendedInterface.php +++ /dev/null @@ -1,8 +0,0 @@ -entityManager = $entityManager; } - public function doFindBy(): void + public function doFindByWithConcreteClass(): void { $entityRepository = $this->entityManager->getRepository(MyEntityImplementingInterface::class); $entityRepository->findBy(['id' => 1]); $entityRepository->findBy(['nonexistent' => 'test']); } + public function doFindByWithInterface(MyInterface $entity): void + { + $entityRepository = $this->entityManager->getRepository(get_class($entity)); + // This is likely to work, but cannot be inferred from the interface + // alone + $repo->findBy(['name' => $entity->getName()]); + // This is NOT likely to work, but can't be proven without examining + // all implementations of the interface + $entityRepository->findBy(['nonexistent' => 'test']); + } + }