Skip to content

Warn when queries cannot be resolved unexpectedly #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 19, 2023
Merged

Warn when queries cannot be resolved unexpectedly #508

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ad62a5c
Warn when queries cannot be resolved unexpectedly
hemberger Feb 13, 2023
b2a7620
Update src/UnresolvableQueryStringTypeException.php
hemberger Feb 16, 2023
940112b
fixup: refactor early return into QueryReflection::isResolvable
hemberger Feb 16, 2023
9be05c9
Merge branch 'main' into issue-504
staabm Feb 19, 2023
a706a5f
cs
staabm Feb 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/QueryReflection/QuerySimulation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;
use staabm\PHPStanDba\DbaException;
use staabm\PHPStanDba\UnresolvableQueryException;
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;

/**
* @internal
Expand All @@ -30,7 +31,7 @@ final class QuerySimulation
private const DATE_FORMAT = 'Y-m-d';

/**
* @throws UnresolvableQueryException
* @throws \staabm\PHPStanDba\UnresolvableQueryException
*/
public static function simulateParamValueType(Type $paramType, bool $preparedParam): ?string
{
Expand Down Expand Up @@ -94,6 +95,10 @@ public static function simulateParamValueType(Type $paramType, bool $preparedPar
}

// plain string types can contain anything.. we cannot reason about it
if (QueryReflection::getRuntimeConfiguration()->isDebugEnabled()) {
throw new UnresolvableQueryStringTypeException('Cannot resolve query with variable type: '.$paramType->describe(VerbosityLevel::precise()));
}

return null;
}

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

return null;
Expand Down
8 changes: 4 additions & 4 deletions src/Rules/PdoStatementExecuteMethodRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\MixedType;
use PHPStan\Type\StringType;
use staabm\PHPStanDba\PdoReflection\PdoStatementReflection;
use staabm\PHPStanDba\QueryReflection\PlaceholderValidation;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
Expand Down Expand Up @@ -67,7 +67,7 @@ private function checkErrors(MethodReflection $methodReflection, MethodCall $met
if (null === $queryExpr) {
return [];
}
if ($scope->getType($queryExpr) instanceof MixedType) {
if ($scope->getType($queryExpr)->isSuperTypeOf(new StringType())->yes()) {
return [];
}

Expand Down Expand Up @@ -98,7 +98,7 @@ private function checkErrors(MethodReflection $methodReflection, MethodCall $met
$parameters = $queryReflection->resolveParameters($parameterTypes) ?? [];
} catch (UnresolvableQueryException $exception) {
return [
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($methodCall->getLine())->build(),
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($methodCall->getLine())->build(),
];
}

Expand All @@ -111,7 +111,7 @@ private function checkErrors(MethodReflection $methodReflection, MethodCall $met
}
} catch (UnresolvableQueryException $exception) {
return [
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($methodCall->getLine())->build(),
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($methodCall->getLine())->build(),
];
}

Expand Down
8 changes: 4 additions & 4 deletions src/Rules/QueryPlanAnalyzerRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\MixedType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\Tests\QueryPlanAnalyzerRuleTest;
use staabm\PHPStanDba\UnresolvableQueryException;
Expand Down Expand Up @@ -94,15 +94,15 @@ public function processNode(Node $callLike, Scope $scope): array
return [];
}

if ($scope->getType($args[$queryArgPosition]->value) instanceof MixedType) {
if ($scope->getType($args[$queryArgPosition]->value)->isSuperTypeOf(new StringType())->yes()) {
return [];
}

try {
return $this->analyze($callLike, $scope);
} catch (UnresolvableQueryException $exception) {
return [
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($callLike->getLine())->build(),
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($callLike->getLine())->build(),
];
}
}
Expand All @@ -126,7 +126,7 @@ private function analyze(CallLike $callLike, Scope $scope): array

$queryExpr = $args[0]->value;

