Skip to content

Commit 81aaa9d

Browse files
hembergerstaabm
andauthored
Warn when queries cannot be resolved unexpectedly (#508)
Co-authored-by: Markus Staab <maggus.staab@googlemail.com>
1 parent 69396bf commit 81aaa9d

19 files changed

+177
-50
lines changed

src/QueryReflection/QueryReflection.php

+22
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use PhpParser\Node\Scalar\EncapsedStringPart;
1212
use PHPStan\Analyser\Scope;
1313
use PHPStan\ShouldNotHappenException;
14+
use PHPStan\TrinaryLogic;
1415
use PHPStan\Type\Accessory\AccessoryNumericStringType;
1516
use PHPStan\Type\Constant\ConstantArrayType;
1617
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
@@ -186,6 +187,27 @@ private function getSchemaReflection(): SchemaReflection
186187
return $this->schemaReflection;
187188
}
188189

190+
/**
191+
* Determine if a query will be resolvable.
192+
*
193+
* - If yes, the query is a literal string.
194+
* - If no, the query is a non-literal string or mixed type.
195+
* - If maybe, the query is neither of the two.
196+
*
197+
* We will typically skip processing of queries that return no, which are
198+
* likely part of a software abstraction layer that we know nothing about.
199+
*/
200+
public function isResolvable(Expr $queryExpr, Scope $scope): TrinaryLogic
201+
{
202+
$type = $scope->getType($queryExpr);
203+
if ($type->isLiteralString()->yes()) {
204+
return TrinaryLogic::createYes();
205+
}
206+
$isStringOrMixed = $type->isSuperTypeOf(new StringType());
207+
208+
return $isStringOrMixed->negate();
209+
}
210+
189211
/**
190212
* @return iterable<string>
191213
*

src/QueryReflection/QuerySimulation.php

+8-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
use PHPStan\Type\UnionType;
2121
use PHPStan\Type\VerbosityLevel;
2222
use staabm\PHPStanDba\DbaException;
23-
use staabm\PHPStanDba\UnresolvableQueryException;
23+
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
24+
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;
2425

2526
/**
2627
* @internal
@@ -30,7 +31,7 @@ final class QuerySimulation
3031
private const DATE_FORMAT = 'Y-m-d';
3132

3233
/**
33-
* @throws UnresolvableQueryException
34+
* @throws \staabm\PHPStanDba\UnresolvableQueryException
3435
*/
3536
public static function simulateParamValueType(Type $paramType, bool $preparedParam): ?string
3637
{
@@ -94,6 +95,10 @@ public static function simulateParamValueType(Type $paramType, bool $preparedPar
9495
}
9596

9697
// plain string types can contain anything.. we cannot reason about it
98+
if (QueryReflection::getRuntimeConfiguration()->isDebugEnabled()) {
99+
throw new UnresolvableQueryStringTypeException('Cannot resolve query with variable type: ' . $paramType->describe(VerbosityLevel::precise()));
100+
}
101+
97102
return null;
98103
}
99104

