Move argument rendering from ArgumentInterface into AbstractSqlRenderer

simon-mundy · simon-mundy · commit 2368e42f0410 · 2026-03-03T14:18:14.000+11:00
Remove render() from ArgumentInterface and all 8 implementations, making
argument objects pure data carriers. The renderer now owns all rendering
via instanceof dispatch in renderArgument(), eliminating a method-call
hop per argument. Argument properties changed from private to public for
direct access. Also exclude top-level test/*.php files from phpcs.
diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -16,6 +16,8 @@
     <file>src</file>
     <file>test</file>
 
+    <exclude-pattern>test/*[^/]\.php$</exclude-pattern>
+
     <!-- Include all rules from Laminas Coding Standard -->
     <rule ref="LaminasCodingStandard">
         <!--
diff --git a/src/Sql/Argument/Identifier.php b/src/Sql/Argument/Identifier.php
@@ -6,7 +6,6 @@
 
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ArgumentType;
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
 
 final readonly class Identifier implements ArgumentInterface
 {
@@ -24,9 +23,4 @@ public function getValue(): string
     {
         return $this->identifier;
     }
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string
-    {
-        return $renderer->platform->quoteIdentifier($this->identifier);
-    }
 }
diff --git a/src/Sql/Argument/Identifiers.php b/src/Sql/Argument/Identifiers.php
@@ -6,9 +6,6 @@
 
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ArgumentType;
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
-
-use function implode;
 
 final readonly class Identifiers implements ArgumentInterface
 {
@@ -43,13 +40,4 @@ public function getValue(): array
         }
         return $result;
     }
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string
-    {
-        $quoted = [];
-        foreach ($this->identifiers as $identifier) {
-            $quoted[] = $identifier->render($renderer, $paramPrefix, $paramIndex);
-        }
-        return implode(', ', $quoted);
-    }
 }
diff --git a/src/Sql/Argument/Literal.php b/src/Sql/Argument/Literal.php
@@ -6,12 +6,11 @@
 
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ArgumentType;
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
 
 final readonly class Literal implements ArgumentInterface
 {
     public function __construct(
-        private string $literal
+        public string $literal
     ) {
     }
 
@@ -24,9 +23,4 @@ public function getValue(): string
     {
         return $this->literal;
     }
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string
-    {
-        return $this->literal;
-    }
 }
diff --git a/src/Sql/Argument/NullValue.php b/src/Sql/Argument/NullValue.php
@@ -6,7 +6,6 @@
 
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ArgumentType;
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
 
 final readonly class NullValue implements ArgumentInterface
 {
@@ -19,9 +18,4 @@ public function getValue(): null
     {
         return null;
     }
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string
-    {
-        return 'NULL';
-    }
 }
diff --git a/src/Sql/Argument/Parameter.php b/src/Sql/Argument/Parameter.php
@@ -6,7 +6,6 @@
 
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ArgumentType;
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
 
 final readonly class Parameter implements ArgumentInterface
 {
@@ -16,9 +15,9 @@
      * @param ?string                    $typeHint       Type hint for ParameterContainer (e.g. TYPE_INTEGER)
      */
     public function __construct(
-        private null|string|int|float|bool $value,
-        private ?string $preferredName = null,
-        private ?string $typeHint = null,
+        public null|string|int|float|bool $value,
+        public ?string $preferredName = null,
+        public ?string $typeHint = null,
     ) {
     }
 
@@ -41,9 +40,4 @@ public function getTypeHint(): ?string
     {
         return $this->typeHint;
     }
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string
-    {
-        return $renderer->bindParameter($this);
-    }
 }
diff --git a/src/Sql/Argument/Select.php b/src/Sql/Argument/Select.php
@@ -7,13 +7,12 @@
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ArgumentType;
 use PhpDb\Sql\ExpressionInterface;
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
 use PhpDb\Sql\SqlInterface;
 
 final readonly class Select implements ArgumentInterface
 {
     public function __construct(
-        private ExpressionInterface|SqlInterface $select
+        public ExpressionInterface|SqlInterface $select
     ) {
     }
 
@@ -26,13 +25,4 @@ public function getValue(): ExpressionInterface|SqlInterface
     {
         return $this->select;
     }
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string
-    {
-        if ($this->select instanceof \PhpDb\Sql\Select) {
-            return '(' . $renderer->processSubSelect($this->select) . ')';
-        }
-
-        return $this->select->toSql($renderer, $paramPrefix, $paramIndex);
-    }
 }
