Skip to content

Syntax Errors for PDO Prepared Statements not supported #760

@realugi

Description

@realugi

Currently, no syntax error is reported in the following case (phpstan-dba v0.4):

$id = $_GET['id'];
$pdo = new PDO(...);

$stmt = $pdo->prepare("select * from cstomer where id = ?");
$stmt->execute([$id]);

I misspelled the customer table in the above example.

The concrete problem here seems to be, that phpstan-dba needs both the query string and placeholder array to be supplied to a single method/function at the same time. When doing it this way, the reflection code can analyze both the query string and placeholder array by simulating the query with the given placeholder array (the types of the placeholder array, to be precise).
I have infered this from the staabm\PHPStanDba\Rules\SyntaxErrorInPreparedStatementMethodRule rule. Please correct me if I am wrong.

However, the PDO API demands that the query string be supplied to the prepare method of the PDO class, and that the placeholder array be supplied to the execute method of the PDOStatement class (which is returned by PDO::prepare). This fact makes analyzing the PDO API pattern very complicated and non-trivial.

Fortunately, there already exists a rule which implements discovering the prepare statement given an execute statement. This rule is: staabm\PHPStanDba\Rules\PdoStatementExecuteMethodRule.
Unfortunately, it currently only supports placeholder validation, but not syntax error checking.

Quick Solution

Add the following syntax-checking code from staabm\PHPStanDba\Rules\SyntaxErrorInPreparedStatementMethodRule:

$queryStrings = $queryReflection->resolvePreparedQueryStrings($queryExpr, $parameterTypes, $scope);

foreach ($queryStrings as $queryString) {
$queryError = $queryReflection->validateQueryString($queryString);
if (null !== $queryError) {
$error = $queryError->asRuleMessage();
$errors[$error] = $error;
}
}

into the staabm\PHPStanDba\Rules\PdoStatementExecuteMethodRule rule after line 105:
$errors = [];
$placeholderValidation = new PlaceholderValidation();
foreach ($placeholderValidation->checkQuery($queryExpr, $scope, $parameters) as $error) {
// make error messages unique
$errors[$error] = $error;
}

I have tested this locally (only very minimally) and it seems to work and report a syntax error for the above case.

Another (more complicated) Solution

Another idea, that is floating in my mind, would be to create a PDO and PDOStatement stub.
The PDOStatement stub would contain a template parameter (@template T of literal-string), which stores the constant type of the sql query string.
The PDO stub would modify the prepare method signature (and friends) to accept the sql query string as a template parameter (@template T of literal-string) and return PDOStatement<T>.
Then, when analyzing the execute method of PDOStatement<T>, one could extract the T type (which would be a constant string) and check the placeholder array against that. There would be no need to discover the prepare method in this way.

There may be other implications to this approach and I am not even sure if this would really work, just my raw thoughts.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions