Give each value of an array expression parameter its own placeholder

docs/book/sql/advanced.md

@@ -46,7 +46,7 @@ CASE WHEN age > 18 THEN ADULT ELSE MINOR END
 ```php title="Array values in expressions"
 $expression = new Expression(
     'id IN (?)',
-    [Argument::value([1, 2, 3, 4, 5])]
+    [Argument::values([1, 2, 3, 4, 5])]
 );
 ```
 
@@ -56,6 +56,14 @@ Produces:
 id IN (?, ?, ?, ?, ?)
 ```
 
+The placeholder for an array (or `Argument\Values`) expands to one
+placeholder per value, separated by commas, so write the surrounding
+parentheses in the expression yourself. An empty array renders as `NULL`.
+The placeholder must sit alone inside its parentheses: `id IN ?` and
+`id IN (?, ?)` with an array throw
+`PhpDb\Sql\Exception\InvalidArgumentException`, since they would render
+invalid SQL.
+
 ```php title="Nested expressions"
 $innerExpression = new Expression('COUNT(*)');
 $outerExpression = new Expression(

src/Sql/Exception/InvalidArgumentException.php

@@ -72,6 +72,10 @@ class InvalidArgumentException extends Exception\InvalidArgumentException
         'A PhpDb\Sql\Select instance cannot be provided'
             . ' with the merge flag';
 
+    final public const string UNENCLOSED_VALUES_PLACEHOLDER =
+        'An array or Values parameter in the expression "%s" must use a placeholder'
+            . ' wrapped in its own parentheses, such as IN (?)';
+
     final public const string UNKNOWN_ARGUMENT_TYPE = 'Unknown argument type';
 
     final public const string UNKNOWN_COLUMN_KEY = 'The key %s was not found in this objects column list';
@@ -188,6 +192,11 @@ public static function forSelectWithMergeFlag(): self
         return new self(self::SELECT_WITH_MERGE_FLAG);
     }
 
+    public static function forUnenclosedValuesPlaceholder(string $expression): self
+    {
+        return new self(sprintf(self::UNENCLOSED_VALUES_PLACEHOLDER, $expression));
+    }
+
     public static function forUnknownArgumentType(): self
     {
         return new self(self::UNKNOWN_ARGUMENT_TYPE);

src/Sql/Expression.php

@@ -9,14 +9,18 @@
 use PhpDb\Sql\Argument\Value;
 use PhpDb\Sql\Argument\Values;
 
+use function array_shift;
 use function array_slice;
 use function array_unique;
 use function count;
+use function explode;
 use function func_get_args;
 use function func_num_args;
 use function is_array;
+use function preg_match;
 use function preg_match_all;
 use function str_replace;
+use function substr;
 
 /**
  * @api
@@ -66,6 +70,7 @@ public function getExpression(): string
     }
 
     /**
+     * @throws Exception\InvalidArgumentException
      * @throws Exception\RuntimeException
      * @inheritDoc
      */
@@ -83,13 +88,16 @@ public function getExpressionData(): array
             ];
         }
 
-        // assign locally, escaping % signs
-        $count         = 0;
-        $specification = str_replace(self::PLACEHOLDER, replace: '%s', subject: $specification, count: $count);
+        $segments      = explode(self::PLACEHOLDER, $specification);
+        $specification = array_shift($segments);
+
+        foreach ($segments as $index => $segment) {
+            $specification .= $this->placeholderFor($parameters[$index] ?? null, $specification, $segment) . $segment;
+        }
 
         // test number of replacements without considering same variable begin used many times first, which is
         // faster, if the test fails then resort to regex which are slow and used rarely
