Skip to content

Commit 5485522

Browse files
whataboutpereirawhataboutpereira
authored
Check field names of orderBy parameter in findBy and findOneBy
1 parent 4821d67 commit 5485522

6 files changed

Lines changed: 148 additions & 3 deletions

File tree

‎rules.neon‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ services:
4646
allowNullablePropertyForRequiredField: %doctrine.allowNullablePropertyForRequiredField%
4747
tags:
4848
- phpstan.rules.rule
49+
-
50+
class: PHPStan\Rules\Doctrine\ORM\RepositoryMethodCallRule
51+
arguments:
52+
checkOrderByFields: %featureToggles.bleedingEdge%
53+
tags:
54+
- phpstan.rules.rule
4955
-
5056
class: PHPStan\Classes\DoctrineProxyForbiddenClassNamesExtension
5157
tags:

‎src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,15 @@ class RepositoryMethodCallRule implements Rule
2121

2222
private ObjectMetadataResolver $objectMetadataResolver;
2323

24-
public function __construct(ObjectMetadataResolver $objectMetadataResolver)
24+
private bool $checkOrderByFields;
25+
26+
public function __construct(
27+
ObjectMetadataResolver $objectMetadataResolver,
28+
bool $checkOrderByFields = false
29+
)
2530
{
2631
$this->objectMetadataResolver = $objectMetadataResolver;
32+
$this->checkOrderByFields = $checkOrderByFields;
2733
}
2834

2935
public function getNodeType(): string
@@ -87,6 +93,44 @@ public function processNode(Node $node, Scope $scope): array
8793
}
8894
}
8995

96+
if (!$this->checkOrderByFields) {
97+
return $messages;
98+
}
99+
100+
if (!in_array($methodName, [
101+
'findBy',
102+
'findOneBy',
103+
], true)) {
104+
return $messages;
105+
}
106+
107+
$orderByType = count($node->getArgs()) > 1 ? $scope->getType($node->getArgs()[1]->value) : null;
108+
109+
if ($orderByType === null) {
110+
return $messages;
111+
}
112+
113+
foreach ($orderByType->getConstantArrays() as $constantArray) {
114+
foreach ($constantArray->getKeyTypes() as $keyType) {
115+
foreach ($keyType->getConstantStrings() as $fieldName) {
116+
if (
117+
$classMetadata->hasField($fieldName->getValue())
118+
|| $classMetadata->hasAssociation($fieldName->getValue())
119+
) {
120+
continue;
121+
}
122+
123+
$messages[] = RuleErrorBuilder::message(sprintf(
124+
'Call to method %s::%s() - entity %s does not have a field named $%s.',
125+
$calledOnType->describe(VerbosityLevel::typeOnly()),
126+
$methodName,
127+
$entityClassNames[0],
128+
$fieldName->getValue(),
129+
))->identifier(sprintf('doctrine.%sArgument', $methodName))->build();
130+
}
131+
}
132+
}
133+
90134
return $messages;
91135
}
92136