if ($scope->getType($queryExpr) instanceof MixedType) {
if ($scope->getType($queryExpr)->isSuperTypeOf(new StringType())->yes()) {
return [];
}

Expand Down
8 changes: 4 additions & 4 deletions src/Rules/SyntaxErrorInPreparedStatementMethodRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\MixedType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use staabm\PHPStanDba\QueryReflection\PlaceholderValidation;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\UnresolvableQueryException;
Expand Down Expand Up @@ -102,7 +102,7 @@ private function checkErrors(CallLike $callLike, Scope $scope): array

$queryExpr = $args[0]->value;

if ($scope->getType($queryExpr) instanceof MixedType) {
if ($scope->getType($queryExpr)->isSuperTypeOf(new StringType())->yes()) {
return [];
}

Expand All @@ -115,7 +115,7 @@ private function checkErrors(CallLike $callLike, Scope $scope): array
$parameters = $queryReflection->resolveParameters($parameterTypes) ?? [];
} catch (UnresolvableQueryException $exception) {
return [
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($callLike->getLine())->build(),
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($callLike->getLine())->build(),
];
}
}
Expand Down Expand Up @@ -152,7 +152,7 @@ private function checkErrors(CallLike $callLike, Scope $scope): array
return $ruleErrors;
} catch (UnresolvableQueryException $exception) {
return [
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($callLike->getLine())->build(),
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($callLike->getLine())->build(),
];
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/Rules/SyntaxErrorInQueryFunctionRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\MixedType;
use PHPStan\Type\StringType;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\Tests\SyntaxErrorInQueryFunctionRuleTest;
use staabm\PHPStanDba\UnresolvableQueryException;
Expand Down Expand Up @@ -85,7 +85,7 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

if ($scope->getType($args[$queryArgPosition]->value) instanceof MixedType) {
if ($scope->getType($args[$queryArgPosition]->value)->isSuperTypeOf(new StringType())->yes()) {
return [];
}

Expand All @@ -101,7 +101,7 @@ public function processNode(Node $node, Scope $scope): array
}
} catch (UnresolvableQueryException $exception) {
return [
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($node->getLine())->build(),
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($node->getLine())->build(),
];
}

Expand Down
6 changes: 3 additions & 3 deletions src/Rules/SyntaxErrorInQueryMethodRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\MixedType;
use PHPStan\Type\StringType;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\UnresolvableQueryException;

Expand Down Expand Up @@ -79,7 +79,7 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

if ($scope->getType($args[$queryArgPosition]->value) instanceof MixedType) {
if ($scope->getType($args[$queryArgPosition]->value)->isSuperTypeOf(new StringType())->yes()) {
Copy link
Owner

@staabm staabm Feb 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figure this one is hard to read - especially one does not immediately see that Mixed is contained.

wdyt about adding a new method on QueryReflection->isResolvable(Type):bool which can be asked about a given type is resolvable/analyzable? this would also have the benefit that custom implemented rules can re-use the logic without re-implementing stuff over and over.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a good suggestion. I've pushed a fix that hopefully addresses this request. Let me know what you think.

return [];
}

Expand All @@ -96,7 +96,7 @@ public function processNode(Node $node, Scope $scope): array
}
} catch (UnresolvableQueryException $exception) {
return [
RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($node->getLine())->build(),
RuleErrorBuilder::message($exception->asRuleMessage())->tip($exception::getTip())->line($node->getLine())->build(),
];
}

Expand Down
7 changes: 5 additions & 2 deletions src/UnresolvableQueryException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

namespace staabm\PHPStanDba;

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