@@ -113,7 +118,7 @@ public static function simulateParamValueType(Type $paramType, bool $preparedPar
113118
// all types which we can't simulate and render a query unresolvable at analysis time
114119
if ($paramType instanceof MixedType || $paramType instanceof IntersectionType) {
115120
if (QueryReflection::getRuntimeConfiguration()->isDebugEnabled()) {
116-
throw new UnresolvableQueryException('Cannot simulate parameter value for type: ' . $paramType->describe(VerbosityLevel::precise()));
121+
throw new UnresolvableQueryMixedTypeException('Cannot simulate parameter value for type: ' . $paramType->describe(VerbosityLevel::precise()));
117122
}
118123

119124
return null;

src/Rules/PdoStatementExecuteMethodRule.php

+3-4
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use PHPStan\Type\Constant\ConstantArrayType;
1616
use PHPStan\Type\Constant\ConstantIntegerType;
1717
use PHPStan\Type\Constant\ConstantStringType;
18-
use PHPStan\Type\MixedType;
1918
use staabm\PHPStanDba\PdoReflection\PdoStatementReflection;
2019
use staabm\PHPStanDba\QueryReflection\PlaceholderValidation;
2120
use staabm\PHPStanDba\QueryReflection\QueryReflection;
@@ -67,7 +66,7 @@ private function checkErrors(MethodReflection $methodReflection, MethodCall $met
6766
if (null === $queryExpr) {
6867
return [];
6968
}
70-
if ($scope->getType($queryExpr) instanceof MixedType) {
69+
if ($queryReflection->isResolvable($queryExpr, $scope)->no()) {
7170
return [];
7271
}
7372

@@ -98,7 +97,7 @@ private function checkErrors(MethodReflection $methodReflection, MethodCall $met
9897
$parameters = $queryReflection->resolveParameters($parameterTypes) ?? [];
9998
} catch (UnresolvableQueryException $exception) {
10099
return [
101-
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($methodCall->getLine())->build(),
100+
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($methodCall->getLine())->build(),
102101
];
103102
}
104103

@@ -111,7 +110,7 @@ private function checkErrors(MethodReflection $methodReflection, MethodCall $met
111110
}
112111
} catch (UnresolvableQueryException $exception) {
113112
return [
114-
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($methodCall->getLine())->build(),
113+
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($methodCall->getLine())->build(),
115114
];
116115
}
117116

src/Rules/QueryPlanAnalyzerRule.php

+3-13
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use PHPStan\Rules\RuleError;
1515
use PHPStan\Rules\RuleErrorBuilder;
1616
use PHPStan\ShouldNotHappenException;
17-
use PHPStan\Type\MixedType;
1817
use PHPStan\Type\ObjectType;
1918
use staabm\PHPStanDba\QueryReflection\QueryReflection;
2019
use staabm\PHPStanDba\Tests\QueryPlanAnalyzerRuleTest;
@@ -89,20 +88,11 @@ public function processNode(Node $callLike, Scope $scope): array
8988
return [];
9089
}
9190

92-
$args = $callLike->getArgs();
93-
if (! \array_key_exists($queryArgPosition, $args)) {
94-
return [];
95-
}
96-
97-
if ($scope->getType($args[$queryArgPosition]->value) instanceof MixedType) {
98-
return [];
99-
}
100-
10191
try {
10292
return $this->analyze($callLike, $scope);
10393
} catch (UnresolvableQueryException $exception) {
10494
return [
105-
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($callLike->getLine())->build(),
95+
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($callLike->getLine())->build(),
10696
];
10797
}
10898
}
@@ -125,8 +115,9 @@ private function analyze(CallLike $callLike, Scope $scope): array
125115
}
126116

127117
$queryExpr = $args[0]->value;
118+
$queryReflection = new QueryReflection();
128119

129-
if ($scope->getType($queryExpr) instanceof MixedType) {
120+
if ($queryReflection->isResolvable($queryExpr, $scope)->no()) {
130121
return [];
131122
}
132123

@@ -136,7 +127,6 @@ private function analyze(CallLike $callLike, Scope $scope): array
136127
}
137128

138129
$ruleErrors = [];
139-
$queryReflection = new QueryReflection();
140130
$proposal = "\n\nConsider optimizing the query.\nIn some cases this is not a problem and this error should be ignored.";
141131

142132
foreach ($queryReflection->analyzeQueryPlan($scope, $queryExpr, $parameterTypes) as $queryPlanResult) {

src/Rules/SyntaxErrorInPreparedStatementMethodRule.php

+4-6
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use PHPStan\Rules\RuleError;
1515
use PHPStan\Rules\RuleErrorBuilder;
1616
use PHPStan\ShouldNotHappenException;
17-
use PHPStan\Type\MixedType;
1817
use PHPStan\Type\ObjectType;
1918
use staabm\PHPStanDba\QueryReflection\PlaceholderValidation;
2019
use staabm\PHPStanDba\QueryReflection\QueryReflection;
@@ -101,21 +100,20 @@ private function checkErrors(CallLike $callLike, Scope $scope): array
101100
}
102101

