Skip to content

Commit b9c52b0

Browse files
staabmclxmstaab
andauthored
added PdoStatementExecuteErrorMethodRule (#58)
Co-authored-by: Markus Staab <m.staab@complex-it.de>
1 parent e7d7e58 commit b9c52b0

File tree

8 files changed

+284
-7
lines changed

8 files changed

+284
-7
lines changed

config/dba.neon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ includes:
33
- extensions.neon
44

55
services:
6+
-
7+
class: staabm\PHPStanDba\Rules\PdoStatementExecuteErrorMethodRule
8+
tags: [phpstan.rules.rule]
9+
610
-
711
class: staabm\PHPStanDba\Rules\SyntaxErrorInQueryMethodRule
812
tags: [phpstan.rules.rule]

src/QueryReflection/QueryReflection.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ public function resolvePreparedQueryString(Expr $queryExpr, Type $parameterTypes
8989
}
9090

9191
$parameters = $this->resolveParameters($parameterTypes);
92+
if (null === $parameters) {
93+
return null;
94+
}
9295

9396
return $this->replaceParameters($queryString, $parameters);
9497
}
@@ -154,7 +157,7 @@ private function getQueryType(string $query): ?string
154157
/**
155158
* @return array<string|int, scalar|null>
156159
*/
157-
private function resolveParameters(Type $parameterTypes): array
160+
public function resolveParameters(Type $parameterTypes): ?array
158161
{
159162
$parameters = [];
160163

@@ -179,9 +182,11 @@ private function resolveParameters(Type $parameterTypes): array
179182
}
180183
}
181184
}
185+
186+
return $parameters;
182187
}
183188

184-
return $parameters;
189+
return null;
185190
}
186191

