Named placeholders only need to be contained in one of the union quer…

…ies (#625)
staabm · Sep 28, 2023 · b413303 · b413303
1 parent dcadb25
commit b413303
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 25 deletions.
diff --git a/src/QueryReflection/PlaceholderValidation.php b/src/QueryReflection/PlaceholderValidation.php
@@ -18,29 +18,26 @@ public function checkQuery(Expr $queryExpr, Scope $scope, array $parameters): it
     {
         $queryReflection = new QueryReflection();
 
+        $queryStrings = [];
+        $namedPlaceholders = false;
         foreach ($queryReflection->resolveQueryStrings($queryExpr, $scope) as $queryString) {
-            foreach ($this->checkErrors($queryString, $parameters) as $error) {
-                yield $error;
+            $queryStrings[] = $queryString;
+
+            if ($queryReflection->containsNamedPlaceholders($queryString, $parameters)) {
+                $namedPlaceholders = true;
             }
         }
-    }
 
-    /**
-     * @param array<string|int, Parameter> $parameters
-     *
-     * @return iterable<string>
-     */
-    private function checkErrors(string $queryString, array $parameters): iterable
-    {
-        $queryReflection = new QueryReflection();
-        if ($queryReflection->containsNamedPlaceholders($queryString, $parameters)) {
-            yield from $this->validateNamedPlaceholders($queryString, $parameters);
+        if ($namedPlaceholders) {
+            yield from $this->validateNamedPlaceholders($queryStrings, $parameters);
 
             return;
         }
 
-        $placeholderCount = $queryReflection->countPlaceholders($queryString);
-        yield from $this->validateUnnamedPlaceholders($parameters, $placeholderCount);
+        foreach ($queryStrings as $queryString) {
+            $placeholderCount = $queryReflection->countPlaceholders($queryString);
+            yield from $this->validateUnnamedPlaceholders($parameters, $placeholderCount);
+        }
     }
 
     /**
@@ -85,18 +82,25 @@ private function validateUnnamedPlaceholders(array $parameters, int $placeholder
     }
 
     /**
+     * @param list<string> $queryStrings
      * @param array<string|int, Parameter> $parameters
      *
      * @return iterable<string>
      */
-    private function validateNamedPlaceholders(string $queryString, array $parameters): iterable
+    private function validateNamedPlaceholders(array $queryStrings, array $parameters): iterable
     {
         $queryReflection = new QueryReflection();
-        $namedPlaceholders = $queryReflection->extractNamedPlaceholders($queryString);
 
-        foreach ($namedPlaceholders as $namedPlaceholder) {
-            if (! \array_key_exists($namedPlaceholder, $parameters)) {
-                yield sprintf('Query expects placeholder %s, but it is missing from values given.', $namedPlaceholder);
+        $allNamedPlaceholders = [];
+        foreach ($queryStrings as $queryString) {
+            $namedPlaceholders = $queryReflection->extractNamedPlaceholders($queryString);
+
+            foreach ($namedPlaceholders as $namedPlaceholder) {
+                if (! \array_key_exists($namedPlaceholder, $parameters)) {
+                    yield sprintf('Query expects placeholder %s, but it is missing from values given.', $namedPlaceholder);
+                }
+
+                $allNamedPlaceholders[] = $namedPlaceholder;
             }
         }
 
@@ -107,7 +111,7 @@ private function validateNamedPlaceholders(string $queryString, array $parameter
             if ($parameter->isOptional) {
                 continue;
             }
-            if (! \in_array($placeholderKey, $namedPlaceholders, true)) {
+            if (! \in_array($placeholderKey, $allNamedPlaceholders, true)) {
                 yield sprintf('Value %s is given, but the query does not contain this placeholder.', $placeholderKey);
             }
         }

diff --git a/tests/rules/PdoStatementExecuteMethodRuleTest.php b/tests/rules/PdoStatementExecuteMethodRuleTest.php
@@ -86,10 +86,6 @@ public function testParameterErrors(): void
                 'Query expects placeholder :asdsa, but it is missing from values given.',
                 54,
             ],
-            [
-                'Value :adaid is given, but the query does not contain this placeholder.',
-                54,
-            ],
             [
                 'Query expects placeholder :adaid, but it is missing from values given.',
                 61,
@@ -116,4 +112,10 @@ public function testParameterErrors(): void
             ],
         ]);
     }
+
+    public function testNamedPlaceholderBug(): void
+    {
+        $this->analyse([__DIR__ . '/data/named-placeholder-bug.php'], [
+        ]);
+    }
 }
diff --git a/tests/rules/data/named-placeholder-bug.php b/tests/rules/data/named-placeholder-bug.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace NamedPlaceholderBug;
+
+
+use PDO;
+
+class Foo
+{
+    public function errors(PDO $pdo, $vkFrom)
+    {
+        $fromCondition = '';
+        if ('0' !== $vkFrom) {
+            $fromCondition = 'and email = :vkFrom';
+        }
+
+        $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid ' . $fromCondition);
+        $stmt->execute([
+            'adaid' => 1,
+            'vkFrom' => 'hello world'
+        ]);
+    }
+}