103102
$queryExpr = $args[0]->value;
103+
$queryReflection = new QueryReflection();
104104

105-
if ($scope->getType($queryExpr) instanceof MixedType) {
105+
if ($queryReflection->isResolvable($queryExpr, $scope)->no()) {
106106
return [];
107107
}
108108

109-
$queryReflection = new QueryReflection();
110-
111109
$parameters = null;
112110
if (\count($args) > 1) {
113111
$parameterTypes = $scope->getType($args[1]->value);
114112
try {
115113
$parameters = $queryReflection->resolveParameters($parameterTypes) ?? [];
116114
} catch (UnresolvableQueryException $exception) {
117115
return [
118-
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($callLike->getLine())->build(),
116+
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($callLike->getLine())->build(),
119117
];
120118
}
121119
}
@@ -152,7 +150,7 @@ private function checkErrors(CallLike $callLike, Scope $scope): array
152150
return $ruleErrors;
153151
} catch (UnresolvableQueryException $exception) {
154152
return [
155-
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($callLike->getLine())->build(),
153+
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($callLike->getLine())->build(),
156154
];
157155
}
158156
}

src/Rules/SyntaxErrorInQueryFunctionRule.php

+6-5
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use PHPStan\Rules\Rule;
1212
use PHPStan\Rules\RuleErrorBuilder;
1313
use PHPStan\ShouldNotHappenException;
14-
use PHPStan\Type\MixedType;
1514
use staabm\PHPStanDba\QueryReflection\QueryReflection;
1615
use staabm\PHPStanDba\Tests\SyntaxErrorInQueryFunctionRuleTest;
1716
use staabm\PHPStanDba\UnresolvableQueryException;
@@ -85,13 +84,15 @@ public function processNode(Node $node, Scope $scope): array
8584
return [];
8685
}
8786