187192
/**
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace staabm\PHPStanDba\Rules;
6+
7+
use PDOStatement;
8+
use PhpParser\Node;
9+
use PhpParser\Node\Expr\MethodCall;
10+
use PHPStan\Analyser\Scope;
11+
use PHPStan\Reflection\MethodReflection;
12+
use PHPStan\Rules\Rule;
13+
use PHPStan\Rules\RuleError;
14+
use PHPStan\Rules\RuleErrorBuilder;
15+
use PHPStan\ShouldNotHappenException;
16+
use staabm\PHPStanDba\PdoReflection\PdoStatementReflection;
17+
use staabm\PHPStanDba\QueryReflection\QueryReflection;
18+
19+
/**
20+
* @implements Rule<MethodCall>
21+
*
22+
* @see PdoStatementExecuteErrorMethodRuleTest
23+
*/
24+
final class PdoStatementExecuteErrorMethodRule implements Rule
25+
{
26+
public function getNodeType(): string
27+
{
28+
return MethodCall::class;
29+
}
30+
31+
public function processNode(Node $methodCall, Scope $scope): array
32+
{
33+
if (!$methodCall->name instanceof Node\Identifier) {
34+
return [];
35+
}
36+
37+
$methodReflection = $scope->getMethodReflection($scope->getType($methodCall->var), $methodCall->name->toString());
38+
if (null === $methodReflection) {
39+
return [];
40+
}
41+
42+
if (PdoStatement::class !== $methodReflection->getDeclaringClass()->getName()) {
43+
return [];
44+
}
45+
46+
if ('execute' !== $methodReflection->getName()) {
47+
return [];
48+
}
49+
50+
return $this->checkErrors($methodReflection, $methodCall, $scope);
51+
}
52+
53+
/**
54+
* @return RuleError[]
55+
*/
56+
private function checkErrors(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): array
57+
{
58+
$stmtReflection = new PdoStatementReflection();
59+
$queryExpr = $stmtReflection->findPrepareQueryStringExpression($methodReflection, $methodCall);
60+
61+
if (null === $queryExpr) {
62+
return [];
63+
}
64+
65+
$queryReflection = new QueryReflection();
66+
$queryString = $queryReflection->resolveQueryString($queryExpr, $scope);
67+
if (null === $queryString) {
68+
return [];
69+
}
70+
71+
$args = $methodCall->getArgs();
72+
$placeholderCount = $this->countPlaceholders($queryString);
73+
74+
if (0 === \count($args)) {
75+
if (0 === $placeholderCount) {
76+
return [];
77+
}
78+
79+
return [
80+
RuleErrorBuilder::message(sprintf('Query expects %s placeholders, but no values are given to execute().', $placeholderCount))->line($methodCall->getLine())->build(),
81+
];
82+
}
83+
84+
return $this->checkParameterValues($methodCall, $scope, $queryString, $placeholderCount);
85+
}
86+
87+
/**
88+
* @return RuleError[]
89+
*/
90+
private function checkParameterValues(MethodCall $methodCall, Scope $scope, string $queryString, int $placeholderCount): array
91+
{
92+
$queryReflection = new QueryReflection();
93+
$args = $methodCall->getArgs();
94+
95+
$parameterTypes = $scope->getType($args[0]->value);
96+
$parameters = $queryReflection->resolveParameters($parameterTypes);
97+
if (null === $parameters) {
98+
return [];
99+
}
100+
$parameterCount = \count($parameters);
101+
102+
if ($parameterCount !== $placeholderCount) {
103+
if (1 === $parameterCount) {
104+
return [
105+
RuleErrorBuilder::message(sprintf('Query expects %s placeholders, but %s value is given to execute().', $placeholderCount, $parameterCount))->line($methodCall->getLine())->build(),
106+
];
107+
}
108+
109+
return [
110+
RuleErrorBuilder::message(sprintf('Query expects %s placeholders, but %s values are given to execute().', $placeholderCount, $parameterCount))->line($methodCall->getLine())->build(),
111+
];
112+
}
113+
114+
$errors = [];
115+
$namedPlaceholders = $this->extractNamedPlaceholders($queryString);
116+
foreach ($namedPlaceholders as $namedPlaceholder) {
117+
if (!\array_key_exists($namedPlaceholder, $parameters)) {
118+
$errors[] = RuleErrorBuilder::message(sprintf('Query expects placeholder %s, but it is missing from values given to execute().', $namedPlaceholder))->line($methodCall->getLine())->build();
119+
}
120+
}
121+
122+
foreach ($parameters as $placeholderKey => $value) {
123+
if (!\in_array($placeholderKey, $namedPlaceholders)) {
124+
$errors[] = RuleErrorBuilder::message(sprintf('Value %s is given to execute(), but the query does not containt this placeholder.', $placeholderKey))->line($methodCall->getLine())->build();
125+
}
126+
}
127+
128+
return $errors;
129+
}
130+
131+
/**
132+
* @return 0|positive-int
133+
*/
134+
private function countPlaceholders(string $queryString): int
135+
{
136+
$numPlaceholders = substr_count($queryString, '?');
137+
138+
if (0 !== $numPlaceholders) {
139+
return $numPlaceholders;
140+
}
141+
142+
$numPlaceholders = preg_match_all('{:[a-z]+}', $queryString);
143+
if (false === $numPlaceholders || $numPlaceholders < 0) {
144+
throw new ShouldNotHappenException();
145+
}
146+
147+
return $numPlaceholders;
148+
}
149+
150+
/**
151+
* @return list<string>
152+
*/
153+
private function extractNamedPlaceholders(string $queryString): array
154+
{
155+
// pdo does not support mixing of named and '?' placeholders
156+
$numPlaceholders = substr_count($queryString, '?');
157+
158+
if (0 !== $numPlaceholders) {
159+
return [];
160+
}
161+
162+
if (preg_match_all('{:[a-z]+}', $queryString, $matches) > 0) {
163+
return $matches[0];
164+
}
165+
166+
return [];
167+
}
168+
}

src/Rules/SyntaxErrorInQueryFunctionRule.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@
1111
use PHPStan\Rules\Rule;
1212
use PHPStan\Rules\RuleErrorBuilder;
1313
use staabm\PHPStanDba\QueryReflection\QueryReflection;
14+
use staabm\PHPStanDba\Tests\SyntaxErrorInQueryFunctionRuleTest;
1415

