diff --git a/rules.neon b/rules.neon index 0853184d..1a864a70 100644 --- a/rules.neon +++ b/rules.neon @@ -46,6 +46,12 @@ services: allowNullablePropertyForRequiredField: %doctrine.allowNullablePropertyForRequiredField% tags: - phpstan.rules.rule + - + class: PHPStan\Rules\Doctrine\ORM\RepositoryMethodCallRule + arguments: + checkOrderByFields: %featureToggles.bleedingEdge% + tags: + - phpstan.rules.rule - class: PHPStan\Classes\DoctrineProxyForbiddenClassNamesExtension tags: diff --git a/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php b/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php index 10b79867..38ca3e9b 100644 --- a/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php +++ b/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php @@ -21,9 +21,15 @@ class RepositoryMethodCallRule implements Rule private ObjectMetadataResolver $objectMetadataResolver; - public function __construct(ObjectMetadataResolver $objectMetadataResolver) + private bool $checkOrderByFields; + + public function __construct( + ObjectMetadataResolver $objectMetadataResolver, + bool $checkOrderByFields = false + ) { $this->objectMetadataResolver = $objectMetadataResolver; + $this->checkOrderByFields = $checkOrderByFields; } public function getNodeType(): string @@ -87,6 +93,44 @@ public function processNode(Node $node, Scope $scope): array } } + if (!$this->checkOrderByFields) { + return $messages; + } + + if (!in_array($methodName, [ + 'findBy', + 'findOneBy', + ], true)) { + return $messages; + } + + $orderByType = count($node->getArgs()) > 1 ? $scope->getType($node->getArgs()[1]->value) : null; + + if ($orderByType === null) { + return $messages; + } + + foreach ($orderByType->getConstantArrays() as $constantArray) { + foreach ($constantArray->getKeyTypes() as $keyType) { + foreach ($keyType->getConstantStrings() as $fieldName) { + if ( + $classMetadata->hasField($fieldName->getValue()) + || $classMetadata->hasAssociation($fieldName->getValue()) + ) { + continue; + } + + $messages[] = RuleErrorBuilder::message(sprintf( + 'Call to method %s::%s() - entity %s does not have a field named $%s.', + $calledOnType->describe(VerbosityLevel::typeOnly()), + $methodName, + $entityClassNames[0], + $fieldName->getValue(), + ))->identifier(sprintf('doctrine.%sArgument', $methodName))->build(); + } + } + } + return $messages; } diff --git a/tests/DoctrineIntegration/ORM/data/entityRepositoryDynamicReturn-0.json b/tests/DoctrineIntegration/ORM/data/entityRepositoryDynamicReturn-0.json index 36e04f9a..501f5273 100644 --- a/tests/DoctrineIntegration/ORM/data/entityRepositoryDynamicReturn-0.json +++ b/tests/DoctrineIntegration/ORM/data/entityRepositoryDynamicReturn-0.json @@ -4,6 +4,16 @@ "line": 94, "ignorable": true }, + { + "message": "Call to method Doctrine\\ORM\\EntityRepository::findOneBy() - entity PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity does not have a field named $blah.", + "line": 94, + "ignorable": true + }, + { + "message": "Call to method Doctrine\\ORM\\EntityRepository::findBy() - entity PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity does not have a field named $blah.", + "line": 116, + "ignorable": true + }, { "message": "Call to method Doctrine\\ORM\\EntityRepository::findBy() - entity PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity does not have a field named $blah.", "line": 116, diff --git a/tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleTest.php b/tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleTest.php index da10cbf3..cf60ea16 100644 --- a/tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleTest.php +++ b/tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleTest.php @@ -14,7 +14,7 @@ class RepositoryMethodCallRuleTest extends RuleTestCase protected function getRule(): Rule { - return new RepositoryMethodCallRule(new ObjectMetadataResolver(__DIR__ . '/entity-manager.php', __DIR__ . '/../../../../tmp')); + return new RepositoryMethodCallRule(new ObjectMetadataResolver(__DIR__ . '/entity-manager.php', __DIR__ . '/../../../../tmp'), true); } /** @@ -76,6 +76,38 @@ public function testRule(): void 'Call to method Doctrine\ORM\EntityRepository::count() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', 45, ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', + 54, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.', + 55, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.', + 56, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', + 56, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', + 64, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.', + 65, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.', + 66, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', + 66, + ], ]); } diff --git a/tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleWithoutObjectManagerLoaderTest.php b/tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleWithoutObjectManagerLoaderTest.php index 849d3675..717188a8 100644 --- a/tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleWithoutObjectManagerLoaderTest.php +++ b/tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleWithoutObjectManagerLoaderTest.php @@ -14,7 +14,7 @@ class RepositoryMethodCallRuleWithoutObjectManagerLoaderTest extends RuleTestCas protected function getRule(): Rule { - return new RepositoryMethodCallRule(new ObjectMetadataResolver(null, __DIR__ . '/../../../../tmp')); + return new RepositoryMethodCallRule(new ObjectMetadataResolver(null, __DIR__ . '/../../../../tmp'), true); } /** @@ -76,6 +76,38 @@ public function testRule(): void 'Call to method Doctrine\ORM\EntityRepository::count() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', 45, ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', + 54, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.', + 55, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.', + 56, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', + 56, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', + 64, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.', + 65, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.', + 66, + ], + [ + 'Call to method Doctrine\ORM\EntityRepository::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.', + 66, + ], ]); } diff --git a/tests/Rules/Doctrine/ORM/data/repository-findBy-etc.php b/tests/Rules/Doctrine/ORM/data/repository-findBy-etc.php index ee5fa6a7..c4271b5c 100644 --- a/tests/Rules/Doctrine/ORM/data/repository-findBy-etc.php +++ b/tests/Rules/Doctrine/ORM/data/repository-findBy-etc.php @@ -45,4 +45,25 @@ public function doCountBy(): void $entityRepository->count(['nonexistent' => 'test', 'transient' => 'test']); } + + public function doFindByOrder(): void + { + $entityRepository = $this->entityManager->getRepository(MyEntity::class); + $entityRepository->findBy([], ['id' => 'ASC']); + $entityRepository->findBy([], ['title' => 'DESC']); + $entityRepository->findBy([], ['transient' => 'ASC']); + $entityRepository->findBy([], ['nonexistent' => 'DESC']); + $entityRepository->findBy([], ['nonexistent' => 'ASC', 'transient' => 'DESC']); + } + + public function doFindOneByOrder(): void + { + $entityRepository = $this->entityManager->getRepository(MyEntity::class); + $entityRepository->findOneBy([], ['id' => 'ASC']); + $entityRepository->findOneBy([], ['title' => 'DESC']); + $entityRepository->findOneBy([], ['transient' => 'ASC']); + $entityRepository->findOneBy([], ['nonexistent' => 'DESC']); + $entityRepository->findOneBy([], ['nonexistent' => 'ASC', 'transient' => 'DESC']); + } + }