From 0aa9b6a6d9f81716a94284fca21fd04dcf91768c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Unger?= Date: Sun, 6 Oct 2019 11:10:12 +0200 Subject: [PATCH 1/4] Implemented entity relation checking rule --- rules.neon | 1 + src/Rules/Doctrine/ORM/EntityRelationRule.php | 100 ++++++++++++++++++ .../Doctrine/ORM/EntityRelationRuleTest.php | 24 +++++ .../Rules/Doctrine/ORM/data/AnotherEntity.php | 25 +++++ .../Doctrine/ORM/data/EntityWithRelations.php | 44 ++++++++ 5 files changed, 194 insertions(+) create mode 100644 src/Rules/Doctrine/ORM/EntityRelationRule.php create mode 100644 tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php create mode 100644 tests/Rules/Doctrine/ORM/data/AnotherEntity.php create mode 100644 tests/Rules/Doctrine/ORM/data/EntityWithRelations.php diff --git a/rules.neon b/rules.neon index 63eaa60f..f38ecafe 100644 --- a/rules.neon +++ b/rules.neon @@ -18,6 +18,7 @@ rules: - PHPStan\Rules\Doctrine\ORM\MagicRepositoryMethodCallRule - PHPStan\Rules\Doctrine\ORM\RepositoryMethodCallRule - PHPStan\Rules\Doctrine\ORM\EntityColumnRule + - PHPStan\Rules\Doctrine\ORM\EntityRelationRule services: - diff --git a/src/Rules/Doctrine/ORM/EntityRelationRule.php b/src/Rules/Doctrine/ORM/EntityRelationRule.php new file mode 100644 index 00000000..0af79c85 --- /dev/null +++ b/src/Rules/Doctrine/ORM/EntityRelationRule.php @@ -0,0 +1,100 @@ +objectMetadataResolver = $objectMetadataResolver; + } + + public function getNodeType(): string + { + return Node\Stmt\PropertyProperty::class; + } + + /** + * @param \PhpParser\Node\Stmt\PropertyProperty $node + * @param \PHPStan\Analyser\Scope $scope + * @return string[] + */ + public function processNode(Node $node, Scope $scope): array + { + $class = $scope->getClassReflection(); + if ($class === null) { + return []; + } + + $objectManager = $this->objectMetadataResolver->getObjectManager(); + if ($objectManager === null) { + return []; + } + + $className = $class->getName(); + if ($objectManager->getMetadataFactory()->isTransient($className)) { + return []; + } + + /** @var \Doctrine\ORM\Mapping\ClassMetadataInfo $metadata */ + $metadata = $objectManager->getClassMetadata($className); + $classMetadataInfo = 'Doctrine\ORM\Mapping\ClassMetadataInfo'; + if (!$metadata instanceof $classMetadataInfo) { + return []; + } + + $propertyName = (string) $node->name; + try { + $property = $class->getNativeProperty($propertyName); + } catch (MissingPropertyFromReflectionException $e) { + return []; + } + + if (!isset($metadata->associationMappings[$propertyName])) { + return []; + } + $associationMapping = $metadata->associationMappings[$propertyName]; + + $columnType = null; + if ((bool) ($associationMapping['type'] & 3)) { // ClassMetadataInfo::TO_ONE + $columnType = new ObjectType($associationMapping['targetEntity']); + if ($associationMapping['joinColumns'][0]['nullable'] ?? true) { + $columnType = TypeCombinator::addNull($columnType); + } + } elseif ((bool) ($associationMapping['type'] & 12)) { // ClassMetadataInfo::TO_MANY + $columnType = TypeCombinator::intersect( + new ObjectType('Doctrine\Common\Collections\Collection'), + new IterableType(new MixedType(), new ObjectType($associationMapping['targetEntity'])) + ); + } + + $errors = []; + if ($columnType !== null) { + if (!$property->getWritableType()->isSuperTypeOf($columnType)->yes()) { + $errors[] = sprintf('Database can contain %s but property expects %s.', $columnType->describe(VerbosityLevel::typeOnly()), $property->getWritableType()->describe(VerbosityLevel::typeOnly())); + } + if (!$columnType->isSuperTypeOf($property->getReadableType())->yes()) { + $errors[] = sprintf('Property can contain %s but database expects %s.', $property->getReadableType()->describe(VerbosityLevel::typeOnly()), $columnType->describe(VerbosityLevel::typeOnly())); + } + } + + return $errors; + } + +} diff --git a/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php b/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php new file mode 100644 index 00000000..bb2dd0cb --- /dev/null +++ b/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php @@ -0,0 +1,24 @@ +analyse([__DIR__ . '/data/EntityWithRelations.php'], []); + } + +} diff --git a/tests/Rules/Doctrine/ORM/data/AnotherEntity.php b/tests/Rules/Doctrine/ORM/data/AnotherEntity.php new file mode 100644 index 00000000..16074080 --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/AnotherEntity.php @@ -0,0 +1,25 @@ + + */ + private $oneToMany; + + /** + * @ORM\ManyToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity") + * @var \Doctrine\Common\Collections\Collection&iterable<\PHPStan\Rules\Doctrine\ORM\AnotherEntity> + */ + private $manyToMany; + +} From 2db8f272a1adf3b1253ba46cb53e38fe3603dcf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Unger?= Date: Sun, 6 Oct 2019 12:05:04 +0200 Subject: [PATCH 2/4] Added tests for OneToOne relations --- .../Doctrine/ORM/EntityRelationRuleTest.php | 31 ++++++++++- .../EntityWithBrokenOneToOneRelations.php | 52 +++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 tests/Rules/Doctrine/ORM/data/EntityWithBrokenOneToOneRelations.php diff --git a/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php b/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php index bb2dd0cb..465f828f 100644 --- a/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php +++ b/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Doctrine\ORM; +use Iterator; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPStan\Type\Doctrine\ObjectMetadataResolver; @@ -16,9 +17,35 @@ protected function getRule(): Rule ); } - public function testRule(): void + /** + * @dataProvider ruleProvider + */ + public function testRule(string $file, array $expectedErrors): void { - $this->analyse([__DIR__ . '/data/EntityWithRelations.php'], []); + $this->analyse([$file], $expectedErrors); + } + + public function ruleProvider(): Iterator + { + yield [__DIR__ . '/data/EntityWithRelations.php', []]; + yield [__DIR__ . '/data/EntityWithBrokenOneToOneRelations.php', [ + [ + 'Property can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', + 31, + ], + [ + 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', + 37, + ], + [ + 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\MyEntity|null.', + 50, + ], + [ + 'Property can contain PHPStan\Rules\Doctrine\ORM\MyEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity|null.', + 50, + ] + ]]; } } diff --git a/tests/Rules/Doctrine/ORM/data/EntityWithBrokenOneToOneRelations.php b/tests/Rules/Doctrine/ORM/data/EntityWithBrokenOneToOneRelations.php new file mode 100644 index 00000000..4605e98e --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/EntityWithBrokenOneToOneRelations.php @@ -0,0 +1,52 @@ + Date: Sun, 6 Oct 2019 12:07:39 +0200 Subject: [PATCH 3/4] Added tests for ManyToOne relations --- .../Doctrine/ORM/EntityRelationRuleTest.php | 24 ++++++++- .../EntityWithBrokenManyToOneRelations.php | 52 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 tests/Rules/Doctrine/ORM/data/EntityWithBrokenManyToOneRelations.php diff --git a/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php b/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php index 465f828f..96c9a684 100644 --- a/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php +++ b/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php @@ -27,8 +27,28 @@ public function testRule(string $file, array $expectedErrors): void public function ruleProvider(): Iterator { - yield [__DIR__ . '/data/EntityWithRelations.php', []]; - yield [__DIR__ . '/data/EntityWithBrokenOneToOneRelations.php', [ + yield 'nice entity' => [__DIR__ . '/data/EntityWithRelations.php', []]; + + yield 'one to one' => [__DIR__ . '/data/EntityWithBrokenOneToOneRelations.php', [ + [ + 'Property can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', + 31, + ], + [ + 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', + 37, + ], + [ + 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\MyEntity|null.', + 50, + ], + [ + 'Property can contain PHPStan\Rules\Doctrine\ORM\MyEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity|null.', + 50, + ] + ]]; + + yield 'many to one' => [__DIR__ . '/data/EntityWithBrokenManyToOneRelations.php', [ [ 'Property can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', 31, diff --git a/tests/Rules/Doctrine/ORM/data/EntityWithBrokenManyToOneRelations.php b/tests/Rules/Doctrine/ORM/data/EntityWithBrokenManyToOneRelations.php new file mode 100644 index 00000000..c97d4567 --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/EntityWithBrokenManyToOneRelations.php @@ -0,0 +1,52 @@ + Date: Sun, 6 Oct 2019 12:20:21 +0200 Subject: [PATCH 4/4] Added tests for ToMany relations --- .../Doctrine/ORM/EntityRelationRuleTest.php | 106 ++++++++++++------ .../Rules/Doctrine/ORM/data/AnotherEntity.php | 25 +++++ .../EntityWithBrokenManyToManyRelations.php | 50 +++++++++ .../EntityWithBrokenOneToManyRelations.php | 50 +++++++++ 4 files changed, 199 insertions(+), 32 deletions(-) create mode 100644 tests/Rules/Doctrine/ORM/data/EntityWithBrokenManyToManyRelations.php create mode 100644 tests/Rules/Doctrine/ORM/data/EntityWithBrokenOneToManyRelations.php diff --git a/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php b/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php index 96c9a684..e2078842 100644 --- a/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php +++ b/tests/Rules/Doctrine/ORM/EntityRelationRuleTest.php @@ -29,43 +29,85 @@ public function ruleProvider(): Iterator { yield 'nice entity' => [__DIR__ . '/data/EntityWithRelations.php', []]; - yield 'one to one' => [__DIR__ . '/data/EntityWithBrokenOneToOneRelations.php', [ + yield 'one to one' => [__DIR__ . '/data/EntityWithBrokenOneToOneRelations.php', [ - 'Property can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', - 31, - ], - [ - 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', - 37, - ], - [ - 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\MyEntity|null.', - 50, - ], - [ - 'Property can contain PHPStan\Rules\Doctrine\ORM\MyEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity|null.', - 50, - ] - ]]; + [ + 'Property can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', + 31, + ], + [ + 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', + 37, + ], + [ + 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\MyEntity|null.', + 50, + ], + [ + 'Property can contain PHPStan\Rules\Doctrine\ORM\MyEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity|null.', + 50, + ], + ]]; - yield 'many to one' => [__DIR__ . '/data/EntityWithBrokenManyToOneRelations.php', [ + yield 'many to one' => [__DIR__ . '/data/EntityWithBrokenManyToOneRelations.php', [ - 'Property can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', - 31, - ], - [ - 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', - 37, - ], + [ + 'Property can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', + 31, + ], + [ + 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\AnotherEntity.', + 37, + ], + [ + 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\MyEntity|null.', + 50, + ], + [ + 'Property can contain PHPStan\Rules\Doctrine\ORM\MyEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity|null.', + 50, + ], + ]]; + + yield 'one to many' => [__DIR__ . '/data/EntityWithBrokenOneToManyRelations.php', [ - 'Database can contain PHPStan\Rules\Doctrine\ORM\AnotherEntity|null but property expects PHPStan\Rules\Doctrine\ORM\MyEntity|null.', - 50, - ], + [ + 'Property can contain iterable but database expects Doctrine\Common\Collections\Collection&iterable.', + 24, + ], + [ + 'Property can contain Doctrine\Common\Collections\Collection but database expects Doctrine\Common\Collections\Collection&iterable.', + 30, + ], + [ + 'Database can contain Doctrine\Common\Collections\Collection&iterable but property expects array.', + 36, + ], + [ + 'Property can contain array but database expects Doctrine\Common\Collections\Collection&iterable.', + 36, + ], + ]]; + + yield 'many to many' => [__DIR__ . '/data/EntityWithBrokenManyToManyRelations.php', [ - 'Property can contain PHPStan\Rules\Doctrine\ORM\MyEntity|null but database expects PHPStan\Rules\Doctrine\ORM\AnotherEntity|null.', - 50, - ] - ]]; + [ + 'Property can contain iterable but database expects Doctrine\Common\Collections\Collection&iterable.', + 24, + ], + [ + 'Property can contain Doctrine\Common\Collections\Collection but database expects Doctrine\Common\Collections\Collection&iterable.', + 30, + ], + [ + 'Database can contain Doctrine\Common\Collections\Collection&iterable but property expects array.', + 36, + ], + [ + 'Property can contain array but database expects Doctrine\Common\Collections\Collection&iterable.', + 36, + ], + ]]; } } diff --git a/tests/Rules/Doctrine/ORM/data/AnotherEntity.php b/tests/Rules/Doctrine/ORM/data/AnotherEntity.php index 16074080..70c91ee8 100644 --- a/tests/Rules/Doctrine/ORM/data/AnotherEntity.php +++ b/tests/Rules/Doctrine/ORM/data/AnotherEntity.php @@ -22,4 +22,29 @@ class AnotherEntity */ private $manyToOne; + /** + * @ORM\ManyToOne(targetEntity="PHPStan\Rules\Doctrine\ORM\EntityWithBrokenOneToManyRelations") + */ + private $one; + + /** + * @ORM\ManyToOne(targetEntity="PHPStan\Rules\Doctrine\ORM\EntityWithBrokenOneToManyRelations") + */ + private $two; + + /** + * @ORM\ManyToOne(targetEntity="PHPStan\Rules\Doctrine\ORM\EntityWithBrokenOneToManyRelations") + */ + private $three; + + /** + * @ORM\ManyToOne(targetEntity="PHPStan\Rules\Doctrine\ORM\EntityWithBrokenOneToManyRelations") + */ + private $four; + + /** + * @ORM\ManyToOne(targetEntity="PHPStan\Rules\Doctrine\ORM\EntityWithBrokenOneToManyRelations") + */ + private $five; + } diff --git a/tests/Rules/Doctrine/ORM/data/EntityWithBrokenManyToManyRelations.php b/tests/Rules/Doctrine/ORM/data/EntityWithBrokenManyToManyRelations.php new file mode 100644 index 00000000..dd2e0d78 --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/EntityWithBrokenManyToManyRelations.php @@ -0,0 +1,50 @@ + + */ + private $manyToManyWithIterableAnnotation; + + /** + * @ORM\ManyToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity") + * @var \Doctrine\Common\Collections\Collection + */ + private $manyToManyWithCollectionAnnotation; + + /** + * @ORM\ManyToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity") + * @var \PHPStan\Rules\Doctrine\ORM\AnotherEntity[] + */ + private $manyToManyWithArrayAnnotation; + + /** + * @ORM\ManyToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity") + * @var \Doctrine\Common\Collections\Collection&iterable<\PHPStan\Rules\Doctrine\ORM\AnotherEntity> + */ + private $manyToManyWithCorrectAnnotation; + + /** + * @ORM\ManyToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity") + * @var \Doctrine\Common\Collections\Collection|\PHPStan\Rules\Doctrine\ORM\AnotherEntity[] + */ + private $manyToManyWithCorrectOldStyleAnnotation; + +} diff --git a/tests/Rules/Doctrine/ORM/data/EntityWithBrokenOneToManyRelations.php b/tests/Rules/Doctrine/ORM/data/EntityWithBrokenOneToManyRelations.php new file mode 100644 index 00000000..421012f0 --- /dev/null +++ b/tests/Rules/Doctrine/ORM/data/EntityWithBrokenOneToManyRelations.php @@ -0,0 +1,50 @@ + + */ + private $oneToManyWithIterableAnnotation; + + /** + * @ORM\OneToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity", mappedBy="two") + * @var \Doctrine\Common\Collections\Collection + */ + private $oneToManyWithCollectionAnnotation; + + /** + * @ORM\OneToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity", mappedBy="three") + * @var \PHPStan\Rules\Doctrine\ORM\AnotherEntity[] + */ + private $oneToManyWithArrayAnnotation; + + /** + * @ORM\OneToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity", mappedBy="four") + * @var \Doctrine\Common\Collections\Collection&iterable<\PHPStan\Rules\Doctrine\ORM\AnotherEntity> + */ + private $oneToManyWithCorrectAnnotation; + + /** + * @ORM\OneToMany(targetEntity="PHPStan\Rules\Doctrine\ORM\AnotherEntity", mappedBy="five") + * @var \Doctrine\Common\Collections\Collection|\PHPStan\Rules\Doctrine\ORM\AnotherEntity[] + */ + private $oneToManyWithCorrectOldStyleAnnotation; + +}