Skip to content

Commit 60fd493

Browse files
hembergerstaabm
andauthored
Add DoctrineKeyValueStyleRule (#529)
Co-authored-by: Markus Staab <[email protected]>
1 parent 3c2e2c0 commit 60fd493

File tree

10 files changed

+1312
-29
lines changed

10 files changed

+1312
-29
lines changed

.phpstan-dba-mysqli.cache

Lines changed: 403 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.phpstan-dba-pdo-mysql.cache

Lines changed: 403 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/rules.neon

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ services:
6262
- 'mysqli_query#1'
6363
- 'mysqli_execute_query#1'
6464

65+
-
66+
class: staabm\PHPStanDba\Rules\DoctrineKeyValueStyleRule
67+
tags: [phpstan.rules.rule]
68+
arguments:
69+
classMethods:
70+
- 'staabm\PHPStanDba\Tests\Fixture\Connection::assembleNoArrays'
71+
- 'staabm\PHPStanDba\Tests\Fixture\Connection::assembleOneArray#1'
72+
- 'staabm\PHPStanDba\Tests\Fixture\Connection::assembleTwoArrays#1,2'
73+
- 'Doctrine\DBAL\Connection::insert#1'
74+
- 'Doctrine\DBAL\Connection::delete#1'
75+
- 'Doctrine\DBAL\Connection::update#1,2'
76+
6577
-
6678
class: staabm\PHPStanDba\Rules\QueryPlanAnalyzerRule
6779
tags: [phpstan.rules.rule]

docs/rules.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,27 @@ services:
5252

5353
__the callable format is `funtionName#parameterIndex`, while the parameter-index defines the position of the query-string argument.__
5454

55+
## use `DoctrineKeyValueStyleRule` for your custom classes
56+
57+
Reuse the `DoctrineKeyValueStyleRule` within your PHPStan configuration to detect syntax errors in class methods that construct queries from table and column name variables, by registering a service:
58+
The rule is designed around doctrine's [data-retrieval-and-manipulation-api](https://www.doctrine-project.org/projects/doctrine-dbal/en/current/reference/data-retrieval-and-manipulation.html#insert).
59+
60+
```
61+
services:
62+
-
63+
class: staabm\PHPStanDba\Rules\DoctrineKeyValueStyleRule
64+
tags: [phpstan.rules.rule]
65+
arguments:
66+
classMethods:
67+
- 'My\Connection::truncate'
68+
- 'My\Connection::insert#1'
69+
- 'My\Connection::delete#1'
70+
- 'My\Connection::update#1,2'
71+
```
72+
73+
__the callable format is `class::method#arrayArgIndex,arrayArgIndex`. phpstan-dba assumes the method takes a table name as the 1st argument and any listed argument indices are arrays with column name keys and column value values.__
74+
75+
5576
## use `QueryPlanAnalyzerRule` for your custom classes
5677

5778
Reuse the `QueryPlanAnalyzerRule` within your PHPStan configuration to detect syntax errors in queries, by registering a service:

src/QueryReflection/QueryReflection.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ private function stringifyType(Type $type): Type
176176
return $type;
177177
}
178178