88-
if ($scope->getType($args[$queryArgPosition]->value) instanceof MixedType) {
87+
$queryExpr = $args[$queryArgPosition]->value;
88+
$queryReflection = new QueryReflection();
89+
90+
if ($queryReflection->isResolvable($queryExpr, $scope)->no()) {
8991
return [];
9092
}
9193

92-
$queryReflection = new QueryReflection();
9394
try {
94-
foreach ($queryReflection->resolveQueryStrings($args[$queryArgPosition]->value, $scope) as $queryString) {
95+
foreach ($queryReflection->resolveQueryStrings($queryExpr, $scope) as $queryString) {
9596
$queryError = $queryReflection->validateQueryString($queryString);
9697
if (null !== $queryError) {
9798
return [
@@ -101,7 +102,7 @@ public function processNode(Node $node, Scope $scope): array
101102
}
102103
} catch (UnresolvableQueryException $exception) {
103104
return [
104-
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($node->getLine())->build(),
105+
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($node->getLine())->build(),
105106
];
106107
}
107108

src/Rules/SyntaxErrorInQueryMethodRule.php

+6-5
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use PHPStan\Rules\Rule;
1111
use PHPStan\Rules\RuleErrorBuilder;
1212
use PHPStan\ShouldNotHappenException;
13-
use PHPStan\Type\MixedType;
1413
use staabm\PHPStanDba\QueryReflection\QueryReflection;
1514
use staabm\PHPStanDba\UnresolvableQueryException;
1615

@@ -79,13 +78,15 @@ public function processNode(Node $node, Scope $scope): array
7978
return [];
8079
}
8180

82-
if ($scope->getType($args[$queryArgPosition]->value) instanceof MixedType) {
81+
$queryExpr = $args[$queryArgPosition]->value;
82+
$queryReflection = new QueryReflection();
83+
84+
if ($queryReflection->isResolvable($queryExpr, $scope)->no()) {
8385
return [];
8486
}
8587

8688
try {
87-
$queryReflection = new QueryReflection();
88-
$queryStrings = $queryReflection->resolveQueryStrings($args[$queryArgPosition]->value, $scope);
89+
$queryStrings = $queryReflection->resolveQueryStrings($queryExpr, $scope);
8990
foreach ($queryStrings as $queryString) {
9091
$queryError = $queryReflection->validateQueryString($queryString);
9192
if (null !== $queryError) {
@@ -96,7 +97,7 @@ public function processNode(Node $node, Scope $scope): array
9697
}
9798
} catch (UnresolvableQueryException $exception) {
9899
return [
99-
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($node->getLine())->build(),
100+
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($node->getLine())->build(),
100101
];
101102
}
102103

src/UnresolvableQueryException.php

+5-2
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44

55
namespace staabm\PHPStanDba;
66

7-
final class UnresolvableQueryException extends DbaException
7+
/**
8+
* @api
9+
*/
10+
abstract class UnresolvableQueryException extends DbaException
811
{
9-
public const RULE_TIP = 'Make sure all variables involved have a non-mixed type and array-types are specified.';
12+
abstract public static function getTip(): string;
1013

1114
public function asRuleMessage(): string
1215
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace staabm\PHPStanDba;
6+
7+
final class UnresolvableQueryMixedTypeException extends UnresolvableQueryException
8+
{
9+
public static function getTip(): string
10+
{
11+
return 'Make sure all variables involved have a non-mixed type and array-types are specified.';
12+
}
13+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace staabm\PHPStanDba;
6+
7+
final class UnresolvableQueryStringTypeException extends UnresolvableQueryException
8+
{
9+
public static function getTip(): string
10+
{
11+
return 'Consider replacing concatenated string-variables with prepared statements or @phpstandba-inference-placeholder.';
12+
}
13+
}

tests/rules/QueryPlanAnalyzerRuleTest.php

+18-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
use PHPStan\Testing\RuleTestCase;
99
use staabm\PHPStanDba\QueryReflection\QueryReflection;
1010
use staabm\PHPStanDba\Rules\QueryPlanAnalyzerRule;
11+
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
12+
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;
1113

1214
/**
1315
* @extends RuleTestCase<QueryPlanAnalyzerRule>
@@ -149,7 +151,22 @@ public function testNotUsingIndexInDebugMode(): void
149151
[
150152
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
151153
61,
152-
'Make sure all variables involved have a non-mixed type and array-types are specified.',
154+
UnresolvableQueryMixedTypeException::getTip(),
155+
],
156+
[
157+
'Unresolvable Query: Cannot resolve query with variable type: string.',
158+
67,
159+
UnresolvableQueryStringTypeException::getTip(),
160+
],
161+
[
162+
'Unresolvable Query: Cannot resolve query with variable type: string.',
163+
70,
164+
UnresolvableQueryStringTypeException::getTip(),
165+
],
166+
[
167+
'Unresolvable Query: Cannot resolve query with variable type: string.',
168+
73,
169+
UnresolvableQueryStringTypeException::getTip(),
153170
],
154171
]);
155172
}

tests/rules/UnresolvablePdoStatementRuleTest.php

+9-3
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
use PHPStan\Testing\RuleTestCase;
99
use staabm\PHPStanDba\QueryReflection\QueryReflection;
1010
use staabm\PHPStanDba\Rules\PdoStatementExecuteMethodRule;
11-
use staabm\PHPStanDba\UnresolvableQueryException;
11+
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
12+
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;
1213

1314
/**
1415
* @extends RuleTestCase<PdoStatementExecuteMethodRule>
@@ -45,12 +46,17 @@ public function testSyntaxErrorInQueryRule(): void
4546
[
4647
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
4748
13,
48-
UnresolvableQueryException::RULE_TIP,
49+
UnresolvableQueryMixedTypeException::getTip(),
4950
],
5051
[
5152
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
5253
17,
53-
UnresolvableQueryException::RULE_TIP,
54+
UnresolvableQueryMixedTypeException::getTip(),
55+
],
56+
[
57+
'Unresolvable Query: Cannot resolve query with variable type: string.',
58+
54,
59+
UnresolvableQueryStringTypeException::getTip(),
5460
],
5561
]);
5662
}

0 commit comments

Comments
 (0)