diff --git a/src/QueryReflection/PlaceholderValidation.php b/src/QueryReflection/PlaceholderValidation.php index f542a7987..ca484ddcb 100644 --- a/src/QueryReflection/PlaceholderValidation.php +++ b/src/QueryReflection/PlaceholderValidation.php @@ -26,7 +26,7 @@ public function checkErrors(string $queryString, array $parameters): iterable $placeholderExpectation = sprintf('Query expects %s placeholders', $placeholderCount); } - yield sprintf($placeholderExpectation.', but no values are given to execute().', $placeholderCount); + yield sprintf($placeholderExpectation.', but no values are given.', $placeholderCount); return; } @@ -51,9 +51,9 @@ private function checkParameterValues(string $queryString, array $parameters, in $placeholderExpectation = sprintf('Query expects %s placeholders', $placeholderCount); } - $parameterActual = sprintf('but %s value is given to execute()', $parameterCount); + $parameterActual = sprintf('but %s value is given', $parameterCount); if ($parameterCount > 1) { - $parameterActual = sprintf('but %s values are given to execute()', $parameterCount); + $parameterActual = sprintf('but %s values are given', $parameterCount); } yield $placeholderExpectation.', '.$parameterActual.'.'; @@ -62,17 +62,18 @@ private function checkParameterValues(string $queryString, array $parameters, in } $namedPlaceholders = $queryReflection->extractNamedPlaceholders($queryString); - if (\count($namedPlaceholders) > 0) { - foreach ($namedPlaceholders as $namedPlaceholder) { - if (!\array_key_exists($namedPlaceholder, $parameters)) { - yield sprintf('Query expects placeholder %s, but it is missing from values given to execute().', $namedPlaceholder); - } + foreach ($namedPlaceholders as $namedPlaceholder) { + if (!\array_key_exists($namedPlaceholder, $parameters)) { + yield sprintf('Query expects placeholder %s, but it is missing from values given.', $namedPlaceholder); } + } - foreach ($parameters as $placeholderKey => $value) { - if (!\in_array($placeholderKey, $namedPlaceholders)) { - yield sprintf('Value %s is given to execute(), but the query does not contain this placeholder.', $placeholderKey); - } + foreach ($parameters as $placeholderKey => $value) { + if (\is_int($placeholderKey)) { + continue; + } + if (!\in_array($placeholderKey, $namedPlaceholders)) { + yield sprintf('Value %s is given, but the query does not contain this placeholder.', $placeholderKey); } } } diff --git a/src/Rules/SyntaxErrorInPreparedStatementMethodRule.php b/src/Rules/SyntaxErrorInPreparedStatementMethodRule.php index 09096cb0f..9ee4bed4f 100644 --- a/src/Rules/SyntaxErrorInPreparedStatementMethodRule.php +++ b/src/Rules/SyntaxErrorInPreparedStatementMethodRule.php @@ -14,6 +14,7 @@ use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\ObjectType; +use staabm\PHPStanDba\QueryReflection\PlaceholderValidation; use staabm\PHPStanDba\QueryReflection\QueryReflection; /** @@ -96,16 +97,29 @@ private function checkErrors(CallLike $callLike, Scope $scope): array $parameterTypes = $scope->getType($args[1]->value); $queryReflection = new QueryReflection(); + $parameters = $queryReflection->resolveParameters($parameterTypes) ?? []; + + $errors = []; + $placeholderReflection = new PlaceholderValidation(); foreach ($queryReflection->resolvePreparedQueryStrings($queryExpr, $parameterTypes, $scope) as $queryString) { - $error = $queryReflection->validateQueryString($queryString); + $queryError = $queryReflection->validateQueryString($queryString); + if (null !== $queryError) { + $error = 'Query error: '.$queryError->getMessage().' ('.$queryError->getCode().').'; + $errors[$error] = $error; + } + } - if (null !== $error) { - return [ - RuleErrorBuilder::message('Query error: '.$error->getMessage().' ('.$error->getCode().').')->line($callLike->getLine())->build(), - ]; + foreach ($queryReflection->resolveQueryStrings($queryExpr, $scope) as $queryString) { + foreach ($placeholderReflection->checkErrors($queryString, $parameters) as $error) { + $errors[$error] = $error; } } - return []; + $ruleErrors = []; + foreach ($errors as $error) { + $ruleErrors[] = RuleErrorBuilder::message($error)->line($callLike->getLine())->build(); + } + + return $ruleErrors; } } diff --git a/tests/PdoStatementExecuteMethodRuleTest.php b/tests/PdoStatementExecuteMethodRuleTest.php index 764bdc0f4..2ca1aab05 100644 --- a/tests/PdoStatementExecuteMethodRuleTest.php +++ b/tests/PdoStatementExecuteMethodRuleTest.php @@ -22,65 +22,65 @@ public function testParameterErrors(): void $this->analyse([__DIR__.'/data/pdo-stmt-execute-error.php'], [ [ - 'Query expects placeholder :adaid, but it is missing from values given to execute().', + 'Query expects placeholder :adaid, but it is missing from values given.', 12, ], [ - 'Value :wrongParamName is given to execute(), but the query does not contain this placeholder.', + 'Value :wrongParamName is given, but the query does not contain this placeholder.', 12, ], [ - 'Query expects placeholder :adaid, but it is missing from values given to execute().', + 'Query expects placeholder :adaid, but it is missing from values given.', 15, ], [ - 'Value :wrongParamName is given to execute(), but the query does not contain this placeholder.', + 'Value :wrongParamName is given, but the query does not contain this placeholder.', 15, ], /* [ - 'Query expects placeholder :adaid, but it is missing from values given to execute().', + 'Query expects placeholder :adaid, but it is missing from values given.', 18, ], [ - 'Value :wrongParamValue is given to execute(), but the query does not contain this placeholder.', + 'Value :wrongParamValue is given, but the query does not contain this placeholder.', 18, ], */ [ - 'Query expects 1 placeholder, but no values are given to execute().', + 'Query expects 1 placeholder, but no values are given.', 21, ], [ - 'Query expects 2 placeholders, but 1 value is given to execute().', + 'Query expects 2 placeholders, but 1 value is given.', 24, ], [ - 'Query expects 2 placeholders, but 1 value is given to execute().', + 'Query expects 2 placeholders, but 1 value is given.', 27, ], [ - 'Query expects 2 placeholders, but 1 value is given to execute().', + 'Query expects 2 placeholders, but 1 value is given.', 30, ], [ - 'Query expects placeholder :adaid, but it is missing from values given to execute().', + 'Query expects placeholder :adaid, but it is missing from values given.', 36, ], [ - 'Value :wrongParamName is given to execute(), but the query does not contain this placeholder.', + 'Value :wrongParamName is given, but the query does not contain this placeholder.', 36, ], [ - 'Query expects 1 placeholder, but 2 values are given to execute().', + 'Query expects 1 placeholder, but 2 values are given.', 38, ], [ - 'Query expects placeholder :asdsa, but it is missing from values given to execute().', + 'Query expects placeholder :asdsa, but it is missing from values given.', 54, ], [ - 'Value :adaid is given to execute(), but the query does not contain this placeholder.', + 'Value :adaid is given, but the query does not contain this placeholder.', 54, ], ]); diff --git a/tests/SyntaxErrorInPreparedStatementMethodRuleTest.php b/tests/SyntaxErrorInPreparedStatementMethodRuleTest.php index 3dc1cac1c..3f02a87d0 100644 --- a/tests/SyntaxErrorInPreparedStatementMethodRuleTest.php +++ b/tests/SyntaxErrorInPreparedStatementMethodRuleTest.php @@ -53,6 +53,10 @@ public function testSyntaxErrorInQueryRule(): void "Query error: Unknown column 'asdsa' in 'where clause' (1054).", 122, ], + [ + 'Value :gesperrt is given, but the query does not contain this placeholder.', + 137, + ], ]); } } diff --git a/tests/data/syntax-error-in-prepared-statement.php b/tests/data/syntax-error-in-prepared-statement.php index b495730b7..88a1b7a75 100644 --- a/tests/data/syntax-error-in-prepared-statement.php +++ b/tests/data/syntax-error-in-prepared-statement.php @@ -121,4 +121,19 @@ public function conditionalSyntaxError(Connection $connection) $connection->preparedQuery($query, [1]); } + + public function placeholderValidation(Connection $connection) + { + $query = 'SELECT email, adaid, gesperrt, freigabe1u1 FROM ada'; + + if (rand(0, 1)) { + // valid condition + $query .= ' WHERE gesperrt=?'; + } else { + // unknown column + $query .= ' WHERE asdsa=?'; + } + + $connection->preparedQuery($query, [':gesperrt' => 1]); + } }