179-
private function getSchemaReflection(): SchemaReflection
179+
public function getSchemaReflection(): SchemaReflection
180180
{
181181
if (null === $this->schemaReflection) {
182182
$this->schemaReflection = new SchemaReflection(function ($queryString) {
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace staabm\PHPStanDba\Rules;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\CallLike;
9+
use PhpParser\Node\Expr\MethodCall;
10+
use PhpParser\Node\Expr\New_;
11+
use PhpParser\Node\Name\FullyQualified;
12+
use PHPStan\Analyser\Scope;
13+
use PHPStan\Rules\Rule;
14+
use PHPStan\Rules\RuleError;
15+
use PHPStan\Rules\RuleErrorBuilder;
16+
use PHPStan\ShouldNotHappenException;
17+
use PHPStan\Type\Constant\ConstantArrayType;
18+
use PHPStan\Type\Constant\ConstantStringType;
19+
use PHPStan\Type\ObjectType;
20+
use PHPStan\Type\VerbosityLevel;
21+
use staabm\PHPStanDba\QueryReflection\QueryReflection;
22+
23+
/**
24+
* @implements Rule<CallLike>
25+
*
26+
* @see DoctrineKeyValueStyleRuleTest
27+
*/
28+
final class DoctrineKeyValueStyleRule implements Rule
29+
{
30+
/**
31+
* @var array<array{string, string, list<int>}>
32+
*/
33+
private $classMethods;
34+
35+
/**
36+
* @var QueryReflection
37+
*/
38+
private $queryReflection;
39+
40+
/**
41+
* @param list<string> $classMethods
42+
*/
43+
public function __construct(array $classMethods)
44+
{
45+
$this->classMethods = [];
46+
foreach ($classMethods as $classMethod) {
47+
sscanf($classMethod, '%[^::]::%[^#]#%[0-9,]', $className, $methodName, $arrayArgPositions);
48+
if (! \is_string($className) || ! \is_string($methodName)) {
49+
throw new ShouldNotHappenException('Invalid classMethod definition');
50+
}
51+
if ($arrayArgPositions !== null) {
52+
$arrayArgPositions = array_map('intval', explode(',', strval($arrayArgPositions)));
53+
} else {
54+
$arrayArgPositions = [];
55+
}
56+
$this->classMethods[] = [$className, $methodName, $arrayArgPositions];
57+
}
58+
$this->queryReflection = new QueryReflection();
59+
}
60+
61+
public function getNodeType(): string
62+
{
63+
return CallLike::class;
64+
}
65+
66+
/**
67+
* @return RuleError[]
68+
*/
69+
public function processNode(Node $callLike, Scope $scope): array
70+
{
71+
if ($callLike instanceof MethodCall) {
72+
if (! $callLike->name instanceof Node\Identifier) {
73+
return [];
74+
}
75+
76+
$methodReflection = $scope->getMethodReflection($scope->getType($callLike->var), $callLike->name->toString());
77+
} elseif ($callLike instanceof New_) {
78+
if (! $callLike->class instanceof FullyQualified) {
79+
return [];
80+
}
81+
$methodReflection = $scope->getMethodReflection(new ObjectType($callLike->class->toCodeString()), '__construct');
82+
} else {
83+
return [];
84+
}
85+
86+
if (null === $methodReflection) {
87+
return [];
88+
}
89+
90+
$unsupportedMethod = true;
91+
$arrayArgPositions = [];
92+
foreach ($this->classMethods as [$className, $methodName, $arrayArgPositionsConfig]) {
93+
if ($methodName === $methodReflection->getName() &&
94+
($methodReflection->getDeclaringClass()->getName() === $className || $methodReflection->getDeclaringClass()->isSubclassOf($className))) {
95+
$arrayArgPositions = $arrayArgPositionsConfig;
96+
$unsupportedMethod = false;
97+
break;
98+
}
99+
}
100+
101+
if ($unsupportedMethod) {
102+
return [];
103+
}
104+
105+
$args = $callLike->getArgs();
106+
107+
if (\count($args) < 1) {
108+
return [];
109+
}
110+
111+
$tableExpr = $args[0]->value;
112+
$tableType = $scope->getType($tableExpr);
113+
if (! $tableType instanceof ConstantStringType) {
114+
return [
115+
RuleErrorBuilder::message('Argument #0 expects a literal string, got ' . $tableType->describe(VerbosityLevel::precise()))->line($callLike->getLine())->build(),
116+
];
117+
}
118+
119+
$schemaReflection = $this->queryReflection->getSchemaReflection();
120+
121+
// Table name may be escaped with backticks
122+
$argTableName = trim($tableType->getValue(), '`');
123+
$table = $schemaReflection->getTable($argTableName);
124+
if (null === $table) {
125+
return [
126+
RuleErrorBuilder::message('Query error: Table "' . $argTableName . '" does not exist')->line($callLike->getLine())->build(),
127+
];
128+
}
129+
130+
// All array arguments should have table columns as keys
131+
$errors = [];
132+
foreach ($arrayArgPositions as $arrayArgPosition) {
133+
// If the argument doesn't exist, just skip it since we don't want
134+
// to error in case it has a default value
135+
if (! \array_key_exists($arrayArgPosition, $args)) {
136+
continue;
137+
}
138+
139+
$argType = $scope->getType($args[$arrayArgPosition]->value);
140+
if (! $argType instanceof ConstantArrayType) {
141+
$errors[] = 'Argument #' . $arrayArgPosition . ' is not a constant array, got ' . $argType->describe(VerbosityLevel::precise());
142+
continue;
143+
}
144+
145+
foreach ($argType->getKeyTypes() as $keyIndex => $keyType) {
146+
if (! $keyType instanceof ConstantStringType) {
147+
$errors[] = 'Element #' . $keyIndex . ' of argument #' . $arrayArgPosition . ' must have a string key, got ' . $keyType->describe(VerbosityLevel::precise());
148+
continue;
149+
}
150+
151+
// Column name may be escaped with backticks
152+
$argColumnName = trim($keyType->getValue(), '`');
153+
154+
$argColumn = null;
155+
foreach ($table->getColumns() as $column) {
156+
if ($argColumnName === $column->getName()) {
157+
$argColumn = $column;
158+
}
159+
}
160+
if (null === $argColumn) {
161+
$errors[] = 'Column "' . $table->getName() . '.' . $argColumnName . '" does not exist';
162+
continue;
163+
}
164+
165+
// Be a bit generous here by allowing column value types that
166+
// are *maybe* accepted. If we want to be strict, we can warn
167+
// unless they are *definitely* accepted.
168+
$valueType = $argType->getValueTypes()[$keyIndex];
169+
if ($argColumn->getType()->accepts($valueType, true)->no()) {
170+
$errors[] = 'Column "' . $table->getName() . '.' . $argColumnName . '" expects value type ' . $argColumn->getType()->describe(VerbosityLevel::precise()) . ', got type ' . $valueType->describe(VerbosityLevel::precise());
171+
}
172+
}
173+
}
174+
175+
$ruleErrors = [];
176+
foreach ($errors as $error) {
177+
$ruleErrors[] = RuleErrorBuilder::message('Query error: ' . $error)->line($callLike->getLine())->build();
178+
}
179+
return $ruleErrors;
180+
}
181+
}

tests/default/Fixture/Connection.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,16 @@ class Connection
99
public function preparedQuery(string $queryString, array $params)
1010
{
1111
}
12+
13+
public function assembleNoArrays(string $tableName)
14+
{
15+
}
16+
17+
public function assembleOneArray(string $tableName, $cols)
18+
{
19+
}
20+
21+
public function assembleTwoArrays(string $tableName, $cols1, $cols2)
22+
{
23+
}
1224
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace staabm\PHPStanDba\Tests;
6+
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Testing\RuleTestCase;
9+
use staabm\PHPStanDba\Rules\DoctrineKeyValueStyleRule;
10+
11+
/**
12+
* @extends RuleTestCase<DoctrineKeyValueStyleRule>
13+
*/
14+
class DoctrineKeyValueStyleRuleTest extends RuleTestCase
15+
{
16+
protected function getRule(): Rule
17+
{
18+
return self::getContainer()->getByType(DoctrineKeyValueStyleRule::class);
19+
}
20+
21+
public static function getAdditionalConfigFiles(): array
22+
{
23+
return [
24+
__DIR__ . '/../../config/dba.neon',
25+
];
26+
}
27+
28+
public function testSyntaxErrorInMethod(): void
29+
{
30+
$expectedErrors = [
31+
[
32+
'Argument #0 expects a literal string, got string',
33+
11,
34+
],
35+
[
36+
'Query error: Table "not_a_table" does not exist',
37+
16,
38+
],
39+
[
40+
'Query error: Argument #1 is not a constant array, got \'not_an_array\'',
41+
21,
42+
],
43+
[
44+
'Query error: Argument #2 is not a constant array, got \'not_an_array\'',
45+
26,
46+
],
47+
[
48+
'Query error: Argument #1 is not a constant array, got non-empty-array<string, \'foo\'>',
49+
31,
50+
],
51+
[
52+
'Query error: Element #0 of argument #1 must have a string key, got 42',
53+
36,
54+
],
55+
[
56+
'Query error: Column "ada.not_a_column" does not exist',
57+
41,
58+
],
59+
[
60+
'Query error: Column "ada.adaid" expects value type int<-32768, 32767>, got type string',
61+
46,
62+
],
63+
];
64+
65+
$this->analyse([__DIR__ . '/data/doctrine-key-value-style.php'], $expectedErrors);
66+
}
67+
}

0 commit comments

Comments
 (0)