1516
/**
1617
* @implements Rule<FuncCall>
18+
*
19+
* @see SyntaxErrorInQueryFunctionRuleTest
1720
*/
1821
final class SyntaxErrorInQueryFunctionRule implements Rule
1922
{

src/Rules/SyntaxErrorInQueryMethodRule.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
/**
1515
* @implements Rule<MethodCall>
16+
*
17+
* @see SyntaxErrorInQueryMethodRuleTest
1618
*/
1719
final class SyntaxErrorInQueryMethodRule implements Rule
1820
{
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
3+
namespace staabm\PHPStanDba\Tests;
4+
5+
use PHPStan\Rules\Rule;
6+
use staabm\PHPStanDba\Rules\PdoStatementExecuteErrorMethodRule;
7+
use Symplify\PHPStanExtensions\Testing\AbstractServiceAwareRuleTestCase;
8+
9+
/**
10+
* @extends AbstractServiceAwareRuleTestCase<PdoStatementExecuteErrorMethodRule>
11+
*/
12+
class PdoStatementExecuteErrorMethodRuleTest extends AbstractServiceAwareRuleTestCase
13+
{
14+
protected function getRule(): Rule
15+
{
16+
return $this->getRuleFromConfig(PdoStatementExecuteErrorMethodRule::class, __DIR__.'/../config/dba.neon');
17+
}
18+
19+
public function testSyntaxErrorInQueryRule(): void
20+
{
21+
require_once __DIR__.'/data/pdo-stmt-execute-error.php';
22+
23+
$this->analyse([__DIR__.'/data/pdo-stmt-execute-error.php'], [
24+
[
25+
'Query expects placeholder :adaid, but it is missing from values given to execute().',
26+
12,
27+
],
28+
[
29+
'Value :wrongParamName is given to execute(), but the query does not containt this placeholder.',
30+
12,
31+
],
32+
[
33+
'Query expects placeholder :adaid, but it is missing from values given to execute().',
34+
15,
35+
],
36+
[
37+
'Value :wrongParamName is given to execute(), but the query does not containt this placeholder.',
38+
15,
39+
],
40+
/*
41+
[
42+
'Query expects placeholder :adaid, but it is missing from values given to execute().',
43+
18,
44+
],
45+
[
46+
'Value :wrongParamValue is given to execute(), but the query does not containt this placeholder.',
47+
18,
48+
],
49+
*/
50+
[
51+
'Query expects 1 placeholders, but no values are given to execute().',
52+
21,
53+
],
54+
[
55+
'Query expects 2 placeholders, but 1 value is given to execute().',
56+
24,
57+
],
58+
[
59+
'Query expects 2 placeholders, but 1 value is given to execute().',
60+
27,
61+
],
62+
[
63+
'Query expects 2 placeholders, but 1 value is given to execute().',
64+
30,
65+
],
66+
]);
67+
}
68+
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace PdoStatementExecuteErrorMethodRuleTest;
4+
5+
use PDO;
6+
7+
class Foo
8+
{
9+
public function errors(PDO $pdo)
10+
{
11+
$stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid');
12+
$stmt->execute(['wrongParamName' => 1]);
13+
14+
$stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid');
15+
$stmt->execute([':wrongParamName' => 1]);
16+
17+
// $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid');
18+
// $stmt->execute([':adaid' => 'hello world']); // wrong param type
19+
20+
$stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid');
21+
$stmt->execute(); // missing parameter
22+
23+
$stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = ? and email = ?');
24+
$stmt->execute([1]); // wrong number of parameters
25+
26+
$stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid and email = :email');
27+
$stmt->execute(['adaid' => 1]); // wrong number of parameters
28+
29+
$stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid and email = :email');
30+
$stmt->execute([':email' => 'email@example.org']); // wrong number of parameters
31+
}
32+
}

tests/data/pdo-stmt-execute.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,5 @@ public function errors(PDO $pdo)
5656
assertType('PDOStatement', $stmt);
5757
$stmt->execute(); // missing parameter
5858
assertType('PDOStatement', $stmt);
59-
60-
$stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid');
61-
assertType('PDOStatement', $stmt);
62-
$stmt->execute([]); // missing parameter
63-
assertType('PDOStatement', $stmt);
6459
}
6560
}

0 commit comments

Comments
 (0)