‎tests/DoctrineIntegration/ORM/data/entityRepositoryDynamicReturn-0.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,16 @@
44
"line": 94,
55
"ignorable": true
66
},
7+
{
8+
"message": "Call to method Doctrine\\ORM\\EntityRepository<PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity>::findOneBy() - entity PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity does not have a field named $blah.",
9+
"line": 94,
10+
"ignorable": true
11+
},
12+
{
13+
"message": "Call to method Doctrine\\ORM\\EntityRepository<PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity>::findBy() - entity PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity does not have a field named $blah.",
14+
"line": 116,
15+
"ignorable": true
16+
},
717
{
818
"message": "Call to method Doctrine\\ORM\\EntityRepository<PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity>::findBy() - entity PHPStan\\DoctrineIntegration\\ORM\\EntityRepositoryDynamicReturn\\MyEntity does not have a field named $blah.",
919
"line": 116,

‎tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleTest.php‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class RepositoryMethodCallRuleTest extends RuleTestCase
1414

1515
protected function getRule(): Rule
1616
{
17-
return new RepositoryMethodCallRule(new ObjectMetadataResolver(__DIR__ . '/entity-manager.php', __DIR__ . '/../../../../tmp'));
17+
return new RepositoryMethodCallRule(new ObjectMetadataResolver(__DIR__ . '/entity-manager.php', __DIR__ . '/../../../../tmp'), true);
1818
}
1919

2020
/**
@@ -76,6 +76,38 @@ public function testRule(): void
7676
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::count() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
7777
45,
7878
],
79+
[
80+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
81+
54,
82+
],
83+
[
84+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.',
85+
55,
86+
],
87+
[
88+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.',
89+
56,
90+
],
91+
[
92+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
93+
56,
94+
],
95+
[
96+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
97+
64,
98+
],
99+
[
100+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.',
101+
65,
102+
],
103+
[
104+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.',
105+
66,
106+
],
107+
[
108+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
109+
66,
110+
],
79111
]);
80112
}
81113

‎tests/Rules/Doctrine/ORM/RepositoryMethodCallRuleWithoutObjectManagerLoaderTest.php‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class RepositoryMethodCallRuleWithoutObjectManagerLoaderTest extends RuleTestCas
1414

1515
protected function getRule(): Rule
1616
{
17-
return new RepositoryMethodCallRule(new ObjectMetadataResolver(null, __DIR__ . '/../../../../tmp'));
17+
return new RepositoryMethodCallRule(new ObjectMetadataResolver(null, __DIR__ . '/../../../../tmp'), true);
1818
}
1919

2020
/**
@@ -76,6 +76,38 @@ public function testRule(): void
7676
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::count() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
7777
45,
7878
],
79+
[
80+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
81+
54,
82+
],
83+
[
84+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.',
85+
55,
86+
],
87+
[
88+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.',
89+
56,
90+
],
91+
[
92+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
93+
56,
94+
],
95+
[
96+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
97+
64,
98+
],
99+
[
100+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.',
101+
65,
102+
],
103+
[
104+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $nonexistent.',
105+
66,
106+
],
107+
[
108+
'Call to method Doctrine\ORM\EntityRepository<PHPStan\Rules\Doctrine\ORM\MyEntity>::findOneBy() - entity PHPStan\Rules\Doctrine\ORM\MyEntity does not have a field named $transient.',
109+
66,
110+
],
79111
]);
80112
}
81113

‎tests/Rules/Doctrine/ORM/data/repository-findBy-etc.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,25 @@ public function doCountBy(): void
4545
$entityRepository->count(['nonexistent' => 'test', 'transient' => 'test']);
4646
}
4747

48+
49+
public function doFindByOrder(): void
50+
{
51+
$entityRepository = $this->entityManager->getRepository(MyEntity::class);
52+
$entityRepository->findBy([], ['id' => 'ASC']);
53+
$entityRepository->findBy([], ['title' => 'DESC']);
54+
$entityRepository->findBy([], ['transient' => 'ASC']);
55+
$entityRepository->findBy([], ['nonexistent' => 'DESC']);
56+
$entityRepository->findBy([], ['nonexistent' => 'ASC', 'transient' => 'DESC']);
57+
}
58+
59+
public function doFindOneByOrder(): void
60+
{
61+
$entityRepository = $this->entityManager->getRepository(MyEntity::class);
62+
$entityRepository->findOneBy([], ['id' => 'ASC']);
63+
$entityRepository->findOneBy([], ['title' => 'DESC']);
64+
$entityRepository->findOneBy([], ['transient' => 'ASC']);
65+
$entityRepository->findOneBy([], ['nonexistent' => 'DESC']);
66+
$entityRepository->findOneBy([], ['nonexistent' => 'ASC', 'transient' => 'DESC']);
67+
}
68+
4869
}

0 commit comments

Comments
 (0)