-        if ($count !== $parametersCount) {
+        if (count($segments) !== $parametersCount) {
             $matches = [];
             preg_match_all('/:\w*/', $specification, $matches);
             if (count(array_unique($matches[0] ?? [])) !== $parametersCount) {
@@ -144,4 +152,22 @@ public function setParameters(
 
         return $this;
     }
+
+    /**
+     * Values are bound one by one, so each needs its own placeholder inside the parentheses around the `?`
+     *
+     * @throws Exception\InvalidArgumentException
+     */
+    private function placeholderFor(?ArgumentInterface $parameter, string $before, string $after): string
+    {
+        if (! $parameter instanceof Values) {
+            return '%s';
+        }
+
+        if (1 !== preg_match('/\(\s*$/', $before) || 1 !== preg_match('/^\s*\)/', $after)) {
+            throw Exception\InvalidArgumentException::forUnenclosedValuesPlaceholder($this->expression);
+        }
+
+        return substr($parameter->getSpecification(), offset: 1, length: -1);
+    }
 }

test/unit/Sql/Exception/InvalidArgumentExceptionTest.php

@@ -38,6 +38,7 @@
 #[CoversMethod(InvalidArgumentException::class, 'forNonStringValueKey')]
 #[CoversMethod(InvalidArgumentException::class, 'forReadOnlyConstructorState')]
 #[CoversMethod(InvalidArgumentException::class, 'forSelectWithMergeFlag')]
+#[CoversMethod(InvalidArgumentException::class, 'forUnenclosedValuesPlaceholder')]
 #[CoversMethod(InvalidArgumentException::class, 'forUnknownArgumentType')]
 #[CoversMethod(InvalidArgumentException::class, 'forUnknownColumnKey')]
 #[CoversMethod(InvalidArgumentException::class, 'forValuesWithMergeFlag')]
@@ -60,122 +61,127 @@ public static function foreignTableProvider(): array
     public static function namedConstructorProvider(): array
     {
         return [
-            'missing decimal digits'      => [
+            'missing decimal digits'        => [
                 'forMissingDecimalDigits',
                 ['price', 'DECIMAL'],
                 InvalidArgumentException::MISSING_DECIMAL_DIGITS,
             ],
-            'missing column length'       => [
+            'missing column length'         => [
                 'forMissingColumnLength',
                 ['name', 'VARCHAR'],
                 InvalidArgumentException::MISSING_COLUMN_LENGTH,
             ],
-            'invalid column length'       => [
+            'invalid column length'         => [
                 'forInvalidColumnLength',
                 ['age'],
                 InvalidArgumentException::INVALID_COLUMN_LENGTH,
             ],
-            'empty check expression'      => [
+            'empty check expression'        => [
                 'forEmptyCheckExpression',
                 [],
                 InvalidArgumentException::EMPTY_CHECK_EXPRESSION,
             ],
-            'already combined'            => [
+            'already combined'              => [
                 'forAlreadyCombined',
                 [],
                 InvalidArgumentException::ALREADY_COMBINED,
             ],
-            'empty expression'            => [
+            'empty expression'              => [
                 'forEmptyExpression',
                 [],
                 InvalidArgumentException::EMPTY_EXPRESSION,
             ],
-            'empty prefix'                => [
+            'empty prefix'                  => [
                 'forEmptyPrefix',
                 [],
                 InvalidArgumentException::EMPTY_PREFIX,
             ],
-            'empty schema'                => [
+            'empty schema'                  => [
                 'forEmptySchema',
                 [],
                 InvalidArgumentException::EMPTY_SCHEMA,
             ],
-            'empty separator'             => [
+            'empty separator'               => [
                 'forEmptySeparator',
                 [],
                 InvalidArgumentException::EMPTY_SEPARATOR,
             ],
-            'empty table'                 => [
+            'empty table'                   => [
                 'forEmptyTable',
                 [],
                 InvalidArgumentException::EMPTY_TABLE,
             ],
-            'foreign table'               => [
+            'foreign table'                 => [
                 'forForeignTable',
                 ['users'],
                 InvalidArgumentException::FOREIGN_TABLE,
             ],
-            'invalid argument type'       => [
+            'invalid argument type'         => [
                 'forInvalidArgumentType',
                 [],
                 InvalidArgumentException::INVALID_ARGUMENT_TYPE,
             ],
-            'invalid from array'          => [
+            'invalid from array'            => [
                 'forInvalidFromArray',
                 [],
                 InvalidArgumentException::INVALID_FROM_ARRAY,
             ],
-            'invalid join name'           => [
+            'invalid join name'             => [
                 'forInvalidJoinName',
                 ['orders'],
                 InvalidArgumentException::INVALID_JOIN_NAME,
             ],
-            'invalid magic property'      => [
+            'invalid magic property'        => [
                 'forInvalidMagicProperty',
                 [],
                 InvalidArgumentException::INVALID_MAGIC_PROPERTY,
             ],
-            'invalid specification name'  => [
+            'invalid specification name'    => [
                 'forInvalidSpecificationName',
                 [],
                 InvalidArgumentException::INVALID_SPECIFICATION_NAME,
             ],
-            'missing values or select'    => [
+            'missing values or select'      => [
                 'forMissingValuesOrSelect',
                 [],
                 InvalidArgumentException::MISSING_VALUES_OR_SELECT,
             ],
-            'non numeric parameter'       => [
+            'non numeric parameter'         => [
                 'forNonNumericParameter',
                 ['PhpDb\\Sql\\Select::limit', 'string'],
                 InvalidArgumentException::NON_NUMERIC_PARAMETER,
             ],
-            'non string value key'        => [
+            'non string value key'          => [
                 'forNonStringValueKey',
                 [],
                 InvalidArgumentException::NON_STRING_VALUE_KEY,
             ],
-            'read only constructor state' => [
+            'read only constructor state'   => [
                 'forReadOnlyConstructorState',
                 [],
                 InvalidArgumentException::READ_ONLY_CONSTRUCTOR_STATE,
             ],
-            'select with merge flag'      => [
+            'select with merge flag'        => [
                 'forSelectWithMergeFlag',
                 [],
                 InvalidArgumentException::SELECT_WITH_MERGE_FLAG,
             ],
-            'unknown argument type'       => [
+            'unenclosed values placeholder' => [
+                'forUnenclosedValuesPlaceholder',
+                ['a IN ?'],
+                InvalidArgumentException::UNENCLOSED_VALUES_PLACEHOLDER,
+            ],
+            'unknown argument type'         => [
                 'forUnknownArgumentType',
                 [],
                 InvalidArgumentException::UNKNOWN_ARGUMENT_TYPE,
             ],
-            'unknown column key'          => [
+            'unknown column key'            => [
                 'forUnknownColumnKey',
                 ['email'],
                 InvalidArgumentException::UNKNOWN_COLUMN_KEY,
             ],
-            'values with merge flag'      => [
+            'values with merge flag'        => [
                 'forValuesWithMergeFlag',
                 [],
                 InvalidArgumentException::VALUES_WITH_MERGE_FLAG,

test/unit/Sql/ExpressionTest.php

@@ -18,6 +18,8 @@
 use PHPUnit\Framework\TestCase;
 use TypeError;
 
+use function sprintf;
+
 /**
  * This is a unit testing test case.
  * A unit here is a method, there will be at least one test per method
@@ -30,8 +32,25 @@
 #[CoversMethod(Expression::class, 'setParameters')]
 #[CoversMethod(Expression::class, 'getParameters')]
 #[CoversMethod(Expression::class, 'getExpressionData')]
+#[CoversMethod(Expression::class, 'placeholderFor')]
 final class ExpressionTest extends TestCase
 {
+    /** @return array<string, array{string, list<mixed>, string}> */
+    public static function arrayParameterSpecificationProvider(): array
+    {
+        return [
+            'array of three values'          => ['a IN (?)', [[1, 2, 3]], 'a IN (%s, %s, %s)'],
+            'empty array'                    => ['a IN (?)', [[]], 'a IN (NULL)'],
+            'array between other parameters' => [
+                '? IN (?) AND ? = ?',
+                [Argument::identifier('a'), [1, 2], Argument::identifier('b'), 4],
+                '%s IN (%s, %s) AND %s = %s',
+            ],
+            'values argument'                => ['a IN (?)', [Argument::values([1, 2])], 'a IN (%s, %s)'],
+            'whitespace inside parentheses'  => ['a IN ( ? )', [[1, 2]], 'a IN ( %s, %s )'],
+        ];
+    }
+
     /** @psalm-return array<array-key, array{0: mixed}> */
     public static function falsyExpressionParametersProvider(): array
     {
@@ -44,6 +63,19 @@ public static function falsyExpressionParametersProvider(): array
         ];
     }
 
+    /** @return array<string, array{string, list<mixed>}> */
+    public static function unenclosedArrayParameterProvider(): array
+    {
+        return [
+            'array'                    => ['a IN ?', [[1, 2, 3]]],
+            'empty array'              => ['a IN ?', [[]]],
+            'values argument'          => ['a IN ?', [Argument::values([1, 2])]],
+            'opening parenthesis only' => ['a IN (?', [[1, 2]]],
+            'closing parenthesis only' => ['a IN ?)', [[1, 2]]],
+            'parenthesis around more'  => ['a IN (?, ?)', [[1, 2], 3]],
+        ];
+    }
+
     #[Test]
     #[DataProvider('falsyExpressionParametersProvider')]
     public function constructorWithFalsyValidParameters(mixed $falsyParameter): void
@@ -113,6 +145,38 @@ public function getExpressionData(): void
         );
     }
 
+    /**
+     * @param list<mixed> $parameters
+     */
+    #[Test]
+    #[Group('unit')]
+    #[DataProvider('arrayParameterSpecificationProvider')]
+    public function getExpressionDataGivesEachArrayValueItsOwnPlaceholder(
+        string $expression,
+        array $parameters,
+        string $expectedSpecification,
+    ): void {
+        $expressionData = (new Expression($expression, $parameters))->getExpressionData();
+
+        static::assertSame($expectedSpecification, $expressionData['spec']);
+    }
+
+    /**
+     * @param list<mixed> $parameters
+     */
+    #[Test]
+    #[Group('unit')]
+    #[DataProvider('unenclosedArrayParameterProvider')]
+    public function getExpressionDataRejectsArrayPlaceholderOutsideParentheses(
+        string $expression,
+        array $parameters,
+    ): void {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage(sprintf(InvalidArgumentException::UNENCLOSED_VALUES_PLACEHOLDER, $expression));
+
+        (new Expression($expression, $parameters))->getExpressionData();
+    }
+
     #[Test]
     public function getExpressionDataThrowsExceptionWhenParameterCountMismatch(): void
     {