Skip to content

prevent wrong type inference on queries containing string #198

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 3 commits into from
Jan 23, 2022
Merged

prevent wrong type inference on queries containing string #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7434b40
added failling reference test
clxmstaab Jan 23, 2022
25290d3
fix
clxmstaab Jan 23, 2022
fad9309
cache
clxmstaab Jan 23, 2022
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
52 changes: 52 additions & 0 deletions .phpstan-dba.cache
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 52 additions & 0 deletions .phpunit-phpstan-dba.cache
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 10 additions & 10 deletions src/QueryReflection/QueryReflection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public function resolvePreparedQueryStrings(Expr $queryExpr, Type $parameterType
*/
public function resolvePreparedQueryString(Expr $queryExpr, Type $parameterTypes, Scope $scope): ?string
{
$queryString = $this->resolveQueryExpr($queryExpr, $scope, true);
$queryString = $this->resolveQueryExpr($queryExpr, $scope);

if (null === $queryString) {
return null;
Expand Down Expand Up @@ -134,7 +134,7 @@ public function resolveQueryStrings(Expr $queryExpr, Scope $scope): iterable
return;
}

$queryString = $this->resolveQueryExpr($queryExpr, $scope, false);
$queryString = $this->resolveQueryExpr($queryExpr, $scope);
if (null !== $queryString) {
yield $queryString;
}
Expand All @@ -145,37 +145,37 @@ public function resolveQueryStrings(Expr $queryExpr, Scope $scope): iterable
*/
public function resolveQueryString(Expr $queryExpr, Scope $scope): ?string
{
return $this->resolveQueryExpr($queryExpr, $scope, false);
return $this->resolveQueryExpr($queryExpr, $scope);
}

/**
* @throws UnresolvableQueryException
*/
private function resolveQueryExpr(Expr $queryExpr, Scope $scope, bool $preparedContext): ?string
private function resolveQueryExpr(Expr $queryExpr, Scope $scope): ?string
{
if ($queryExpr instanceof Expr\Variable) {
$finder = new ExpressionFinder();
$queryStringExpr = $finder->findQueryStringExpression($queryExpr);

if (null !== $queryStringExpr) {
return $this->resolveQueryStringExpr($queryStringExpr, $scope, $preparedContext);
return $this->resolveQueryStringExpr($queryStringExpr, $scope);
}
}

return $this->resolveQueryStringExpr($queryExpr, $scope, $preparedContext);
return $this->resolveQueryStringExpr($queryExpr, $scope);
}

/**
* @throws UnresolvableQueryException
*/
private function resolveQueryStringExpr(Expr $queryExpr, Scope $scope, bool $preparedContext): ?string
private function resolveQueryStringExpr(Expr $queryExpr, Scope $scope): ?string
{
if ($queryExpr instanceof Concat) {
$left = $queryExpr->left;
$right = $queryExpr->right;

$leftString = $this->resolveQueryStringExpr($left, $scope, $preparedContext);
$rightString = $this->resolveQueryStringExpr($right, $scope, $preparedContext);
$leftString = $this->resolveQueryStringExpr($left, $scope);
$rightString = $this->resolveQueryStringExpr($right, $scope);

if (null === $leftString || null === $rightString) {
return null;
Expand All @@ -186,7 +186,7 @@ private function resolveQueryStringExpr(Expr $queryExpr, Scope $scope, bool $pre

$type = $scope->getType($queryExpr);

return QuerySimulation::simulateParamValueType($type, $preparedContext);
return QuerySimulation::simulateParamValueType($type, false);
}

public static function getQueryType(string $query): ?string
Expand Down
16 changes: 16 additions & 0 deletions tests/data/pdo-prepare.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,20 @@ public function specifiedArray(PDO $pdo, array $idsToUpdate, string $time)
]);
assertType('PDOStatement<array{adaid: int<0, 4294967295>, 0: int<0, 4294967295>}>', $stmt);
}

public function noInferenceOnBug196(PDO $pdo, array $minorPhpVersions, \DateTimeImmutable $updateDate)
{
$sumQueries = [];
$dataPointDate = $updateDate->format('Ymd');
foreach ($minorPhpVersions as $index => $version) {
$sumQueries[] = 'SUM(DATA->\'$."'.$version.'"."'.$dataPointDate.'"\')';
}
$stmt = $pdo->prepare(
'SELECT '.implode(', ', $sumQueries).' FROM ada WHERE adaid = :package'
);
$stmt->execute(['package' => 'abc']);
// this query is too dynamic for beeing analyzed.
// make sure we don't infer a wrong type.
assertType('PDOStatement', $stmt);
}
}