public function asRuleMessage(): string
{
Expand Down
11 changes: 11 additions & 0 deletions src/UnresolvableQueryMixedTypeException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace staabm\PHPStanDba;

final class UnresolvableQueryMixedTypeException extends UnresolvableQueryException
{
public static function getTip(): string
{
return 'Make sure all variables involved have a non-mixed type and array-types are specified.';
}
}
11 changes: 11 additions & 0 deletions src/UnresolvableQueryStringTypeException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace staabm\PHPStanDba;

final class UnresolvableQueryStringTypeException extends UnresolvableQueryException
{
public static function getTip(): string
{
return 'Consider replacing variable strings with prepared statements or @phpstandba-inference-placeholder.';
}
}
19 changes: 18 additions & 1 deletion tests/rules/QueryPlanAnalyzerRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use PHPStan\Testing\RuleTestCase;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\Rules\QueryPlanAnalyzerRule;
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;

/**
* @extends RuleTestCase<QueryPlanAnalyzerRule>
Expand Down Expand Up @@ -137,7 +139,22 @@ public function testNotUsingIndexInDebugMode(): void
[
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
61,
'Make sure all variables involved have a non-mixed type and array-types are specified.',
UnresolvableQueryMixedTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
67,
UnresolvableQueryStringTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
70,
UnresolvableQueryStringTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
73,
UnresolvableQueryStringTypeException::getTip(),
],
]);
}
Expand Down
12 changes: 9 additions & 3 deletions tests/rules/UnresolvablePdoStatementRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
use PHPStan\Testing\RuleTestCase;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\Rules\PdoStatementExecuteMethodRule;
use staabm\PHPStanDba\UnresolvableQueryException;
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;

/**
* @extends RuleTestCase<PdoStatementExecuteMethodRule>
Expand Down Expand Up @@ -43,12 +44,17 @@ public function testSyntaxErrorInQueryRule(): void
[
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
13,
UnresolvableQueryException::RULE_TIP,
UnresolvableQueryMixedTypeException::getTip(),
],
[
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
17,
UnresolvableQueryException::RULE_TIP,
UnresolvableQueryMixedTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
54,
UnresolvableQueryStringTypeException::getTip(),
],
]);
}
Expand Down
10 changes: 8 additions & 2 deletions tests/rules/UnresolvablePreparedStatementRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
use PHPStan\Testing\RuleTestCase;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\Rules\SyntaxErrorInPreparedStatementMethodRule;
use staabm\PHPStanDba\UnresolvableQueryException;
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;

/**
* @extends RuleTestCase<SyntaxErrorInPreparedStatementMethodRule>
Expand Down Expand Up @@ -43,7 +44,12 @@ public function testSyntaxErrorInQueryRule(): void
[
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
11,
UnresolvableQueryException::RULE_TIP,
UnresolvableQueryMixedTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
30,
UnresolvableQueryStringTypeException::getTip(),
],
]);
}
Expand Down
17 changes: 14 additions & 3 deletions tests/rules/UnresolvableQueryFunctionRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
use PHPStan\Testing\RuleTestCase;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\Rules\SyntaxErrorInQueryFunctionRule;
use staabm\PHPStanDba\UnresolvableQueryException;
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;

/**
* @extends RuleTestCase<SyntaxErrorInQueryFunctionRule>
Expand Down Expand Up @@ -43,12 +44,22 @@ public function testSyntaxErrorInQueryRule(): void
[
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
9,
UnresolvableQueryException::RULE_TIP,
UnresolvableQueryMixedTypeException::getTip(),
],
[
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
15,
UnresolvableQueryException::RULE_TIP,
UnresolvableQueryMixedTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
32,
UnresolvableQueryStringTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
37,
UnresolvableQueryStringTypeException::getTip(),
],
]);
}
Expand Down
17 changes: 14 additions & 3 deletions tests/rules/UnresolvableQueryMethodRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
use PHPStan\Testing\RuleTestCase;
use staabm\PHPStanDba\QueryReflection\QueryReflection;
use staabm\PHPStanDba\Rules\SyntaxErrorInQueryMethodRule;
use staabm\PHPStanDba\UnresolvableQueryException;
use staabm\PHPStanDba\UnresolvableQueryMixedTypeException;
use staabm\PHPStanDba\UnresolvableQueryStringTypeException;

/**
* @extends RuleTestCase<SyntaxErrorInQueryMethodRule>
Expand Down Expand Up @@ -43,12 +44,22 @@ public function testSyntaxErrorInQueryRule(): void
[
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
11,
UnresolvableQueryException::RULE_TIP,
UnresolvableQueryMixedTypeException::getTip(),
],
[
'Unresolvable Query: Cannot simulate parameter value for type: mixed.',
17,
UnresolvableQueryException::RULE_TIP,
UnresolvableQueryMixedTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
34,
UnresolvableQueryStringTypeException::getTip(),
],
[
'Unresolvable Query: Cannot resolve query with variable type: string.',
39,
UnresolvableQueryStringTypeException::getTip(),
],
]);
}
Expand Down
Loading