diff --git a/src/Sql/Argument/Value.php b/src/Sql/Argument/Value.php
@@ -6,15 +6,14 @@
 
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ArgumentType;
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
 
 final readonly class Value implements ArgumentInterface
 {
     /**
      * @param null|string|int|float|bool $value Scalar value, or null
      */
     public function __construct(
-        private null|string|int|float|bool $value
+        public null|string|int|float|bool $value
     ) {
     }
 
@@ -27,11 +26,4 @@ public function getValue(): null|string|int|float|bool
     {
         return $this->value;
     }
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string
-    {
-        return $renderer->parameterContainer !== null
-            ? $renderer->bindValue($this->value, $paramPrefix, $paramIndex)
-            : $renderer->platform->quoteValue((string) $this->value);
-    }
 }
diff --git a/src/Sql/Argument/Values.php b/src/Sql/Argument/Values.php
@@ -6,15 +6,13 @@
 
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ArgumentType;
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
 
 use function array_values;
-use function implode;
 
 final readonly class Values implements ArgumentInterface
 {
     /** @var list<null|string|int|float|bool> */
-    private array $values;
+    public array $values;
 
     /**
      * @param list<null|string|int|float|bool> $values
@@ -36,21 +34,4 @@ public function getValue(): array
     {
         return $this->values;
     }
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string
-    {
-        $rendered = [];
-
-        if ($renderer->parameterContainer !== null) {
-            foreach ($this->values as $value) {
-                $rendered[] = $renderer->bindValue($value, $paramPrefix, $paramIndex);
-            }
-        } else {
-            foreach ($this->values as $value) {
-                $rendered[] = $renderer->platform->quoteValue((string) $value);
-            }
-        }
-
-        return '(' . implode(', ', $rendered) . ')';
-    }
 }
diff --git a/src/Sql/ArgumentInterface.php b/src/Sql/ArgumentInterface.php
@@ -4,13 +4,9 @@
 
 namespace PhpDb\Sql;
 
-use PhpDb\Sql\Platform\AbstractSqlRenderer;
-
 interface ArgumentInterface
 {
     public function getType(): ArgumentType;
 
     public function getValue(): ExpressionInterface|SqlInterface|string|int|float|bool|array|null;
-
-    public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string;
 }
diff --git a/src/Sql/Part/Columns.php b/src/Sql/Part/Columns.php
@@ -61,7 +61,7 @@ public function toSql(AbstractSqlRenderer $renderer, string $paramPrefix = '', i
             if ($column instanceof Identifier) {
                 $columnSql = $prefix . $platform->quoteIdentifier($column->identifier);
             } elseif ($column instanceof ArgumentInterface) {
-                $columnSql = $prefix . $column->render($renderer, '', $pi);
+                $columnSql = $prefix . $renderer->renderArgument($column, '', $pi);
             } else {
                 $columnSql = $renderer->render($column, $ref->alias ?? 'column');
             }
diff --git a/src/Sql/Part/GroupBy.php b/src/Sql/Part/GroupBy.php
@@ -32,7 +32,7 @@ public function toSql(AbstractSqlRenderer $renderer, string $paramPrefix = '', i
                 $groups[] = $platform->quoteIdentifier($column->identifier);
             } elseif ($column instanceof ArgumentInterface) {
                 $pi       = 0;
-                $groups[] = $column->render($renderer, '', $pi);
+                $groups[] = $renderer->renderArgument($column, '', $pi);
             } else {
                 $groups[] = $renderer->render($column);
             }
diff --git a/src/Sql/Part/OrderBy.php b/src/Sql/Part/OrderBy.php
@@ -41,7 +41,7 @@ public function toSql(AbstractSqlRenderer $renderer, string $paramPrefix = '', i
                            . ' ' . $spec->direction;
             } elseif ($column instanceof ArgumentInterface) {
                 $pi       = 0;
-                $orders[] = $column->render($renderer, '', $pi)
+                $orders[] = $renderer->renderArgument($column, '', $pi)
                            . ' ' . $spec->direction;
             } else {
                 $orders[] = $renderer->render($column);
diff --git a/src/Sql/Part/Set.php b/src/Sql/Part/Set.php
@@ -39,7 +39,7 @@ public function toSql(AbstractSqlRenderer $renderer, string $paramPrefix = '', i
         $isPdoDriver = $renderer->driver instanceof PdoDriverInterface;
 
         foreach ($this->model as $column => $arg) {
-            $prefix = $this->columnIds[$column]->render($renderer, '', $pi) . ' = ';
+            $prefix = $renderer->platform->quoteIdentifier($this->columnIds[$column]->identifier) . ' = ';
 
             $setSql[] = $prefix . match ($arg->getType()) {
                 ArgumentType::Parameter => $renderer->bindParameter(
diff --git a/src/Sql/Platform/AbstractSqlRenderer.php b/src/Sql/Platform/AbstractSqlRenderer.php
@@ -7,11 +7,19 @@
 use PhpDb\Adapter\Driver\DriverInterface;
 use PhpDb\Adapter\ParameterContainer;
 use PhpDb\Adapter\Platform\PlatformInterface;
+use PhpDb\Sql\Argument\Identifier;
+use PhpDb\Sql\Argument\Identifiers;
+use PhpDb\Sql\Argument\Literal;
+use PhpDb\Sql\Argument\NullValue;
 use PhpDb\Sql\Argument\Parameter;
+use PhpDb\Sql\Argument\Select as SelectArgument;
+use PhpDb\Sql\Argument\Value;
+use PhpDb\Sql\Argument\Values;
 use PhpDb\Sql\ArgumentInterface;
 use PhpDb\Sql\ExpressionInterface;
 use PhpDb\Sql\Select;
 use PhpDb\Sql\TableIdentifier;
+use UnexpectedValueException;
 
 use function count;
 use function implode;
@@ -169,7 +177,21 @@ public function renderArgument(
         string $paramPrefix,
         int &$paramIndex,
     ): string {
-        return $argument->render($this, $paramPrefix, $paramIndex);
+        return match (true) {
+            $argument instanceof Identifier    => $this->platform->quoteIdentifier($argument->identifier),
+            $argument instanceof Literal       => $argument->literal,
+            $argument instanceof NullValue     => 'NULL',
+            $argument instanceof Parameter     => $this->bindParameter($argument),
+            $argument instanceof Value         => $this->parameterContainer !== null
+                ? $this->bindValue($argument->value, $paramPrefix, $paramIndex)
+                : $this->platform->quoteValue((string) $argument->value),
+            $argument instanceof SelectArgument => $argument->select instanceof Select
+                ? '(' . $this->processSubSelect($argument->select) . ')'
+                : $argument->select->toSql($this, $paramPrefix, $paramIndex),
+            $argument instanceof Values        => $this->renderValues($argument->values, $paramPrefix, $paramIndex),
+            $argument instanceof Identifiers   => $this->renderIdentifiers($argument->identifiers),
+            default => throw new UnexpectedValueException('Unknown argument type: ' . $argument::class),
+        };
     }
 
     public function bindParameter(Parameter $param, ?string $nameOverride = null): string
@@ -212,6 +234,32 @@ public function processSubSelect(Select $subselect): string
         return $subselect->buildSqlString($this);
     }
 
+    /** @param list<Identifier> $identifiers */
+    private function renderIdentifiers(array $identifiers): string
+    {
+        $quoted = [];
+        foreach ($identifiers as $id) {
+            $quoted[] = $this->platform->quoteIdentifier($id->identifier);
+        }
+        return implode(', ', $quoted);
+    }
+
+    /** @param list<null|string|int|float|bool> $values */
+    private function renderValues(array $values, string $paramPrefix, int &$paramIndex): string
+    {
+        $rendered = [];
+        if ($this->parameterContainer !== null) {
+            foreach ($values as $value) {
+                $rendered[] = $this->bindValue($value, $paramPrefix, $paramIndex);
+            }
+        } else {
+            foreach ($values as $value) {
+                $rendered[] = $this->platform->quoteValue((string) $value);
+            }
+        }
+        return '(' . implode(', ', $rendered) . ')';
+    }
+
     private function resolveParamPrefix(?string $paramPrefix): string
     {
         if ($paramPrefix === null || $paramPrefix === '') {
diff --git a/src/Sql/Predicate/Operator.php b/src/Sql/Predicate/Operator.php
@@ -144,8 +144,8 @@ public function toSql(AbstractSqlRenderer $renderer, string $paramPrefix = '', i
             );
         }
 
-        $leftSql  = $this->left->render($renderer, $paramPrefix, $paramIndex);
-        $rightSql = $this->right->render($renderer, $paramPrefix, $paramIndex);
+        $leftSql  = $renderer->renderArgument($this->left, $paramPrefix, $paramIndex);
+        $rightSql = $renderer->renderArgument($this->right, $paramPrefix, $paramIndex);
 
         if ($this->specification !== null) {
             return vsprintf($this->specification, [$leftSql, $rightSql]);
diff --git a/src/Sql/TokenizesExpressionTrait.php b/src/Sql/TokenizesExpressionTrait.php
@@ -51,7 +51,7 @@ protected function renderTokens(AbstractSqlRenderer $renderer, string $paramPref
     {
         $sql = '';
         foreach ($this->tokens as $token) {
-            $sql .= $token->render($renderer, $paramPrefix, $paramIndex);
+            $sql .= $renderer->renderArgument($token, $paramPrefix, $paramIndex);
         }
         return $sql;
     }

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,6 @@`
`6`	`6`
`7`	`7`	`use PhpDb\Sql\ArgumentInterface;`
`8`	`8`	`use PhpDb\Sql\ArgumentType;`
`9`		`-use PhpDb\Sql\Platform\AbstractSqlRenderer;`
`10`	`9`
`11`	`10`	`final readonly class Identifier implements ArgumentInterface`
`12`	`11`	`{`
`@@ -24,9 +23,4 @@ public function getValue(): string`
`24`	`23`	`{`
`25`	`24`	`return $this->identifier;`
`26`	`25`	`}`
`27`		`-`
`28`		`- public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string`
`29`		`- {`
`30`		`- return $renderer->platform->quoteIdentifier($this->identifier);`
`31`		`- }`
`32`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,9 +6,6 @@`
`6`	`6`
`7`	`7`	`use PhpDb\Sql\ArgumentInterface;`
`8`	`8`	`use PhpDb\Sql\ArgumentType;`
`9`		`-use PhpDb\Sql\Platform\AbstractSqlRenderer;`
`10`		`-`
`11`		`-use function implode;`
`12`	`9`
`13`	`10`	`final readonly class Identifiers implements ArgumentInterface`
`14`	`11`	`{`
`@@ -43,13 +40,4 @@ public function getValue(): array`
`43`	`40`	`}`
`44`	`41`	`return $result;`
`45`	`42`	`}`
`46`		`-`
`47`		`- public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string`
`48`		`- {`
`49`		`- $quoted = [];`
`50`		`- foreach ($this->identifiers as $identifier) {`
`51`		`- $quoted[] = $identifier->render($renderer, $paramPrefix, $paramIndex);`
`52`		`- }`
`53`		`- return implode(', ', $quoted);`
`54`		`- }`
`55`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,12 +6,11 @@`
`6`	`6`
`7`	`7`	`use PhpDb\Sql\ArgumentInterface;`
`8`	`8`	`use PhpDb\Sql\ArgumentType;`
`9`		`-use PhpDb\Sql\Platform\AbstractSqlRenderer;`
`10`	`9`
`11`	`10`	`final readonly class Literal implements ArgumentInterface`
`12`	`11`	`{`
`13`	`12`	`public function __construct(`
`14`		`- private string $literal`
	`13`	`+ public string $literal`
`15`	`14`	`) {`
`16`	`15`	`}`
`17`	`16`
`@@ -24,9 +23,4 @@ public function getValue(): string`
`24`	`23`	`{`
`25`	`24`	`return $this->literal;`
`26`	`25`	`}`
`27`		`-`
`28`		`- public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string`
`29`		`- {`
`30`		`- return $this->literal;`
`31`		`- }`
`32`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -7,13 +7,12 @@`
`7`	`7`	`use PhpDb\Sql\ArgumentInterface;`
`8`	`8`	`use PhpDb\Sql\ArgumentType;`
`9`	`9`	`use PhpDb\Sql\ExpressionInterface;`
`10`		`-use PhpDb\Sql\Platform\AbstractSqlRenderer;`
`11`	`10`	`use PhpDb\Sql\SqlInterface;`
`12`	`11`
`13`	`12`	`final readonly class Select implements ArgumentInterface`
`14`	`13`	`{`
`15`	`14`	`public function __construct(`
`16`		`- private ExpressionInterface\|SqlInterface $select`
	`15`	`+ public ExpressionInterface\|SqlInterface $select`
`17`	`16`	`) {`
`18`	`17`	`}`
`19`	`18`
`@@ -26,13 +25,4 @@ public function getValue(): ExpressionInterface\|SqlInterface`
`26`	`25`	`{`
`27`	`26`	`return $this->select;`
`28`	`27`	`}`
`29`		`-`
`30`		`- public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string`
`31`		`- {`
`32`		`- if ($this->select instanceof \PhpDb\Sql\Select) {`
`33`		`- return '(' . $renderer->processSubSelect($this->select) . ')';`
`34`		`- }`
`35`		`-`
`36`		`- return $this->select->toSql($renderer, $paramPrefix, $paramIndex);`
`37`		`- }`
`38`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,15 +6,14 @@`
`6`	`6`
`7`	`7`	`use PhpDb\Sql\ArgumentInterface;`
`8`	`8`	`use PhpDb\Sql\ArgumentType;`
`9`		`-use PhpDb\Sql\Platform\AbstractSqlRenderer;`
`10`	`9`
`11`	`10`	`final readonly class Value implements ArgumentInterface`
`12`	`11`	`{`
`13`	`12`	`/**`
`14`	`13`	`* @param null\|string\|int\|float\|bool $value Scalar value, or null`
`15`	`14`	`*/`
`16`	`15`	`public function __construct(`
`17`		`- private null\|string\|int\|float\|bool $value`
	`16`	`+ public null\|string\|int\|float\|bool $value`
`18`	`17`	`) {`
`19`	`18`	`}`
`20`	`19`
`@@ -27,11 +26,4 @@ public function getValue(): null\|string\|int\|float\|bool`
`27`	`26`	`{`
`28`	`27`	`return $this->value;`
`29`	`28`	`}`
`30`		`-`
`31`		`- public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string`
`32`		`- {`
`33`		`- return $renderer->parameterContainer !== null`
`34`		`- ? $renderer->bindValue($this->value, $paramPrefix, $paramIndex)`
`35`		`- : $renderer->platform->quoteValue((string) $this->value);`
`36`		`- }`
`37`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,13 +4,9 @@`
`4`	`4`
`5`	`5`	`namespace PhpDb\Sql;`
`6`	`6`
`7`		`-use PhpDb\Sql\Platform\AbstractSqlRenderer;`
`8`		`-`
`9`	`7`	`interface ArgumentInterface`
`10`	`8`	`{`
`11`	`9`	`public function getType(): ArgumentType;`
`12`	`10`
`13`	`11`	`public function getValue(): ExpressionInterface\|SqlInterface\|string\|int\|float\|bool\|array\|null;`
`14`		`-`
`15`		`- public function render(AbstractSqlRenderer $renderer, string $paramPrefix, int &$paramIndex): string;`
`16`	`12`	`}`