From 2e3ee1a69762d8c82899134f1e0bb190d7cf8557 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 17:01:55 +1000 Subject: [PATCH 01/17] Validate DDL column options and PDO DSN parameters Reject charset/collate values that are not bare names, restrict COLUMN_FORMAT and STORAGE to enum keywords, and reject DSN control characters in connection parameters. Consolidates the triplicated column option handling into ColumnOptionTrait so each option is validated in one place. --- src/Pdo/Connection.php | 31 ++- src/Sql/ColumnFormatEnum.php | 42 ++++ src/Sql/Ddl/AlterTableDecorator.php | 236 ++---------------- src/Sql/Ddl/ColumnOptionTrait.php | 184 ++++++++++++++ src/Sql/Ddl/CreateTableDecorator.php | 154 +----------- src/Sql/StorageEnum.php | 43 ++++ test/unit/Pdo/ConnectionTest.php | 34 +++ test/unit/Sql/Ddl/AlterTableDecoratorTest.php | 139 +++++++++++ .../unit/Sql/Ddl/CreateTableDecoratorTest.php | 101 ++++++++ .../Sql/Ddl/TestAsset/ColumnOptionMatrix.php | 58 +++++ 10 files changed, 653 insertions(+), 369 deletions(-) create mode 100644 src/Sql/ColumnFormatEnum.php create mode 100644 src/Sql/Ddl/ColumnOptionTrait.php create mode 100644 src/Sql/StorageEnum.php create mode 100644 test/unit/Sql/Ddl/TestAsset/ColumnOptionMatrix.php diff --git a/src/Pdo/Connection.php b/src/Pdo/Connection.php index 8c93155..8cddd81 100644 --- a/src/Pdo/Connection.php +++ b/src/Pdo/Connection.php @@ -17,6 +17,8 @@ use function is_array; use function is_int; use function is_string; +use function preg_match; +use function sprintf; use function strtolower; class Connection extends AbstractPdoConnection @@ -55,6 +57,23 @@ public function getCurrentSchema(): string|false return false; } + /** + * Return a value that is safe to interpolate into a generated DSN. + * + * @throws Exception\InvalidConnectionParametersException If the value contains DSN control characters. + */ + private function getDsnParameter(string $name, string $value): string + { + if (preg_match('/[;\x00-\x1f]/', $value) === 1) { + throw new Exception\InvalidConnectionParametersException( + sprintf('The "%s" connection parameter contains invalid characters', $name), + $this->connectionParameters + ); + } + + return $value; + } + /** * {@inheritDoc} * @@ -102,22 +121,22 @@ public function connect(): ConnectionInterface if (! isset($dsn)) { $dsn = []; if (isset($database)) { - $dsn[] = "dbname={$database}"; + $dsn[] = 'dbname=' . $this->getDsnParameter('dbname', $database); } if (isset($hostname)) { - $dsn[] = "host={$hostname}"; + $dsn[] = 'host=' . $this->getDsnParameter('host', $hostname); } if (isset($port)) { - $dsn[] = "port={$port}"; + $dsn[] = 'port=' . $port; } if (isset($charset)) { - $dsn[] = "charset={$charset}"; + $dsn[] = 'charset=' . $this->getDsnParameter('charset', $charset); } if (isset($unixSocket)) { - $dsn[] = "unix_socket={$unixSocket}"; + $dsn[] = 'unix_socket=' . $this->getDsnParameter('unix_socket', $unixSocket); } if (isset($version)) { - $dsn[] = "version={$version}"; + $dsn[] = 'version=' . $this->getDsnParameter('version', $version); } $dsn = 'mysql:' . implode(';', $dsn); } diff --git a/src/Sql/ColumnFormatEnum.php b/src/Sql/ColumnFormatEnum.php new file mode 100644 index 0000000..ca40b1c --- /dev/null +++ b/src/Sql/ColumnFormatEnum.php @@ -0,0 +1,42 @@ + $case->value, self::cases())), + is_string($value) ? $value : get_debug_type($value) + )); + } +} diff --git a/src/Sql/Ddl/AlterTableDecorator.php b/src/Sql/Ddl/AlterTableDecorator.php index 0dd89b6..510e245 100644 --- a/src/Sql/Ddl/AlterTableDecorator.php +++ b/src/Sql/Ddl/AlterTableDecorator.php @@ -10,49 +10,11 @@ use PhpDb\Sql\PreparableSqlInterface; use PhpDb\Sql\SqlInterface; -use function count; -use function range; -use function str_replace; -use function strlen; -use function strpos; -use function strtolower; -use function strtoupper; -use function substr_replace; -use function uksort; - final class AlterTableDecorator extends AlterTable implements PlatformDecoratorInterface { - protected SqlInterface|PreparableSqlInterface|null $subject; + use ColumnOptionTrait; - /** @var array{ - * unsigned: int, - * zerofill: int, - * charset: int, - * collate: int, - * identity: int, - * serial: int, - * autoincrement: int, - * comment: int, - * columnformat: int, - * format: int, - * storage: int, - * after: int - * } $columnOptionSortOrder - */ - protected array $columnOptionSortOrder = [ - 'unsigned' => 0, - 'zerofill' => 1, - 'charset' => 2, - 'collate' => 3, - 'identity' => 4, - 'serial' => 4, - 'autoincrement' => 4, - 'comment' => 5, - 'columnformat' => 6, - 'format' => 6, - 'storage' => 7, - 'after' => 8, - ]; + protected SqlInterface|PreparableSqlInterface|null $subject; public function setSubject( SqlInterface|PreparableSqlInterface|null $subject @@ -62,176 +24,34 @@ public function setSubject( return $this; } - protected function getSqlInsertOffsets(string $sql): array - { - $sqlLength = strlen($sql); - $insertStart = []; - - foreach (['NOT NULL', 'NULL', 'DEFAULT', 'UNIQUE', 'PRIMARY', 'REFERENCES'] as $needle) { - $insertPos = strpos($sql, ' ' . $needle); - - if ($insertPos !== false) { - switch ($needle) { - case 'REFERENCES': - $insertStart[2] = ! isset($insertStart[2]) ? $insertPos : $insertStart[2]; - // no break - case 'PRIMARY': - case 'UNIQUE': - $insertStart[1] = ! isset($insertStart[1]) ? $insertPos : $insertStart[1]; - // no break - default: - $insertStart[0] = ! isset($insertStart[0]) ? $insertPos : $insertStart[0]; - } - } - } - - foreach (range(0, 3) as $i) { - $insertStart[$i] = $insertStart[$i] ?? $sqlLength; - } - - return $insertStart; - } - protected function processAddColumns(?PlatformInterface $adapterPlatform = null): array { $sqls = []; foreach ($this->addColumns as $i => $column) { - $sql = $this->processExpression($column, $adapterPlatform); - $insertStart = $this->getSqlInsertOffsets($sql); - $columnOptions = $column->getOptions(); - - uksort($columnOptions, [$this, 'compareColumnOptions']); - - foreach ($columnOptions as $coName => $coValue) { - $insert = ''; - - if (! $coValue) { - continue; - } - - switch ($this->normalizeColumnOption($coName)) { - case 'unsigned': - $insert = ' UNSIGNED'; - $j = 0; - break; - case 'zerofill': - $insert = ' ZEROFILL'; - $j = 0; - break; - case 'charset': - $insert = ' CHARACTER SET ' . $coValue; - $j = 0; - break; - case 'collate': - $insert = ' COLLATE ' . $coValue; - $j = 0; - break; - case 'identity': - case 'serial': - case 'autoincrement': - $insert = ' AUTO_INCREMENT'; - $j = 1; - break; - case 'comment': - $insert = ' COMMENT ' . $adapterPlatform->quoteValue($coValue); - $j = 2; - break; - case 'columnformat': - case 'format': - $insert = ' COLUMN_FORMAT ' . strtoupper($coValue); - $j = 2; - break; - case 'storage': - $insert = ' STORAGE ' . strtoupper($coValue); - $j = 2; - break; - case 'after': - $insert = ' AFTER ' . $adapterPlatform->quoteIdentifier($coValue); - $j = 2; - } - - if ($insert) { - $j = $j ?? 0; - $sql = substr_replace($sql, $insert, $insertStart[$j], 0); - $insertStartCount = count($insertStart); - for (; $j < $insertStartCount; ++$j) { - $insertStart[$j] += strlen($insert); - } - } - } - $sqls[$i] = $sql; + $sqls[$i] = $this->processColumnOptions( + $this->processExpression($column, $adapterPlatform), + $column->getOptions(), + $adapterPlatform, + $this->resolveAfterOption(...) + ); } + return [$sqls]; } protected function processChangeColumns(?PlatformInterface $adapterPlatform = null): array { $sqls = []; - foreach ($this->changeColumns as $name => $column) { - $sql = $this->processExpression($column, $adapterPlatform); - $insertStart = $this->getSqlInsertOffsets($sql); - $columnOptions = $column->getOptions(); - - uksort($columnOptions, [$this, 'compareColumnOptions']); - - foreach ($columnOptions as $coName => $coValue) { - $insert = ''; - - if (! $coValue) { - continue; - } - - switch ($this->normalizeColumnOption($coName)) { - case 'unsigned': - $insert = ' UNSIGNED'; - $j = 0; - break; - case 'zerofill': - $insert = ' ZEROFILL'; - $j = 0; - break; - case 'charset': - $insert = ' CHARACTER SET ' . $coValue; - $j = 0; - break; - case 'collate': - $insert = ' COLLATE ' . $coValue; - $j = 0; - break; - case 'identity': - case 'serial': - case 'autoincrement': - $insert = ' AUTO_INCREMENT'; - $j = 1; - break; - case 'comment': - $insert = ' COMMENT ' . $adapterPlatform->quoteValue($coValue); - $j = 2; - break; - case 'columnformat': - case 'format': - $insert = ' COLUMN_FORMAT ' . strtoupper($coValue); - $j = 2; - break; - case 'storage': - $insert = ' STORAGE ' . strtoupper($coValue); - $j = 2; - break; - } - if ($insert) { - $j = $j ?? 0; - $sql = substr_replace($sql, $insert, $insertStart[$j], 0); - $insertStartCount = count($insertStart); - for (; $j < $insertStartCount; ++$j) { - $insertStart[$j] += strlen($insert); - } - } - } + foreach ($this->changeColumns as $name => $column) { $sqls[] = [ $adapterPlatform->quoteIdentifier($name), - $sql, + $this->processColumnOptions( + $this->processExpression($column, $adapterPlatform), + $column->getOptions(), + $adapterPlatform + ), ]; } @@ -239,28 +59,12 @@ protected function processChangeColumns(?PlatformInterface $adapterPlatform = nu } /** - * @param string $name - * @return string - */ - private function normalizeColumnOption($name) - { - return strtolower(str_replace(['-', '_', ' '], '', $name)); - } - - /** - * @param string $columnA - * @param string $columnB - * @return int + * @return array{string, int}|null */ - // phpcs:ignore SlevomatCodingStandard.Classes.UnusedPrivateElements.UnusedMethod - private function compareColumnOptions($columnA, $columnB) + private function resolveAfterOption(string $option, mixed $value, ?PlatformInterface $platform): ?array { - $columnA = $this->normalizeColumnOption($columnA); - $columnA = $this->columnOptionSortOrder[$columnA] ?? count($this->columnOptionSortOrder); - - $columnB = $this->normalizeColumnOption($columnB); - $columnB = $this->columnOptionSortOrder[$columnB] ?? count($this->columnOptionSortOrder); - - return $columnA - $columnB; + return $option === 'after' + ? [' AFTER ' . $platform->quoteIdentifier($value), 2] + : null; } } diff --git a/src/Sql/Ddl/ColumnOptionTrait.php b/src/Sql/Ddl/ColumnOptionTrait.php new file mode 100644 index 0000000..96cb224 --- /dev/null +++ b/src/Sql/Ddl/ColumnOptionTrait.php @@ -0,0 +1,184 @@ + $columnOptionSortOrder Order options are emitted in, lowest first. */ + protected array $columnOptionSortOrder = [ + 'unsigned' => 0, + 'zerofill' => 1, + 'charset' => 2, + 'collate' => 3, + 'identity' => 4, + 'serial' => 4, + 'autoincrement' => 4, + 'comment' => 5, + 'columnformat' => 6, + 'format' => 6, + 'storage' => 7, + 'after' => 8, + ]; + + /** + * Appends each option to $sql at the offset its keyword belongs to. + * + * @param array $options + * @param (callable(string, mixed, ?PlatformInterface): ?array{string, int})|null $resolveExtra + * Resolver for options only valid in the calling statement, tried before the common ones. + */ + protected function processColumnOptions( + string $sql, + array $options, + ?PlatformInterface $platform = null, + ?callable $resolveExtra = null + ): string { + $insertStart = $this->getSqlInsertOffsets($sql); + + uksort($options, $this->compareColumnOptions(...)); + + foreach ($options as $name => $value) { + if (! $value) { + continue; + } + + $option = $this->normalizeColumnOption($name); + $resolved = $resolveExtra !== null ? $resolveExtra($option, $value, $platform) : null; + $resolved ??= $this->resolveColumnOption($option, $value, $platform); + + if ($resolved === null) { + continue; + } + + [$insert, $j] = $resolved; + + $sql = substr_replace($sql, $insert, $insertStart[$j], length: 0); + $insertStartCount = count($insertStart); + + for (; $j < $insertStartCount; ++$j) { + $insertStart[$j] += strlen($insert); + } + } + + return $sql; + } + + /** + * @return array{string, int}|null The SQL to insert and the offset index it belongs at. + * @throws InvalidArgumentException If the option value would not be safe to emit unquoted. + */ + private function resolveColumnOption(string $option, mixed $value, ?PlatformInterface $platform): ?array + { + return match ($option) { + 'unsigned' => [' UNSIGNED', 0], + 'zerofill' => [' ZEROFILL', 0], + 'charset' => [' CHARACTER SET ' . $this->getColumnOptionName('charset', $value), 0], + 'collate' => [' COLLATE ' . $this->getColumnOptionName('collate', $value), 0], + 'identity', 'serial', 'autoincrement' => [' AUTO_INCREMENT', 1], + 'comment' => [' COMMENT ' . $platform->quoteValue($value), 2], + 'columnformat', 'format' => [' COLUMN_FORMAT ' . ColumnFormatEnum::getOptionValue($value)->value, 2], + 'storage' => [' STORAGE ' . StorageEnum::getOptionValue($value)->value, 2], + default => null, + }; + } + + /** + * @return string The validated name, unchanged. + * @throws InvalidArgumentException If the value is not a bare character set or collation name. + */ + private function getColumnOptionName(string $option, mixed $value): string + { + if (! is_string($value) || preg_match(self::NAME_PATTERN, $value) !== 1) { + throw new InvalidArgumentException(sprintf( + 'Invalid value for the "%s" column option; expected an unquoted name matching %s, received "%s"', + $option, + self::NAME_PATTERN, + is_string($value) ? $value : get_debug_type($value) + )); + } + + return $value; + } + + /** @return array Offsets keyed by how late in the definition an option may be inserted. */ + protected function getSqlInsertOffsets(string $sql): array + { + $sqlLength = strlen($sql); + $insertStart = []; + + foreach (['NOT NULL', 'NULL', 'DEFAULT', 'UNIQUE', 'PRIMARY', 'REFERENCES'] as $needle) { + $insertPos = strpos($sql, ' ' . $needle); + + if ($insertPos !== false) { + switch ($needle) { + case 'REFERENCES': + $insertStart[2] = ! isset($insertStart[2]) ? $insertPos : $insertStart[2]; + // no break + case 'PRIMARY': + case 'UNIQUE': + $insertStart[1] = ! isset($insertStart[1]) ? $insertPos : $insertStart[1]; + // no break + default: + $insertStart[0] = ! isset($insertStart[0]) ? $insertPos : $insertStart[0]; + } + } + } + + foreach (range(0, 3) as $i) { + $insertStart[$i] = $insertStart[$i] ?? $sqlLength; + } + + return $insertStart; + } + + private function normalizeColumnOption(string $name): string + { + return strtolower(str_replace(['-', '_', ' '], '', $name)); + } + + private function compareColumnOptions(string $columnA, string $columnB): int + { + $columnA = $this->normalizeColumnOption($columnA); + $columnA = $this->columnOptionSortOrder[$columnA] ?? count($this->columnOptionSortOrder); + + $columnB = $this->normalizeColumnOption($columnB); + $columnB = $this->columnOptionSortOrder[$columnB] ?? count($this->columnOptionSortOrder); + + return $columnA - $columnB; + } +} diff --git a/src/Sql/Ddl/CreateTableDecorator.php b/src/Sql/Ddl/CreateTableDecorator.php index fd1f575..e9aaaba 100644 --- a/src/Sql/Ddl/CreateTableDecorator.php +++ b/src/Sql/Ddl/CreateTableDecorator.php @@ -10,34 +10,11 @@ use PhpDb\Sql\PreparableSqlInterface; use PhpDb\Sql\SqlInterface; -use function count; -use function range; -use function str_replace; -use function strlen; -use function strpos; -use function strtolower; -use function strtoupper; -use function substr_replace; -use function uksort; - final class CreateTableDecorator extends CreateTable implements PlatformDecoratorInterface { - protected SqlInterface|PreparableSqlInterface|null $subject; + use ColumnOptionTrait; - /** @var int[] */ - protected $columnOptionSortOrder = [ - 'unsigned' => 0, - 'zerofill' => 1, - 'charset' => 2, - 'collate' => 3, - 'identity' => 4, - 'serial' => 4, - 'autoincrement' => 4, - 'comment' => 5, - 'columnformat' => 6, - 'format' => 6, - 'storage' => 7, - ]; + protected SqlInterface|PreparableSqlInterface|null $subject; public function setSubject( PreparableSqlInterface|SqlInterface|null $subject @@ -47,40 +24,6 @@ public function setSubject( return $this; } - /** - * @param string $sql - * @return array - */ - protected function getSqlInsertOffsets($sql) - { - $sqlLength = strlen($sql); - $insertStart = []; - - foreach (['NOT NULL', 'NULL', 'DEFAULT', 'UNIQUE', 'PRIMARY', 'REFERENCES'] as $needle) { - $insertPos = strpos($sql, ' ' . $needle); - - if ($insertPos !== false) { - switch ($needle) { - case 'REFERENCES': - $insertStart[2] = ! isset($insertStart[2]) ? $insertPos : $insertStart[2]; - // no break - case 'PRIMARY': - case 'UNIQUE': - $insertStart[1] = ! isset($insertStart[1]) ? $insertPos : $insertStart[1]; - // no break - default: - $insertStart[0] = ! isset($insertStart[0]) ? $insertPos : $insertStart[0]; - } - } - } - - foreach (range(0, 3) as $i) { - $insertStart[$i] = $insertStart[$i] ?? $sqlLength; - } - - return $insertStart; - } - /** * {@inheritDoc} */ @@ -93,96 +36,13 @@ protected function processColumns(?PlatformInterface $platform = null): ?array $sqls = []; foreach ($this->columns as $i => $column) { - $sql = $this->processExpression($column, $platform); - $insertStart = $this->getSqlInsertOffsets($sql); - $columnOptions = $column->getOptions(); - - uksort($columnOptions, [$this, 'compareColumnOptions']); - - foreach ($columnOptions as $coName => $coValue) { - $insert = ''; - - if (! $coValue) { - continue; - } - - switch ($this->normalizeColumnOption($coName)) { - case 'unsigned': - $insert = ' UNSIGNED'; - $j = 0; - break; - case 'zerofill': - $insert = ' ZEROFILL'; - $j = 0; - break; - case 'charset': - $insert = ' CHARACTER SET ' . $coValue; - $j = 0; - break; - case 'collate': - $insert = ' COLLATE ' . $coValue; - $j = 0; - break; - case 'identity': - case 'serial': - case 'autoincrement': - $insert = ' AUTO_INCREMENT'; - $j = 1; - break; - case 'comment': - $insert = ' COMMENT ' . $platform->quoteValue($coValue); - $j = 2; - break; - case 'columnformat': - case 'format': - $insert = ' COLUMN_FORMAT ' . strtoupper($coValue); - $j = 2; - break; - case 'storage': - $insert = ' STORAGE ' . strtoupper($coValue); - $j = 2; - break; - } - - if ($insert) { - $j = $j ?? 0; - $sql = substr_replace($sql, $insert, $insertStart[$j], 0); - $insertStartCount = count($insertStart); - for (; $j < $insertStartCount; ++$j) { - $insertStart[$j] += strlen($insert); - } - } - } - - $sqls[$i] = $sql; + $sqls[$i] = $this->processColumnOptions( + $this->processExpression($column, $platform), + $column->getOptions(), + $platform + ); } return [$sqls]; } - - /** - * @param string $name - * @return string - */ - private function normalizeColumnOption($name) - { - return strtolower(str_replace(['-', '_', ' '], '', $name)); - } - - /** - * @param string $columnA - * @param string $columnB - * @return int - */ - // phpcs:ignore SlevomatCodingStandard.Classes.UnusedPrivateElements.UnusedMethod - private function compareColumnOptions($columnA, $columnB) - { - $columnA = $this->normalizeColumnOption($columnA); - $columnA = $this->columnOptionSortOrder[$columnA] ?? count($this->columnOptionSortOrder); - - $columnB = $this->normalizeColumnOption($columnB); - $columnB = $this->columnOptionSortOrder[$columnB] ?? count($this->columnOptionSortOrder); - - return $columnA - $columnB; - } } diff --git a/src/Sql/StorageEnum.php b/src/Sql/StorageEnum.php new file mode 100644 index 0000000..abd1d74 --- /dev/null +++ b/src/Sql/StorageEnum.php @@ -0,0 +1,43 @@ + $case->value, self::cases())), + is_string($value) ? $value : get_debug_type($value) + )); + } +} diff --git a/test/unit/Pdo/ConnectionTest.php b/test/unit/Pdo/ConnectionTest.php index 61be739..471d254 100644 --- a/test/unit/Pdo/ConnectionTest.php +++ b/test/unit/Pdo/ConnectionTest.php @@ -10,11 +10,15 @@ use PhpDb\Adapter\Exception\RuntimeException; use PhpDb\Mysql\Pdo\Connection; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; +use function sprintf; + #[CoversMethod(Connection::class, 'getResource')] #[CoversMethod(Connection::class, 'getDsn')] +#[CoversMethod(Connection::class, 'getDsnParameter')] final class ConnectionTest extends TestCase { protected Connection $connection; @@ -93,4 +97,34 @@ public function testHostnameAndUnixSocketThrowsInvalidConnectionParametersExcept ]); $connection->connect(); } + + #[DataProvider('unsafeDsnParameterProvider')] + public function testRejectsConnectionParameterContainingDsnControlCharacters( + string $parameter, + string $value, + string $reportedParameter + ): void { + $this->expectException(InvalidConnectionParametersException::class); + $this->expectExceptionMessage( + sprintf('The "%s" connection parameter contains invalid characters', $reportedParameter) + ); + + $connection = new Connection([ + 'driver' => 'pdo_mysql', + $parameter => $value, + ]); + $connection->connect(); + } + + /** @return array */ + public static function unsafeDsnParameterProvider(): array + { + return [ + 'dbname appends parameter' => ['dbname', 'foo;host=attacker.example.com', 'dbname'], + 'host appends parameter' => ['host', '127.0.0.1;dbname=other', 'host'], + 'charset appends parameter' => ['charset', 'utf8;dbname=other', 'charset'], + 'unix_socket appends parameter' => ['unix_socket', '/tmp/mysql.sock;dbname=other', 'unix_socket'], + 'newline in host' => ['host', "127.0.0.1\nhost=attacker.example.com", 'host'], + ]; + } } diff --git a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php index d9f6809..54dfa77 100644 --- a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php @@ -12,9 +12,14 @@ use PhpDb\Mysql\Sql\Ddl\AlterTableDecorator; use PhpDb\Sql\Ddl\AlterTable; use PhpDb\Sql\Ddl\Column; +use PhpDb\Sql\Exception\InvalidArgumentException; +use PhpDbTest\Mysql\Sql\Ddl\TestAsset\ColumnOptionMatrix; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use function sprintf; + #[CoversMethod(AlterTableDecorator::class, 'processAddColumns')] #[CoversMethod(AlterTableDecorator::class, 'processChangeColumns')] #[CoversMethod(AlterTableDecorator::class, 'getSqlInsertOffsets')] @@ -160,4 +165,138 @@ public function testAddColumnUnsigned(): void self::assertStringContainsString('UNSIGNED', $sql); self::assertStringContainsString('AUTO_INCREMENT', $sql); } + + public function testAddColumnFormatAndStorage(): void + { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption('column_format', 'fixed'); + $col->setOption('storage', 'memory'); + $alter->addColumn($col); + + $sql = $this->buildSql($alter); + + self::assertStringContainsString('COLUMN_FORMAT FIXED STORAGE MEMORY', $sql); + } + + #[DataProvider('unsafeColumnOptionProvider')] + public function testAddColumnRejectsOptionValueThatWouldInjectSql( + string $option, + string $value, + string $reportedOption + ): void { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, $value); + $alter->addColumn($col); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $reportedOption)); + + $this->buildSql($alter); + } + + #[DataProvider('unsafeColumnOptionProvider')] + public function testChangeColumnRejectsOptionValueThatWouldInjectSql( + string $option, + string $value, + string $reportedOption + ): void { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, $value); + $alter->changeColumn('name', $col); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $reportedOption)); + + $this->buildSql($alter); + } + + /** @return array */ + public static function unsafeColumnOptionProvider(): array + { + return [ + 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], + 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], + 'collate statement terminator' => [ + 'collate', + 'utf8mb3_unicode_ci; DROP TABLE users; --', + 'collate', + ], + 'columnformat statement terminator' => ['column_format', 'FIXED; DROP TABLE users; --', 'columnformat'], + 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', 'columnformat'], + 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', 'storage'], + 'storage unknown keyword' => ['storage', 'TAPE', 'storage'], + ]; + } + + /** + * Pins the exact DDL produced for a matrix of column options on an added column. + * + * @param array $options + */ + #[DataProvider('addColumnMatrixProvider')] + public function testGeneratesExpectedSqlForAddedColumnOptions(array $options, string $expected): void + { + $alter = new AlterTable('test'); + $alter->addColumn($this->makeColumn($options)); + + self::assertSame($expected, $this->buildSql($alter)); + } + + /** + * Pins the exact DDL produced for a matrix of column options on a changed column. + * + * @param array $options + */ + #[DataProvider('changeColumnMatrixProvider')] + public function testGeneratesExpectedSqlForChangedColumnOptions(array $options, string $expected): void + { + $alter = new AlterTable('test'); + $alter->changeColumn('name', $this->makeColumn($options)); + + self::assertSame($expected, $this->buildSql($alter)); + } + + /** @param array $options */ + private function makeColumn(array $options): Column\Varchar + { + $col = new Column\Varchar('name', 255); + $col->setNullable(false); + + foreach ($options as $name => $value) { + $col->setOption($name, $value); + } + + return $col; + } + + /** @return array, string}> */ + public static function addColumnMatrixProvider(): array + { + return ColumnOptionMatrix::pairedWith([ + 'all options' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY AFTER `id`", + 'charset collate' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL", + 'format storage' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK", + 'reverse declared' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK", + 'unknown option' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL", + 'after only' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) NOT NULL AFTER `other_col`", + 'falsy skipped' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL", + ]); + } + + /** @return array, string}> */ + public static function changeColumnMatrixProvider(): array + { + return ColumnOptionMatrix::pairedWith([ + 'all options' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY", + 'charset collate' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL", + 'format storage' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK", + 'reverse declared' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK", + 'unknown option' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL", + 'after only' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) NOT NULL", + 'falsy skipped' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL", + ]); + } } diff --git a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php index 1aa25da..16969c1 100644 --- a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php @@ -13,9 +13,14 @@ use PhpDb\Sql\Ddl\Column; use PhpDb\Sql\Ddl\Constraint; use PhpDb\Sql\Ddl\CreateTable; +use PhpDb\Sql\Exception\InvalidArgumentException; +use PhpDbTest\Mysql\Sql\Ddl\TestAsset\ColumnOptionMatrix; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use function sprintf; + #[CoversMethod(CreateTableDecorator::class, 'processColumns')] #[CoversMethod(CreateTableDecorator::class, 'getSqlInsertOffsets')] final class CreateTableDecoratorTest extends TestCase @@ -156,4 +161,100 @@ public function testFullColumnDefinition(): void self::assertStringContainsString('AUTO_INCREMENT', $sql); self::assertStringContainsString('CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL', $sql); } + + public function testColumnFormatOption(): void + { + $table = new CreateTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption('column_format', 'dynamic'); + $table->addColumn($col); + + $sql = $this->buildSql($table); + + self::assertStringContainsString('COLUMN_FORMAT DYNAMIC', $sql); + } + + public function testStorageOption(): void + { + $table = new CreateTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption('storage', 'disk'); + $table->addColumn($col); + + $sql = $this->buildSql($table); + + self::assertStringContainsString('STORAGE DISK', $sql); + } + + #[DataProvider('unsafeColumnOptionProvider')] + public function testRejectsColumnOptionValueThatWouldInjectSql( + string $option, + string $value, + string $reportedOption + ): void { + $table = new CreateTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, $value); + $table->addColumn($col); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $reportedOption)); + + $this->buildSql($table); + } + + /** @return array */ + public static function unsafeColumnOptionProvider(): array + { + return [ + 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], + 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], + 'charset backtick' => ['charset', 'utf8mb3` DEFAULT `', 'charset'], + 'collate statement terminator' => [ + 'collate', + 'utf8mb3_unicode_ci; DROP TABLE users; --', + 'collate', + ], + 'collate trailing clause' => ['collate', 'utf8mb3_unicode_ci COMMENT "x"', 'collate'], + 'columnformat statement terminator' => ['column_format', 'FIXED; DROP TABLE users; --', 'columnformat'], + 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', 'columnformat'], + 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', 'storage'], + 'storage unknown keyword' => ['storage', 'TAPE', 'storage'], + ]; + } + + /** + * Pins the exact DDL produced for a matrix of column options. + * + * @param array $options + */ + #[DataProvider('columnOptionMatrixProvider')] + public function testGeneratesExpectedSqlForColumnOptions(array $options, string $expected): void + { + $table = new CreateTable('test'); + $col = new Column\Varchar('name', 255); + $col->setNullable(false); + + foreach ($options as $name => $value) { + $col->setOption($name, $value); + } + + $table->addColumn($col); + + self::assertSame($expected, $this->buildSql($table)); + } + + /** @return array, string}> */ + public static function columnOptionMatrixProvider(): array + { + return ColumnOptionMatrix::pairedWith([ + 'all options' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY \n)", + 'charset collate' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL \n)", + 'format storage' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK \n)", + 'reverse declared' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK \n)", + 'unknown option' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL \n)", + 'after only' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) NOT NULL \n)", + 'falsy skipped' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL \n)", + ]); + } } diff --git a/test/unit/Sql/Ddl/TestAsset/ColumnOptionMatrix.php b/test/unit/Sql/Ddl/TestAsset/ColumnOptionMatrix.php new file mode 100644 index 0000000..4498db6 --- /dev/null +++ b/test/unit/Sql/Ddl/TestAsset/ColumnOptionMatrix.php @@ -0,0 +1,58 @@ +> */ + public static function all(): array + { + return [ + 'all options' => [ + 'unsigned' => true, + 'zerofill' => true, + 'charset' => 'utf8mb4', + 'collate' => 'utf8mb4_bin', + 'auto_increment' => true, + 'comment' => 'here', + 'column_format' => 'dynamic', + 'storage' => 'memory', + 'after' => 'id', + ], + 'charset collate' => ['charset' => 'utf8mb3', 'collate' => 'utf8mb3_unicode_ci'], + 'format storage' => ['column_format' => 'fixed', 'storage' => 'disk'], + 'reverse declared' => [ + 'storage' => 'DISK', + 'comment' => 'c', + 'unsigned' => true, + 'charset' => 'latin1', + ], + 'unknown option' => ['charset' => 'utf8mb4', 'nonsense' => 'ignored'], + 'after only' => ['after' => 'other_col'], + 'falsy skipped' => ['charset' => '', 'unsigned' => false, 'collate' => 'utf8mb4_bin'], + ]; + } + + /** + * @param array $expected Keyed by the option set name. + * @return array, string}> + */ + public static function pairedWith(array $expected): array + { + $cases = []; + foreach (self::all() as $name => $options) { + $cases[$name] = [$options, $expected[$name]]; + } + + return $cases; + } +} From 0446b2a1713fe1bb6c8a3e13e1f9c19a530e73c5 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 20:24:01 +1000 Subject: [PATCH 02/17] Add @todo to promote getDsnParameter to AbstractPdoConnection The DSN parameter validation is generic to all semicolon-delimited PDO DSN formats and has no MySQL-specific dependencies, so it can move to php-db/phpdb once a second PDO driver package needs it. Signed-off-by: Simon Mundy --- src/Pdo/Connection.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Pdo/Connection.php b/src/Pdo/Connection.php index 8cddd81..5a2bf10 100644 --- a/src/Pdo/Connection.php +++ b/src/Pdo/Connection.php @@ -60,6 +60,10 @@ public function getCurrentSchema(): string|false /** * Return a value that is safe to interpolate into a generated DSN. * + * @todo Promote to AbstractPdoConnection in php-db/phpdb as a protected method once a second + * PDO driver package needs it — the validation is generic to all semicolon-delimited + * PDO DSN formats and has no MySQL-specific dependencies. + * * @throws Exception\InvalidConnectionParametersException If the value contains DSN control characters. */ private function getDsnParameter(string $name, string $value): string From 912ba8fab8ee5d2e8f32eb8019d49935d9830d76 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 21:37:10 +1000 Subject: [PATCH 03/17] Apply mago 1.47 formatting Formats the files this branch touched plus one straggler the mago-format-reformat pass missed (IntegrationTestStoppedListener). Whitespace, member ordering, and trailing commas only - no behavior change. Signed-off-by: Simon Mundy --- src/Pdo/Connection.php | 42 ++-- src/Sql/ColumnFormatEnum.php | 12 +- src/Sql/Ddl/AlterTableDecorator.php | 8 +- src/Sql/Ddl/ColumnOptionTrait.php | 124 +++++----- src/Sql/StorageEnum.php | 12 +- .../IntegrationTestStoppedListener.php | 2 +- test/unit/Pdo/ConnectionTest.php | 24 +- test/unit/Sql/Ddl/AlterTableDecoratorTest.php | 230 +++++++++--------- .../unit/Sql/Ddl/CreateTableDecoratorTest.php | 182 +++++++------- 9 files changed, 321 insertions(+), 315 deletions(-) diff --git a/src/Pdo/Connection.php b/src/Pdo/Connection.php index 2f20af3..ad8dcfc 100644 --- a/src/Pdo/Connection.php +++ b/src/Pdo/Connection.php @@ -50,27 +50,6 @@ public function __construct( $this->setResource($connectionParameters); } - /** - * Return a value that is safe to interpolate into a generated DSN. - * - * @todo Promote to AbstractPdoConnection in php-db/phpdb as a protected method once a second - * PDO driver package needs it — the validation is generic to all semicolon-delimited - * PDO DSN formats and has no MySQL-specific dependencies. - * - * @throws Exception\InvalidConnectionParametersException If the value contains DSN control characters. - */ - private function getDsnParameter(string $name, string $value): string - { - if (preg_match('/[;\x00-\x1f]/', $value) === 1) { - throw new Exception\InvalidConnectionParametersException( - sprintf('The "%s" connection parameter contains invalid characters', $name), - $this->connectionParameters - ); - } - - return $value; - } - /** * {@inheritDoc} * @@ -209,4 +188,25 @@ public function getLastGeneratedValue(?string $name = null): string|int|false|nu return false; } + + /** + * Return a value that is safe to interpolate into a generated DSN. + * + * @todo Promote to AbstractPdoConnection in php-db/phpdb as a protected method once a second + * PDO driver package needs it — the validation is generic to all semicolon-delimited + * PDO DSN formats and has no MySQL-specific dependencies. + * + * @throws Exception\InvalidConnectionParametersException If the value contains DSN control characters. + */ + private function getDsnParameter(string $name, string $value): string + { + if (preg_match('/[;\x00-\x1f]/', $value) === 1) { + throw new Exception\InvalidConnectionParametersException( + sprintf('The "%s" connection parameter contains invalid characters', $name), + $this->connectionParameters, + ); + } + + return $value; + } } diff --git a/src/Sql/ColumnFormatEnum.php b/src/Sql/ColumnFormatEnum.php index ca40b1c..fc2c46f 100644 --- a/src/Sql/ColumnFormatEnum.php +++ b/src/Sql/ColumnFormatEnum.php @@ -33,10 +33,12 @@ public static function getOptionValue(mixed $value): self { $keyword = is_string($value) ? strtoupper(trim($value)) : ''; - return self::tryFrom($keyword) ?? throw new InvalidArgumentException(sprintf( - 'Invalid value for the "columnformat" column option; expected one of %s, received "%s"', - implode(', ', array_map(static fn (self $case): string => $case->value, self::cases())), - is_string($value) ? $value : get_debug_type($value) - )); + return ( + self::tryFrom($keyword) ?? throw new InvalidArgumentException(sprintf( + 'Invalid value for the "columnformat" column option; expected one of %s, received "%s"', + implode(', ', array_map(static fn(self $case): string => $case->value, self::cases())), + is_string($value) ? $value : get_debug_type($value), + )) + ); } } diff --git a/src/Sql/Ddl/AlterTableDecorator.php b/src/Sql/Ddl/AlterTableDecorator.php index 72d55ab..5e3e19a 100644 --- a/src/Sql/Ddl/AlterTableDecorator.php +++ b/src/Sql/Ddl/AlterTableDecorator.php @@ -88,8 +88,10 @@ protected function processChangeColumns(?PlatformInterface $adapterPlatform = nu */ private function resolveAfterOption(string $option, mixed $value, ?PlatformInterface $platform): ?array { - return $option === 'after' - ? [' AFTER ' . $platform->quoteIdentifier($value), 2] - : null; + return ( + $option === 'after' + ? [' AFTER ' . $platform->quoteIdentifier($value), 2] + : null + ); } } diff --git a/src/Sql/Ddl/ColumnOptionTrait.php b/src/Sql/Ddl/ColumnOptionTrait.php index 959e96e..38fc9de 100644 --- a/src/Sql/Ddl/ColumnOptionTrait.php +++ b/src/Sql/Ddl/ColumnOptionTrait.php @@ -55,6 +55,42 @@ trait ColumnOptionTrait 'after' => 8, ]; + /** + * Offsets keyed by how late in the definition an option may be inserted. + * + * @return array{0: int, 1: int, 2: int, 3: int} + */ + protected function getSqlInsertOffsets(string $sql): array + { + $sqlLength = strlen($sql); + $insertStart = []; + + foreach (['NOT NULL', 'NULL', 'DEFAULT', 'UNIQUE', 'PRIMARY', 'REFERENCES'] as $needle) { + $insertPos = strpos($sql, " {$needle}"); + + if (false !== $insertPos) { + switch ($needle) { + case 'REFERENCES': + $insertStart[2] ??= $insertPos; + // no break + case 'PRIMARY': + case 'UNIQUE': + $insertStart[1] ??= $insertPos; + // no break + default: + $insertStart[0] ??= $insertPos; + } + } + } + + foreach (range(0, 3) as $i) { + $insertStart[$i] ??= $sqlLength; + } + + /** @var array{0: int, 1: int, 2: int, 3: int} $insertStart */ + return $insertStart; + } + /** * Appends each option to $sql at the offset its keyword belongs to. * @@ -66,7 +102,7 @@ protected function processColumnOptions( string $sql, array $options, ?PlatformInterface $platform = null, - ?callable $resolveExtra = null + ?callable $resolveExtra = null, ): string { $insertStart = $this->getSqlInsertOffsets($sql); @@ -98,23 +134,15 @@ protected function processColumnOptions( return $sql; } - /** - * @return array{string, int}|null The SQL to insert and the offset index it belongs at. - * @throws InvalidArgumentException If the option value would not be safe to emit unquoted. - */ - private function resolveColumnOption(string $option, mixed $value, ?PlatformInterface $platform): ?array + private function compareColumnOptions(string $columnA, string $columnB): int { - return match ($option) { - 'unsigned' => [' UNSIGNED', 0], - 'zerofill' => [' ZEROFILL', 0], - 'charset' => [' CHARACTER SET ' . $this->getColumnOptionName('charset', $value), 0], - 'collate' => [' COLLATE ' . $this->getColumnOptionName('collate', $value), 0], - 'identity', 'serial', 'autoincrement' => [' AUTO_INCREMENT', 1], - 'comment' => [' COMMENT ' . $platform->quoteValue($value), 2], - 'columnformat', 'format' => [' COLUMN_FORMAT ' . ColumnFormatEnum::getOptionValue($value)->value, 2], - 'storage' => [' STORAGE ' . StorageEnum::getOptionValue($value)->value, 2], - default => null, - }; + $columnA = $this->normalizeColumnOption($columnA); + $columnA = $this->columnOptionSortOrder[$columnA] ?? count($this->columnOptionSortOrder); + + $columnB = $this->normalizeColumnOption($columnB); + $columnB = $this->columnOptionSortOrder[$columnB] ?? count($this->columnOptionSortOrder); + + return $columnA - $columnB; } /** @@ -128,62 +156,34 @@ private function getColumnOptionName(string $option, mixed $value): string 'Invalid value for the "%s" column option; expected an unquoted name matching %s, received "%s"', $option, self::NAME_PATTERN, - is_string($value) ? $value : get_debug_type($value) + is_string($value) ? $value : get_debug_type($value), )); } return $value; } - /** - * Offsets keyed by how late in the definition an option may be inserted. - * - * @return array{0: int, 1: int, 2: int, 3: int} - */ - protected function getSqlInsertOffsets(string $sql): array - { - $sqlLength = strlen($sql); - $insertStart = []; - - foreach (['NOT NULL', 'NULL', 'DEFAULT', 'UNIQUE', 'PRIMARY', 'REFERENCES'] as $needle) { - $insertPos = strpos($sql, " {$needle}"); - - if (false !== $insertPos) { - switch ($needle) { - case 'REFERENCES': - $insertStart[2] ??= $insertPos; - // no break - case 'PRIMARY': - case 'UNIQUE': - $insertStart[1] ??= $insertPos; - // no break - default: - $insertStart[0] ??= $insertPos; - } - } - } - - foreach (range(0, 3) as $i) { - $insertStart[$i] ??= $sqlLength; - } - - /** @var array{0: int, 1: int, 2: int, 3: int} $insertStart */ - return $insertStart; - } - private function normalizeColumnOption(string $name): string { return strtolower(str_replace(['-', '_', ' '], '', $name)); } - private function compareColumnOptions(string $columnA, string $columnB): int + /** + * @return array{string, int}|null The SQL to insert and the offset index it belongs at. + * @throws InvalidArgumentException If the option value would not be safe to emit unquoted. + */ + private function resolveColumnOption(string $option, mixed $value, ?PlatformInterface $platform): ?array { - $columnA = $this->normalizeColumnOption($columnA); - $columnA = $this->columnOptionSortOrder[$columnA] ?? count($this->columnOptionSortOrder); - - $columnB = $this->normalizeColumnOption($columnB); - $columnB = $this->columnOptionSortOrder[$columnB] ?? count($this->columnOptionSortOrder); - - return $columnA - $columnB; + return match ($option) { + 'unsigned' => [' UNSIGNED', 0], + 'zerofill' => [' ZEROFILL', 0], + 'charset' => [' CHARACTER SET ' . $this->getColumnOptionName('charset', $value), 0], + 'collate' => [' COLLATE ' . $this->getColumnOptionName('collate', $value), 0], + 'identity', 'serial', 'autoincrement' => [' AUTO_INCREMENT', 1], + 'comment' => [' COMMENT ' . $platform->quoteValue($value), 2], + 'columnformat', 'format' => [' COLUMN_FORMAT ' . ColumnFormatEnum::getOptionValue($value)->value, 2], + 'storage' => [' STORAGE ' . StorageEnum::getOptionValue($value)->value, 2], + default => null, + }; } } diff --git a/src/Sql/StorageEnum.php b/src/Sql/StorageEnum.php index abd1d74..4911415 100644 --- a/src/Sql/StorageEnum.php +++ b/src/Sql/StorageEnum.php @@ -34,10 +34,12 @@ public static function getOptionValue(mixed $value): self { $keyword = is_string($value) ? strtoupper(trim($value)) : ''; - return self::tryFrom($keyword) ?? throw new InvalidArgumentException(sprintf( - 'Invalid value for the "storage" column option; expected one of %s, received "%s"', - implode(', ', array_map(static fn (self $case): string => $case->value, self::cases())), - is_string($value) ? $value : get_debug_type($value) - )); + return ( + self::tryFrom($keyword) ?? throw new InvalidArgumentException(sprintf( + 'Invalid value for the "storage" column option; expected one of %s, received "%s"', + implode(', ', array_map(static fn(self $case): string => $case->value, self::cases())), + is_string($value) ? $value : get_debug_type($value), + )) + ); } } diff --git a/test/integration/Extension/IntegrationTestStoppedListener.php b/test/integration/Extension/IntegrationTestStoppedListener.php index 7e6d6f3..603c161 100644 --- a/test/integration/Extension/IntegrationTestStoppedListener.php +++ b/test/integration/Extension/IntegrationTestStoppedListener.php @@ -19,7 +19,7 @@ public function notify(Finished $event): void { if ( $event->testSuite()->name() !== 'integration test' - || [] === $this->fixtureLoaders + || [] === $this->fixtureLoaders ) { return; } diff --git a/test/unit/Pdo/ConnectionTest.php b/test/unit/Pdo/ConnectionTest.php index a91ef9e..b31ef1d 100644 --- a/test/unit/Pdo/ConnectionTest.php +++ b/test/unit/Pdo/ConnectionTest.php @@ -22,6 +22,18 @@ final class ConnectionTest extends TestCase { protected Connection $connection; + /** @return array */ + public static function unsafeDsnParameterProvider(): array + { + return [ + 'dbname appends parameter' => ['dbname', 'foo;host=attacker.example.com', 'dbname'], + 'host appends parameter' => ['host', '127.0.0.1;dbname=other', 'host'], + 'charset appends parameter' => ['charset', 'utf8;dbname=other', 'charset'], + 'unix_socket appends parameter' => ['unix_socket', '/tmp/mysql.sock;dbname=other', 'unix_socket'], + 'newline in host' => ['host', "127.0.0.1\nhost=attacker.example.com", 'host'], + ]; + } + #[Test] #[Group('2622')] public function arrayOfConnectionParametersCreatesCorrectDsn(): void @@ -104,18 +116,6 @@ public function rejectsConnectionParameterContainingDsnControlCharacters( $connection->connect(); } - /** @return array */ - public static function unsafeDsnParameterProvider(): array - { - return [ - 'dbname appends parameter' => ['dbname', 'foo;host=attacker.example.com', 'dbname'], - 'host appends parameter' => ['host', '127.0.0.1;dbname=other', 'host'], - 'charset appends parameter' => ['charset', 'utf8;dbname=other', 'charset'], - 'unix_socket appends parameter' => ['unix_socket', '/tmp/mysql.sock;dbname=other', 'unix_socket'], - 'newline in host' => ['host', "127.0.0.1\nhost=attacker.example.com", 'host'], - ]; - } - /** * Test getResource method tries to connect to the database, it should never return null */ diff --git a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php index 01bc18d..240964e 100644 --- a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php @@ -31,6 +31,52 @@ final class AlterTableDecoratorTest extends TestCase { protected AdapterPlatform $platform; + /** @return array, string}> */ + public static function addColumnMatrixProvider(): array + { + return ColumnOptionMatrix::pairedWith([ + 'all options' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY AFTER `id`", + 'charset collate' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL", + 'format storage' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK", + 'reverse declared' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK", + 'unknown option' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL", + 'after only' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) NOT NULL AFTER `other_col`", + 'falsy skipped' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL", + ]); + } + + /** @return array, string}> */ + public static function changeColumnMatrixProvider(): array + { + return ColumnOptionMatrix::pairedWith([ + 'all options' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY", + 'charset collate' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL", + 'format storage' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK", + 'reverse declared' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK", + 'unknown option' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL", + 'after only' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) NOT NULL", + 'falsy skipped' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL", + ]); + } + + /** @return array */ + public static function unsafeColumnOptionProvider(): array + { + return [ + 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], + 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], + 'collate statement terminator' => [ + 'collate', + 'utf8mb3_unicode_ci; DROP TABLE users; --', + 'collate', + ], + 'columnformat statement terminator' => ['column_format', 'FIXED; DROP TABLE users; --', 'columnformat'], + 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', 'columnformat'], + 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', 'storage'], + 'storage unknown keyword' => ['storage', 'TAPE', 'storage'], + ]; + } + #[Test] public function addColumnAfter(): void { @@ -293,46 +339,28 @@ public function changeColumnZerofill(): void static::assertStringContainsString('ZEROFILL', $this->buildSql($alter)); } - protected function setUp(): void - { - $driver = new Driver( - $this->createStub(AbstractPdoConnection::class), - $this->createStub(Statement::class), - $this->createStub(Result::class), - ); - $this->platform = new AdapterPlatform($driver); - } - - private function buildSql(AlterTable $table): string - { - $decorator = new AlterTableDecorator(); - $decorator->setSubject($table); - - return $decorator->getSqlString($this->platform); - } - - public function testAddColumnCharset(): void + public function testAddColumnAfter(): void { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); - $col->setOption('charset', 'utf8mb3'); + $col->setOption('after', 'id'); $alter->addColumn($col); $sql = $this->buildSql($alter); - self::assertStringContainsString('CHARACTER SET utf8mb3', $sql); + self::assertStringContainsString('AFTER `id`', $sql); } - public function testAddColumnCollate(): void + public function testAddColumnCharset(): void { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); - $col->setOption('collate', 'utf8mb3_unicode_ci'); + $col->setOption('charset', 'utf8mb3'); $alter->addColumn($col); $sql = $this->buildSql($alter); - self::assertStringContainsString('COLLATE utf8mb3_unicode_ci', $sql); + self::assertStringContainsString('CHARACTER SET utf8mb3', $sql); } public function testAddColumnCharsetAndCollate(): void @@ -365,57 +393,46 @@ public function testAddColumnCharsetBeforeNotNull(): void ); } - public function testChangeColumnCharset(): void - { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('charset', 'utf8mb3'); - $alter->changeColumn('name', $col); - - $sql = $this->buildSql($alter); - - self::assertStringContainsString('CHARACTER SET utf8mb3', $sql); - } - - public function testChangeColumnCollate(): void + public function testAddColumnCollate(): void { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); $col->setOption('collate', 'utf8mb3_unicode_ci'); - $alter->changeColumn('name', $col); + $alter->addColumn($col); $sql = $this->buildSql($alter); self::assertStringContainsString('COLLATE utf8mb3_unicode_ci', $sql); } - public function testChangeColumnCharsetAndCollate(): void + public function testAddColumnFormatAndStorage(): void { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); - $col->setNullable(false); - $col->setOption('charset', 'utf8mb3'); - $col->setOption('collate', 'utf8mb3_unicode_ci'); - $alter->changeColumn('name', $col); + $col->setOption('column_format', 'fixed'); + $col->setOption('storage', 'memory'); + $alter->addColumn($col); $sql = $this->buildSql($alter); - self::assertMatchesRegularExpression( - '/CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL/', - $sql, - ); + self::assertStringContainsString('COLUMN_FORMAT FIXED STORAGE MEMORY', $sql); } - public function testAddColumnAfter(): void - { + #[DataProvider('unsafeColumnOptionProvider')] + public function testAddColumnRejectsOptionValueThatWouldInjectSql( + string $option, + string $value, + string $reportedOption, + ): void { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); - $col->setOption('after', 'id'); + $col->setOption($option, $value); $alter->addColumn($col); - $sql = $this->buildSql($alter); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $reportedOption)); - self::assertStringContainsString('AFTER `id`', $sql); + $this->buildSql($alter); } public function testAddColumnUnsigned(): void @@ -432,41 +449,52 @@ public function testAddColumnUnsigned(): void self::assertStringContainsString('AUTO_INCREMENT', $sql); } - public function testAddColumnFormatAndStorage(): void + public function testChangeColumnCharset(): void { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); - $col->setOption('column_format', 'fixed'); - $col->setOption('storage', 'memory'); - $alter->addColumn($col); + $col->setOption('charset', 'utf8mb3'); + $alter->changeColumn('name', $col); $sql = $this->buildSql($alter); - self::assertStringContainsString('COLUMN_FORMAT FIXED STORAGE MEMORY', $sql); + self::assertStringContainsString('CHARACTER SET utf8mb3', $sql); } - #[DataProvider('unsafeColumnOptionProvider')] - public function testAddColumnRejectsOptionValueThatWouldInjectSql( - string $option, - string $value, - string $reportedOption - ): void { + public function testChangeColumnCharsetAndCollate(): void + { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); - $col->setOption($option, $value); - $alter->addColumn($col); + $col->setNullable(false); + $col->setOption('charset', 'utf8mb3'); + $col->setOption('collate', 'utf8mb3_unicode_ci'); + $alter->changeColumn('name', $col); - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $reportedOption)); + $sql = $this->buildSql($alter); - $this->buildSql($alter); + self::assertMatchesRegularExpression( + '/CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL/', + $sql, + ); + } + + public function testChangeColumnCollate(): void + { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption('collate', 'utf8mb3_unicode_ci'); + $alter->changeColumn('name', $col); + + $sql = $this->buildSql($alter); + + self::assertStringContainsString('COLLATE utf8mb3_unicode_ci', $sql); } #[DataProvider('unsafeColumnOptionProvider')] public function testChangeColumnRejectsOptionValueThatWouldInjectSql( string $option, string $value, - string $reportedOption + string $reportedOption, ): void { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); @@ -479,24 +507,6 @@ public function testChangeColumnRejectsOptionValueThatWouldInjectSql( $this->buildSql($alter); } - /** @return array */ - public static function unsafeColumnOptionProvider(): array - { - return [ - 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], - 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], - 'collate statement terminator' => [ - 'collate', - 'utf8mb3_unicode_ci; DROP TABLE users; --', - 'collate', - ], - 'columnformat statement terminator' => ['column_format', 'FIXED; DROP TABLE users; --', 'columnformat'], - 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', 'columnformat'], - 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', 'storage'], - 'storage unknown keyword' => ['storage', 'TAPE', 'storage'], - ]; - } - /** * Pins the exact DDL produced for a matrix of column options on an added column. * @@ -525,6 +535,24 @@ public function testGeneratesExpectedSqlForChangedColumnOptions(array $options, self::assertSame($expected, $this->buildSql($alter)); } + protected function setUp(): void + { + $driver = new Driver( + $this->createStub(AbstractPdoConnection::class), + $this->createStub(Statement::class), + $this->createStub(Result::class), + ); + $this->platform = new AdapterPlatform($driver); + } + + private function buildSql(AlterTable $table): string + { + $decorator = new AlterTableDecorator(); + $decorator->setSubject($table); + + return $decorator->getSqlString($this->platform); + } + /** @param array $options */ private function makeColumn(array $options): Column\Varchar { @@ -537,32 +565,4 @@ private function makeColumn(array $options): Column\Varchar return $col; } - - /** @return array, string}> */ - public static function addColumnMatrixProvider(): array - { - return ColumnOptionMatrix::pairedWith([ - 'all options' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY AFTER `id`", - 'charset collate' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL", - 'format storage' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK", - 'reverse declared' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK", - 'unknown option' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL", - 'after only' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) NOT NULL AFTER `other_col`", - 'falsy skipped' => "ALTER TABLE `test`\n ADD COLUMN `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL", - ]); - } - - /** @return array, string}> */ - public static function changeColumnMatrixProvider(): array - { - return ColumnOptionMatrix::pairedWith([ - 'all options' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY", - 'charset collate' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL", - 'format storage' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK", - 'reverse declared' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK", - 'unknown option' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL", - 'after only' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) NOT NULL", - 'falsy skipped' => "ALTER TABLE `test`\n CHANGE COLUMN `name` `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL", - ]); - } } diff --git a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php index 31f718f..75f35a0 100644 --- a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php @@ -31,6 +31,40 @@ final class CreateTableDecoratorTest extends TestCase { protected AdapterPlatform $platform; + /** @return array, string}> */ + public static function columnOptionMatrixProvider(): array + { + return ColumnOptionMatrix::pairedWith([ + 'all options' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY \n)", + 'charset collate' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL \n)", + 'format storage' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK \n)", + 'reverse declared' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK \n)", + 'unknown option' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL \n)", + 'after only' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) NOT NULL \n)", + 'falsy skipped' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL \n)", + ]); + } + + /** @return array */ + public static function unsafeColumnOptionProvider(): array + { + return [ + 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], + 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], + 'charset backtick' => ['charset', 'utf8mb3` DEFAULT `', 'charset'], + 'collate statement terminator' => [ + 'collate', + 'utf8mb3_unicode_ci; DROP TABLE users; --', + 'collate', + ], + 'collate trailing clause' => ['collate', 'utf8mb3_unicode_ci COMMENT "x"', 'collate'], + 'columnformat statement terminator' => ['column_format', 'FIXED; DROP TABLE users; --', 'columnformat'], + 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', 'columnformat'], + 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', 'storage'], + 'storage unknown keyword' => ['storage', 'TAPE', 'storage'], + ]; + } + #[Test] public function charsetAppearsAfterUnsigned(): void { @@ -174,50 +208,6 @@ public function storageOption(): void static::assertStringContainsString('STORAGE DISK', $this->buildSql($table)); } - #[Test] - public function unsignedOption(): void - { - $table = new CreateTable('test'); - $col = new Column\Integer('id'); - $col->setOption('unsigned', true); - $col->setOption('auto_increment', true); - $table->addColumn($col); - - $sql = $this->buildSql($table); - - static::assertStringContainsString('UNSIGNED', $sql); - static::assertStringContainsString('AUTO_INCREMENT', $sql); - } - - #[Test] - public function zerofillOption(): void - { - $table = new CreateTable('test'); - $col = new Column\Integer('id'); - $col->setOption('zerofill', true); - $table->addColumn($col); - - static::assertStringContainsString('ZEROFILL', $this->buildSql($table)); - } - - protected function setUp(): void - { - $driver = new Driver( - $this->createStub(AbstractPdoConnection::class), - $this->createStub(Statement::class), - $this->createStub(Result::class), - ); - $this->platform = new AdapterPlatform($driver); - } - - private function buildSql(CreateTable $table): string - { - $decorator = new CreateTableDecorator(); - $decorator->setSubject($table); - - return $decorator->getSqlString($this->platform); - } - public function testColumnFormatOption(): void { $table = new CreateTable('test'); @@ -230,23 +220,32 @@ public function testColumnFormatOption(): void self::assertStringContainsString('COLUMN_FORMAT DYNAMIC', $sql); } - public function testStorageOption(): void + /** + * Pins the exact DDL produced for a matrix of column options. + * + * @param array $options + */ + #[DataProvider('columnOptionMatrixProvider')] + public function testGeneratesExpectedSqlForColumnOptions(array $options, string $expected): void { $table = new CreateTable('test'); $col = new Column\Varchar('name', 255); - $col->setOption('storage', 'disk'); - $table->addColumn($col); + $col->setNullable(false); - $sql = $this->buildSql($table); + foreach ($options as $name => $value) { + $col->setOption($name, $value); + } - self::assertStringContainsString('STORAGE DISK', $sql); + $table->addColumn($col); + + self::assertSame($expected, $this->buildSql($table)); } #[DataProvider('unsafeColumnOptionProvider')] public function testRejectsColumnOptionValueThatWouldInjectSql( string $option, string $value, - string $reportedOption + string $reportedOption, ): void { $table = new CreateTable('test'); $col = new Column\Varchar('name', 255); @@ -259,58 +258,59 @@ public function testRejectsColumnOptionValueThatWouldInjectSql( $this->buildSql($table); } - /** @return array */ - public static function unsafeColumnOptionProvider(): array + public function testStorageOption(): void { - return [ - 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], - 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], - 'charset backtick' => ['charset', 'utf8mb3` DEFAULT `', 'charset'], - 'collate statement terminator' => [ - 'collate', - 'utf8mb3_unicode_ci; DROP TABLE users; --', - 'collate', - ], - 'collate trailing clause' => ['collate', 'utf8mb3_unicode_ci COMMENT "x"', 'collate'], - 'columnformat statement terminator' => ['column_format', 'FIXED; DROP TABLE users; --', 'columnformat'], - 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', 'columnformat'], - 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', 'storage'], - 'storage unknown keyword' => ['storage', 'TAPE', 'storage'], - ]; + $table = new CreateTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption('storage', 'disk'); + $table->addColumn($col); + + $sql = $this->buildSql($table); + + self::assertStringContainsString('STORAGE DISK', $sql); } - /** - * Pins the exact DDL produced for a matrix of column options. - * - * @param array $options - */ - #[DataProvider('columnOptionMatrixProvider')] - public function testGeneratesExpectedSqlForColumnOptions(array $options, string $expected): void + #[Test] + public function unsignedOption(): void { $table = new CreateTable('test'); - $col = new Column\Varchar('name', 255); - $col->setNullable(false); + $col = new Column\Integer('id'); + $col->setOption('unsigned', true); + $col->setOption('auto_increment', true); + $table->addColumn($col); - foreach ($options as $name => $value) { - $col->setOption($name, $value); - } + $sql = $this->buildSql($table); + + static::assertStringContainsString('UNSIGNED', $sql); + static::assertStringContainsString('AUTO_INCREMENT', $sql); + } + #[Test] + public function zerofillOption(): void + { + $table = new CreateTable('test'); + $col = new Column\Integer('id'); + $col->setOption('zerofill', true); $table->addColumn($col); - self::assertSame($expected, $this->buildSql($table)); + static::assertStringContainsString('ZEROFILL', $this->buildSql($table)); } - /** @return array, string}> */ - public static function columnOptionMatrixProvider(): array + protected function setUp(): void { - return ColumnOptionMatrix::pairedWith([ - 'all options' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) UNSIGNED ZEROFILL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL AUTO_INCREMENT COMMENT 'here' COLUMN_FORMAT DYNAMIC STORAGE MEMORY \n)", - 'charset collate' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL \n)", - 'format storage' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) NOT NULL COLUMN_FORMAT FIXED STORAGE DISK \n)", - 'reverse declared' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) UNSIGNED CHARACTER SET latin1 NOT NULL COMMENT 'c' STORAGE DISK \n)", - 'unknown option' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) CHARACTER SET utf8mb4 NOT NULL \n)", - 'after only' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) NOT NULL \n)", - 'falsy skipped' => "CREATE TABLE `test` ( \n `name` VARCHAR(255) COLLATE utf8mb4_bin NOT NULL \n)", - ]); + $driver = new Driver( + $this->createStub(AbstractPdoConnection::class), + $this->createStub(Statement::class), + $this->createStub(Result::class), + ); + $this->platform = new AdapterPlatform($driver); + } + + private function buildSql(CreateTable $table): string + { + $decorator = new CreateTableDecorator(); + $decorator->setSubject($table); + + return $decorator->getSqlString($this->platform); } } From 3ede3bbb19bc7c9ab865484ac64ac44520dfaf46 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 21:37:20 +1000 Subject: [PATCH 04/17] Remove tests duplicated by the 0.5.x merge The merge combined this branch's test-prefixed additions with the PHPUnit 12 attribute-style renames of the same base scenarios from 0.5.x, leaving 11 duplicate tests. Keeps the unique additions - the SQL-injection rejection tests, the DDL option matrix tests, and the column_format alias test - renamed to the attribute convention. Signed-off-by: Simon Mundy --- test/unit/Sql/Ddl/AlterTableDecoratorTest.php | 214 ++++-------------- .../unit/Sql/Ddl/CreateTableDecoratorTest.php | 51 ++--- 2 files changed, 63 insertions(+), 202 deletions(-) diff --git a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php index 240964e..5cb8834 100644 --- a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php @@ -181,6 +181,24 @@ public function addColumnFormat(): void static::assertStringContainsString('COLUMN_FORMAT FIXED', $this->buildSql($alter)); } + #[Test] + #[DataProvider('unsafeColumnOptionProvider')] + public function addColumnRejectsOptionValueThatWouldInjectSql( + string $option, + string $value, + string $reportedOption, + ): void { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, $value); + $alter->addColumn($col); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $reportedOption)); + + $this->buildSql($alter); + } + #[Test] public function addColumnStorage(): void { @@ -307,119 +325,8 @@ public function changeColumnIdentity(): void } #[Test] - public function changeColumnStorage(): void - { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('storage', 'disk'); - $alter->changeColumn('name', $col); - - static::assertStringContainsString('STORAGE DISK', $this->buildSql($alter)); - } - - #[Test] - public function changeColumnUnsigned(): void - { - $alter = new AlterTable('test'); - $col = new Column\Integer('id'); - $col->setOption('unsigned', true); - $alter->changeColumn('id', $col); - - static::assertStringContainsString('UNSIGNED', $this->buildSql($alter)); - } - - #[Test] - public function changeColumnZerofill(): void - { - $alter = new AlterTable('test'); - $col = new Column\Integer('id'); - $col->setOption('zerofill', true); - $alter->changeColumn('id', $col); - - static::assertStringContainsString('ZEROFILL', $this->buildSql($alter)); - } - - public function testAddColumnAfter(): void - { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('after', 'id'); - $alter->addColumn($col); - - $sql = $this->buildSql($alter); - - self::assertStringContainsString('AFTER `id`', $sql); - } - - public function testAddColumnCharset(): void - { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('charset', 'utf8mb3'); - $alter->addColumn($col); - - $sql = $this->buildSql($alter); - - self::assertStringContainsString('CHARACTER SET utf8mb3', $sql); - } - - public function testAddColumnCharsetAndCollate(): void - { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('charset', 'utf8mb3'); - $col->setOption('collate', 'utf8mb3_unicode_ci'); - $alter->addColumn($col); - - $sql = $this->buildSql($alter); - - self::assertStringContainsString('CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci', $sql); - } - - public function testAddColumnCharsetBeforeNotNull(): void - { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setNullable(false); - $col->setOption('charset', 'utf8mb3'); - $col->setOption('collate', 'utf8mb3_unicode_ci'); - $alter->addColumn($col); - - $sql = $this->buildSql($alter); - - self::assertMatchesRegularExpression( - '/CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL/', - $sql, - ); - } - - public function testAddColumnCollate(): void - { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('collate', 'utf8mb3_unicode_ci'); - $alter->addColumn($col); - - $sql = $this->buildSql($alter); - - self::assertStringContainsString('COLLATE utf8mb3_unicode_ci', $sql); - } - - public function testAddColumnFormatAndStorage(): void - { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('column_format', 'fixed'); - $col->setOption('storage', 'memory'); - $alter->addColumn($col); - - $sql = $this->buildSql($alter); - - self::assertStringContainsString('COLUMN_FORMAT FIXED STORAGE MEMORY', $sql); - } - #[DataProvider('unsafeColumnOptionProvider')] - public function testAddColumnRejectsOptionValueThatWouldInjectSql( + public function changeColumnRejectsOptionValueThatWouldInjectSql( string $option, string $value, string $reportedOption, @@ -427,7 +334,7 @@ public function testAddColumnRejectsOptionValueThatWouldInjectSql( $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); $col->setOption($option, $value); - $alter->addColumn($col); + $alter->changeColumn('name', $col); $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $reportedOption)); @@ -435,76 +342,37 @@ public function testAddColumnRejectsOptionValueThatWouldInjectSql( $this->buildSql($alter); } - public function testAddColumnUnsigned(): void - { - $alter = new AlterTable('test'); - $col = new Column\Integer('id'); - $col->setOption('unsigned', true); - $col->setOption('auto_increment', true); - $alter->addColumn($col); - - $sql = $this->buildSql($alter); - - self::assertStringContainsString('UNSIGNED', $sql); - self::assertStringContainsString('AUTO_INCREMENT', $sql); - } - - public function testChangeColumnCharset(): void + #[Test] + public function changeColumnStorage(): void { $alter = new AlterTable('test'); $col = new Column\Varchar('name', 255); - $col->setOption('charset', 'utf8mb3'); + $col->setOption('storage', 'disk'); $alter->changeColumn('name', $col); - $sql = $this->buildSql($alter); - - self::assertStringContainsString('CHARACTER SET utf8mb3', $sql); + static::assertStringContainsString('STORAGE DISK', $this->buildSql($alter)); } - public function testChangeColumnCharsetAndCollate(): void + #[Test] + public function changeColumnUnsigned(): void { $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setNullable(false); - $col->setOption('charset', 'utf8mb3'); - $col->setOption('collate', 'utf8mb3_unicode_ci'); - $alter->changeColumn('name', $col); - - $sql = $this->buildSql($alter); + $col = new Column\Integer('id'); + $col->setOption('unsigned', true); + $alter->changeColumn('id', $col); - self::assertMatchesRegularExpression( - '/CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL/', - $sql, - ); + static::assertStringContainsString('UNSIGNED', $this->buildSql($alter)); } - public function testChangeColumnCollate(): void + #[Test] + public function changeColumnZerofill(): void { $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('collate', 'utf8mb3_unicode_ci'); - $alter->changeColumn('name', $col); - - $sql = $this->buildSql($alter); - - self::assertStringContainsString('COLLATE utf8mb3_unicode_ci', $sql); - } - - #[DataProvider('unsafeColumnOptionProvider')] - public function testChangeColumnRejectsOptionValueThatWouldInjectSql( - string $option, - string $value, - string $reportedOption, - ): void { - $alter = new AlterTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption($option, $value); - $alter->changeColumn('name', $col); - - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $reportedOption)); + $col = new Column\Integer('id'); + $col->setOption('zerofill', true); + $alter->changeColumn('id', $col); - $this->buildSql($alter); + static::assertStringContainsString('ZEROFILL', $this->buildSql($alter)); } /** @@ -512,13 +380,14 @@ public function testChangeColumnRejectsOptionValueThatWouldInjectSql( * * @param array $options */ + #[Test] #[DataProvider('addColumnMatrixProvider')] - public function testGeneratesExpectedSqlForAddedColumnOptions(array $options, string $expected): void + public function generatesExpectedSqlForAddedColumnOptions(array $options, string $expected): void { $alter = new AlterTable('test'); $alter->addColumn($this->makeColumn($options)); - self::assertSame($expected, $this->buildSql($alter)); + static::assertSame($expected, $this->buildSql($alter)); } /** @@ -526,13 +395,14 @@ public function testGeneratesExpectedSqlForAddedColumnOptions(array $options, st * * @param array $options */ + #[Test] #[DataProvider('changeColumnMatrixProvider')] - public function testGeneratesExpectedSqlForChangedColumnOptions(array $options, string $expected): void + public function generatesExpectedSqlForChangedColumnOptions(array $options, string $expected): void { $alter = new AlterTable('test'); $alter->changeColumn('name', $this->makeColumn($options)); - self::assertSame($expected, $this->buildSql($alter)); + static::assertSame($expected, $this->buildSql($alter)); } protected function setUp(): void diff --git a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php index 75f35a0..6c92b52 100644 --- a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php @@ -148,6 +148,19 @@ public function columnFormatOption(): void static::assertStringContainsString('COLUMN_FORMAT FIXED', $this->buildSql($table)); } + #[Test] + public function columnFormatOptionWithUnderscoreAlias(): void + { + $table = new CreateTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption('column_format', 'dynamic'); + $table->addColumn($col); + + $sql = $this->buildSql($table); + + static::assertStringContainsString('COLUMN_FORMAT DYNAMIC', $sql); + } + #[Test] public function commentOption(): void { @@ -197,36 +210,14 @@ public function fullColumnDefinition(): void static::assertStringContainsString('CHARACTER SET utf8mb3 COLLATE utf8mb3_unicode_ci NOT NULL', $sql); } - #[Test] - public function storageOption(): void - { - $table = new CreateTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('storage', 'disk'); - $table->addColumn($col); - - static::assertStringContainsString('STORAGE DISK', $this->buildSql($table)); - } - - public function testColumnFormatOption(): void - { - $table = new CreateTable('test'); - $col = new Column\Varchar('name', 255); - $col->setOption('column_format', 'dynamic'); - $table->addColumn($col); - - $sql = $this->buildSql($table); - - self::assertStringContainsString('COLUMN_FORMAT DYNAMIC', $sql); - } - /** * Pins the exact DDL produced for a matrix of column options. * * @param array $options */ + #[Test] #[DataProvider('columnOptionMatrixProvider')] - public function testGeneratesExpectedSqlForColumnOptions(array $options, string $expected): void + public function generatesExpectedSqlForColumnOptions(array $options, string $expected): void { $table = new CreateTable('test'); $col = new Column\Varchar('name', 255); @@ -238,11 +229,12 @@ public function testGeneratesExpectedSqlForColumnOptions(array $options, string $table->addColumn($col); - self::assertSame($expected, $this->buildSql($table)); + static::assertSame($expected, $this->buildSql($table)); } + #[Test] #[DataProvider('unsafeColumnOptionProvider')] - public function testRejectsColumnOptionValueThatWouldInjectSql( + public function rejectsColumnOptionValueThatWouldInjectSql( string $option, string $value, string $reportedOption, @@ -258,16 +250,15 @@ public function testRejectsColumnOptionValueThatWouldInjectSql( $this->buildSql($table); } - public function testStorageOption(): void + #[Test] + public function storageOption(): void { $table = new CreateTable('test'); $col = new Column\Varchar('name', 255); $col->setOption('storage', 'disk'); $table->addColumn($col); - $sql = $this->buildSql($table); - - self::assertStringContainsString('STORAGE DISK', $sql); + static::assertStringContainsString('STORAGE DISK', $this->buildSql($table)); } #[Test] From b90afb7bb023dd7aeeeaa5f0191099947ac6561e Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 21:37:48 +1000 Subject: [PATCH 05/17] Prefer guard clauses over parenthesised multi-line returns Replaces the formatter's paren-wrapped multi-line return expressions with separate statements: the enums resolve tryFrom() into a variable and throw on null, and resolveAfterOption() returns early for other options. Signed-off-by: Simon Mundy --- src/Sql/ColumnFormatEnum.php | 11 +++++++---- src/Sql/Ddl/AlterTableDecorator.php | 10 +++++----- src/Sql/StorageEnum.php | 11 +++++++---- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Sql/ColumnFormatEnum.php b/src/Sql/ColumnFormatEnum.php index fc2c46f..1eea274 100644 --- a/src/Sql/ColumnFormatEnum.php +++ b/src/Sql/ColumnFormatEnum.php @@ -32,13 +32,16 @@ enum ColumnFormatEnum: string public static function getOptionValue(mixed $value): self { $keyword = is_string($value) ? strtoupper(trim($value)) : ''; + $format = self::tryFrom($keyword); - return ( - self::tryFrom($keyword) ?? throw new InvalidArgumentException(sprintf( + if (null === $format) { + throw new InvalidArgumentException(sprintf( 'Invalid value for the "columnformat" column option; expected one of %s, received "%s"', implode(', ', array_map(static fn(self $case): string => $case->value, self::cases())), is_string($value) ? $value : get_debug_type($value), - )) - ); + )); + } + + return $format; } } diff --git a/src/Sql/Ddl/AlterTableDecorator.php b/src/Sql/Ddl/AlterTableDecorator.php index 5e3e19a..2a1f6df 100644 --- a/src/Sql/Ddl/AlterTableDecorator.php +++ b/src/Sql/Ddl/AlterTableDecorator.php @@ -88,10 +88,10 @@ protected function processChangeColumns(?PlatformInterface $adapterPlatform = nu */ private function resolveAfterOption(string $option, mixed $value, ?PlatformInterface $platform): ?array { - return ( - $option === 'after' - ? [' AFTER ' . $platform->quoteIdentifier($value), 2] - : null - ); + if ('after' !== $option) { + return null; + } + + return [" AFTER {$platform->quoteIdentifier($value)}", 2]; } } diff --git a/src/Sql/StorageEnum.php b/src/Sql/StorageEnum.php index 4911415..5af1342 100644 --- a/src/Sql/StorageEnum.php +++ b/src/Sql/StorageEnum.php @@ -33,13 +33,16 @@ enum StorageEnum: string public static function getOptionValue(mixed $value): self { $keyword = is_string($value) ? strtoupper(trim($value)) : ''; + $storage = self::tryFrom($keyword); - return ( - self::tryFrom($keyword) ?? throw new InvalidArgumentException(sprintf( + if (null === $storage) { + throw new InvalidArgumentException(sprintf( 'Invalid value for the "storage" column option; expected one of %s, received "%s"', implode(', ', array_map(static fn(self $case): string => $case->value, self::cases())), is_string($value) ? $value : get_debug_type($value), - )) - ); + )); + } + + return $storage; } } From 256f146738ee326e3f3b475d60ab08ad55aaaaa1 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 21:38:07 +1000 Subject: [PATCH 06/17] mago analyze: fix findings in DDL option handling, regenerate baseline Asserts ColumnInterface and option-array types locally where the upstream properties are untyped arrays, makes the trait's platform parameter non-nullable (callers already guard), casts option values fed to quoteValue()/quoteIdentifier(), and rewrites the offset-bump loop to iterate the offsets instead of variable-indexing into the array shape, which mago cannot prove defined. Corrects the inherited processChangeColumns() return docblock to the actual pair-list shape. Carries the decorators' complexity suppressions onto the trait the consolidation moved that complexity into, and regenerates the analyze baseline with mago 1.47, whose changed type phrasing had orphaned the old entries (the SelectDecorator false positive and 39 stale entries). The baseline contains no entries for the code this branch adds. Signed-off-by: Simon Mundy --- analysis-baseline.toml | 166 +-------------------------- src/Sql/Ddl/AlterTableDecorator.php | 29 +++-- src/Sql/Ddl/ColumnOptionTrait.php | 42 ++++--- src/Sql/Ddl/CreateTableDecorator.php | 11 +- 4 files changed, 59 insertions(+), 189 deletions(-) diff --git a/analysis-baseline.toml b/analysis-baseline.toml index 384d919..bcc7fb4 100644 --- a/analysis-baseline.toml +++ b/analysis-baseline.toml @@ -369,7 +369,7 @@ count = 6 [[issues]] file = "src/Metadata/Source.php" code = "unused-method" -message = "Method `loadconstraintdatanames()` is never used." +message = "Method `loadConstraintDataNames()` is never used." count = 1 [[issues]] @@ -456,172 +456,10 @@ code = "unused-property" message = "Property `$numberOfRows` is never used." count = 1 -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\Mysql\Sql\Ddl\AlterTableDecorator::processChangeColumns`: expected `array>`, but found `list{array{}|non-empty-list}`.' -count = 1 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "less-specific-argument" -message = 'Argument type mismatch for argument #1 of `PhpDb\Adapter\Platform\PlatformInterface::quoteIdentifier`: expected `string`, but provided type `array-key` is less specific.' -count = 1 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Adapter\Platform\PlatformInterface::quoteIdentifier`: expected `string`, but found `truthy-mixed`.' -count = 1 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Adapter\Platform\PlatformInterface::quoteValue`: expected `string`, but found `truthy-mixed`.' -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Sql\AbstractSql::processExpression`: expected `PhpDb\Sql\ExpressionInterface`, but found `mixed`.' -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "mixed-argument" -message = "Invalid argument type for argument #1 of `strtoupper`: expected `string`, but found `truthy-mixed`." -count = 4 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "mixed-argument" -message = "Invalid argument type for argument #1 of `uksort`: expected `array`, but found `mixed`." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 6 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "mixed-method-access" -message = "Attempting to access a method on a non-object type (`mixed`)." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "possibly-null-argument" -message = "Argument #3 of function `substr_replace` is possibly `null`, but parameter type `array|int` does not accept it." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "possibly-null-operand" -message = "Left operand in arithmetic operation might be `null` (type `int|null`)." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `int(0)|int(1)|int(2)` accessed on `array{0: int, 1: int, 2: int, 3: int, ...}`." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `int(0)|int(1)|int(2)` accessed on `array{0: int, 1: int, 2: int, 3: int}`." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `non-negative-int` accessed on `array{0: int, 1: int, 2: int, 3: int, ...}`." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/AlterTableDecorator.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `non-negative-int` accessed on `array{0: int, 1: int, 2: int, 3: int}`." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Adapter\Platform\PlatformInterface::quoteValue`: expected `string`, but found `truthy-mixed`.' -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Sql\AbstractSql::processExpression`: expected `PhpDb\Sql\ExpressionInterface`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "mixed-argument" -message = "Invalid argument type for argument #1 of `strtoupper`: expected `string`, but found `truthy-mixed`." -count = 2 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "mixed-argument" -message = "Invalid argument type for argument #1 of `uksort`: expected `array`, but found `mixed`." -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 3 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "mixed-method-access" -message = "Attempting to access a method on a non-object type (`mixed`)." -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "possibly-null-argument" -message = "Argument #3 of function `substr_replace` is possibly `null`, but parameter type `array|int` does not accept it." -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "possibly-null-operand" -message = "Left operand in arithmetic operation might be `null` (type `int|null`)." -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `int(0)|int(1)|int(2)` accessed on `array{0: int, 1: int, 2: int, 3: int, ...}`." -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `int(0)|int(1)|int(2)` accessed on `array{0: int, 1: int, 2: int, 3: int}`." -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `non-negative-int` accessed on `array{0: int, 1: int, 2: int, 3: int, ...}`." -count = 1 - -[[issues]] -file = "src/Sql/Ddl/CreateTableDecorator.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `non-negative-int` accessed on `array{0: int, 1: int, 2: int, 3: int}`." -count = 1 - [[issues]] file = "src/Sql/SelectDecorator.php" code = "invalid-property-assignment-value" -message = "Invalid type for property `$specifications`: expected `array>|array`, but got `array{'limit': string('LIMIT 18446744073709551615'), ...|string>}`." +message = "Invalid type for property `$specifications`: expected `array>|array`, but got `array{'limit': string('LIMIT 18446744073709551615'), ...>}|array{'limit': string('LIMIT 18446744073709551615'), ...}`." count = 1 [[issues]] diff --git a/src/Sql/Ddl/AlterTableDecorator.php b/src/Sql/Ddl/AlterTableDecorator.php index 2a1f6df..5531903 100644 --- a/src/Sql/Ddl/AlterTableDecorator.php +++ b/src/Sql/Ddl/AlterTableDecorator.php @@ -7,6 +7,7 @@ use Override; use PhpDb\Adapter\Platform\PlatformInterface; use PhpDb\Sql\Ddl\AlterTable; +use PhpDb\Sql\Ddl\Column\ColumnInterface; use PhpDb\Sql\Exception; use PhpDb\Sql\Platform\PlatformDecoratorInterface; use PhpDb\Sql\PreparableSqlInterface; @@ -43,10 +44,16 @@ protected function processAddColumns(?PlatformInterface $adapterPlatform = null) $sqls = []; - foreach ($this->addColumns as $i => $column) { + /** @var array $addColumns */ + $addColumns = $this->addColumns; + + foreach ($addColumns as $i => $column) { + /** @var array $options */ + $options = $column->getOptions(); + $sqls[$i] = $this->processColumnOptions( $this->processExpression($column, $adapterPlatform), - $column->getOptions(), + $options, $adapterPlatform, $this->resolveAfterOption(...), ); @@ -56,7 +63,7 @@ protected function processAddColumns(?PlatformInterface $adapterPlatform = null) } /** - * @return array> + * @return array{0: list} * * @throws Exception\RuntimeException */ @@ -69,12 +76,18 @@ protected function processChangeColumns(?PlatformInterface $adapterPlatform = nu $sqls = []; - foreach ($this->changeColumns as $name => $column) { + /** @var array $changeColumns */ + $changeColumns = $this->changeColumns; + + foreach ($changeColumns as $name => $column) { + /** @var array $options */ + $options = $column->getOptions(); + $sqls[] = [ $adapterPlatform->quoteIdentifier($name), $this->processColumnOptions( $this->processExpression($column, $adapterPlatform), - $column->getOptions(), + $options, $adapterPlatform, ), ]; @@ -84,14 +97,14 @@ protected function processChangeColumns(?PlatformInterface $adapterPlatform = nu } /** - * @return array{string, int}|null + * @return array{string, int<0, 3>}|null */ - private function resolveAfterOption(string $option, mixed $value, ?PlatformInterface $platform): ?array + private function resolveAfterOption(string $option, mixed $value, PlatformInterface $platform): ?array { if ('after' !== $option) { return null; } - return [" AFTER {$platform->quoteIdentifier($value)}", 2]; + return [" AFTER {$platform->quoteIdentifier((string) $value)}", 2]; } } diff --git a/src/Sql/Ddl/ColumnOptionTrait.php b/src/Sql/Ddl/ColumnOptionTrait.php index 38fc9de..5048910 100644 --- a/src/Sql/Ddl/ColumnOptionTrait.php +++ b/src/Sql/Ddl/ColumnOptionTrait.php @@ -35,6 +35,8 @@ * * @internal */ +// @mago-expect lint:cyclomatic-complexity +// @mago-expect lint:kan-defect trait ColumnOptionTrait { private const string NAME_PATTERN = '/^[A-Za-z0-9_]+$/'; @@ -83,7 +85,10 @@ protected function getSqlInsertOffsets(string $sql): array } } - foreach (range(0, 3) as $i) { + foreach (range( + start: 0, + end: 3, + ) as $i) { $insertStart[$i] ??= $sqlLength; } @@ -95,39 +100,46 @@ protected function getSqlInsertOffsets(string $sql): array * Appends each option to $sql at the offset its keyword belongs to. * * @param array $options - * @param (callable(string, mixed, ?PlatformInterface): ?array{string, int})|null $resolveExtra + * @param (callable(string, mixed, PlatformInterface): ?array{string, int<0, 3>})|null $resolveExtra * Resolver for options only valid in the calling statement, tried before the common ones. */ protected function processColumnOptions( string $sql, array $options, - ?PlatformInterface $platform = null, + PlatformInterface $platform, ?callable $resolveExtra = null, ): string { $insertStart = $this->getSqlInsertOffsets($sql); uksort($options, $this->compareColumnOptions(...)); + // @mago-expect analysis:mixed-assignment - option values are heterogeneous by design foreach ($options as $name => $value) { if (! $value) { continue; } $option = $this->normalizeColumnOption($name); - $resolved = $resolveExtra !== null ? $resolveExtra($option, $value, $platform) : null; + $resolved = null === $resolveExtra ? null : $resolveExtra($option, $value, $platform); $resolved ??= $this->resolveColumnOption($option, $value, $platform); - if ($resolved === null) { + if (null === $resolved) { continue; } [$insert, $j] = $resolved; + $length = strlen($insert); - $sql = substr_replace($sql, $insert, $insertStart[$j], length: 0); - $insertStartCount = count($insertStart); + foreach ($insertStart as $slot => $offset) { + if ($slot < $j) { + continue; + } + + if ($slot === $j) { + $sql = substr_replace($sql, $insert, $offset, length: 0); + } - for (; $j < $insertStartCount; ++$j) { - $insertStart[$j] += strlen($insert); + $insertStart[$slot] = $offset + $length; } } @@ -165,22 +177,22 @@ private function getColumnOptionName(string $option, mixed $value): string private function normalizeColumnOption(string $name): string { - return strtolower(str_replace(['-', '_', ' '], '', $name)); + return strtolower(str_replace(['-', '_', ' '], replace: '', subject: $name)); } /** - * @return array{string, int}|null The SQL to insert and the offset index it belongs at. + * @return array{string, int<0, 3>}|null The SQL to insert and the offset index it belongs at. * @throws InvalidArgumentException If the option value would not be safe to emit unquoted. */ - private function resolveColumnOption(string $option, mixed $value, ?PlatformInterface $platform): ?array + private function resolveColumnOption(string $option, mixed $value, PlatformInterface $platform): ?array { return match ($option) { 'unsigned' => [' UNSIGNED', 0], 'zerofill' => [' ZEROFILL', 0], - 'charset' => [' CHARACTER SET ' . $this->getColumnOptionName('charset', $value), 0], - 'collate' => [' COLLATE ' . $this->getColumnOptionName('collate', $value), 0], + 'charset' => [" CHARACTER SET {$this->getColumnOptionName('charset', $value)}", 0], + 'collate' => [" COLLATE {$this->getColumnOptionName('collate', $value)}", 0], 'identity', 'serial', 'autoincrement' => [' AUTO_INCREMENT', 1], - 'comment' => [' COMMENT ' . $platform->quoteValue($value), 2], + 'comment' => [" COMMENT {$platform->quoteValue((string) $value)}", 2], 'columnformat', 'format' => [' COLUMN_FORMAT ' . ColumnFormatEnum::getOptionValue($value)->value, 2], 'storage' => [' STORAGE ' . StorageEnum::getOptionValue($value)->value, 2], default => null, diff --git a/src/Sql/Ddl/CreateTableDecorator.php b/src/Sql/Ddl/CreateTableDecorator.php index e8d9b7b..fd575b0 100644 --- a/src/Sql/Ddl/CreateTableDecorator.php +++ b/src/Sql/Ddl/CreateTableDecorator.php @@ -6,6 +6,7 @@ use Override; use PhpDb\Adapter\Platform\PlatformInterface; +use PhpDb\Sql\Ddl\Column\ColumnInterface; use PhpDb\Sql\Ddl\CreateTable; use PhpDb\Sql\Exception; use PhpDb\Sql\Platform\PlatformDecoratorInterface; @@ -47,10 +48,16 @@ protected function processColumns(?PlatformInterface $adapterPlatform = null): ? $sqls = []; - foreach ($this->columns as $i => $column) { + /** @var array $columns */ + $columns = $this->columns; + + foreach ($columns as $i => $column) { + /** @var array $options */ + $options = $column->getOptions(); + $sqls[$i] = $this->processColumnOptions( $this->processExpression($column, $adapterPlatform), - $column->getOptions(), + $options, $adapterPlatform, ); } From 90a858f2ddfcdb49bd9a7886ed7798d4c593efac Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 21:46:05 +1000 Subject: [PATCH 07/17] Implement ResultInterface::getQueryResult() for the mysqli Result Upstream phpdb 0.6.x-dev added getQueryResult() to ResultInterface after this repo's lock (e037464 -> b57f549), which fataled the CI latest/lowest dependency legs - both resolve the dev branch to its tip. Mirrors the upstream Pdo\Result implementation, bumps the lock to the same tip so all three legs test one phpdb revision, and regenerates the analyze baseline for the vendor's shifted types. Signed-off-by: Simon Mundy --- analysis-baseline.toml | 60 +++++++++++++++++++++++++++++++++++------- composer.lock | 17 ++++++------ src/Result.php | 25 ++++++++++++++++++ 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/analysis-baseline.toml b/analysis-baseline.toml index bcc7fb4..a22bef2 100644 --- a/analysis-baseline.toml +++ b/analysis-baseline.toml @@ -296,9 +296,39 @@ count = 1 [[issues]] file = "src/Metadata/Source.php" -code = "mixed-array-assignment" -message = "Unsafe array assignment on type `mixed`." -count = 9 +code = "impossible-condition" +message = "This condition (type `false`) will always evaluate to false." +count = 1 + +[[issues]] +file = "src/Metadata/Source.php" +code = "impossible-nonnull-entry-check" +message = "Impossible `isset` check on key `'constraint_names'` accessed on `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`." +count = 1 + +[[issues]] +file = "src/Metadata/Source.php" +code = "invalid-property-assignment-value" +message = "Invalid type for property `$data`: expected `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`, but got `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_names': non-empty-list>>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`." +count = 1 + +[[issues]] +file = "src/Metadata/Source.php" +code = "invalid-property-assignment-value" +message = "Invalid type for property `$data`: expected `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`, but got `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers': non-empty-array>}`." +count = 1 + +[[issues]] +file = "src/Metadata/Source.php" +code = "mixed-property-type-coercion" +message = "A value with a less specific type `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys': non-empty-array|array{'column_name': mixed, 'constraint_name': mixed, 'ordinal_position': mixed, 'table_name': mixed}>>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}` is being assigned to property `$data` (array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>})." +count = 1 + +[[issues]] +file = "src/Metadata/Source.php" +code = "mixed-property-type-coercion" +message = "A value with a less specific type `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references': non-empty-array|array{'constraint_name': mixed, 'delete_rule': mixed, 'referenced_column_name': mixed, 'referenced_table_name': mixed, 'update_rule': mixed}>>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}` is being assigned to property `$data` (array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>})." +count = 1 [[issues]] file = "src/Metadata/Source.php" @@ -356,15 +386,27 @@ count = 2 [[issues]] file = "src/Metadata/Source.php" -code = "reference-constraint-violation" -message = "Invalid assignment to by-reference parameter `$c`." -count = 7 +code = "property-type-coercion" +message = "A value of a less specific type `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints': non-empty-array, 'constraint_name'?: string, 'constraint_type'?: string, 'delete_rule'?: null|string, 'match_option'?: null|string, 'referenced_columns'?: list, 'referenced_table_name'?: null|string, 'referenced_table_schema'?: null|string, 'table_name'?: string, 'update_rule'?: null|string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}` is being assigned to property `$data` (array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>})." +count = 1 [[issues]] file = "src/Metadata/Source.php" -code = "too-many-arguments" -message = 'Too many arguments provided for method `PhpDb\Metadata\Source\AbstractSource::prepareDataHierarchy`.' -count = 6 +code = "redundant-comparison" +message = "Redundant `!==` comparison: left-hand side is never identical to (always false for !==) right-hand side." +count = 1 + +[[issues]] +file = "src/Metadata/Source.php" +code = "redundant-null-coalesce" +message = "Redundant null coalesce: left-hand side is always `null`." +count = 1 + +[[issues]] +file = "src/Metadata/Source.php" +code = "reference-constraint-violation" +message = "Invalid assignment to by-reference parameter `$c`." +count = 7 [[issues]] file = "src/Metadata/Source.php" diff --git a/composer.lock b/composer.lock index e95afe9..0453b9d 100644 --- a/composer.lock +++ b/composer.lock @@ -262,30 +262,29 @@ "source": { "type": "git", "url": "https://github.com/php-db/phpdb.git", - "reference": "e037464a435005e04a3fde3c113324c26406ce7e" + "reference": "b57f549b411d0b4fe46de8a469983816395be850" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/php-db/phpdb/zipball/e037464a435005e04a3fde3c113324c26406ce7e", - "reference": "e037464a435005e04a3fde3c113324c26406ce7e", + "url": "https://api.github.com/repos/php-db/phpdb/zipball/b57f549b411d0b4fe46de8a469983816395be850", + "reference": "b57f549b411d0b4fe46de8a469983816395be850", "shasum": "" }, "require": { "laminas/laminas-servicemanager": "^3.0.0 || ^4.0.0", "laminas/laminas-stdlib": "^3.20.0", - "php": "~8.2.0 || ~8.3.0 || ~8.4.0 || ~8.5.0" + "php": "~8.3.0 || ~8.4.0 || ~8.5.0" }, "conflict": { "laminas/laminas-db": "*", "zendframework/zend-db": "*" }, "require-dev": { - "laminas/laminas-coding-standard": "^3.0.1", + "infection/infection": "^0.34.1", "laminas/laminas-eventmanager": "^3.14.0", "laminas/laminas-hydrator": "^4.6.0", + "php-db/phpdb-qa-tools": "0.1.x-dev", "phpbench/phpbench": "^1.4", - "phpstan/phpstan": "^2.1", - "phpstan/phpstan-phpunit": "^2.0", "phpunit/phpunit": "^11.5.42", "rector/rector": "^2.0" }, @@ -323,7 +322,7 @@ "issues": "https://github.com/php-db/phpdb/issues", "source": "https://github.com/php-db/phpdb" }, - "time": "2026-07-08T05:31:35+00:00" + "time": "2026-08-24T01:49:39+00:00" }, { "name": "psr/container", @@ -4500,5 +4499,5 @@ "platform-overrides": { "php": "8.3.99" }, - "plugin-api-version": "2.9.0" + "plugin-api-version": "2.6.0" } diff --git a/src/Result.php b/src/Result.php index 6c8d4bf..d5bb03d 100644 --- a/src/Result.php +++ b/src/Result.php @@ -11,6 +11,8 @@ use Override; use PhpDb\Adapter\Driver\ResultInterface; use PhpDb\Adapter\Exception; +use PhpDb\ResultSet\ResultSet; +use PhpDb\ResultSet\ResultSetInterface; // phpcs:ignore SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse use ReturnTypeWillChange; @@ -140,6 +142,29 @@ public function getGeneratedValue(): string|int|false|null return $this->generatedValue; } + /** + * {@inheritDoc} + * + * @throws Exception\RuntimeException When isQueryResult() is false. + * @throws \Exception If the seeded result set rejects this result as its data source. + */ + #[Override] + public function getQueryResult(?ResultSetInterface $resultPrototype = null): ResultSetInterface + { + if (! $this->isQueryResult()) { + throw new Exception\RuntimeException( + 'Cannot produce a query result set from a result that is not a query result;' + . ' check isQueryResult() first', + ); + } + + $resultPrototype ??= new ResultSet(); + $resultSet = clone $resultPrototype; + $resultSet->initialize($this); + + return $resultSet; + } + /** * {@inheritDoc} */ From 1e54de93fab7e66b4cfcb9a210c7c880862aa49d Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 21:55:46 +1000 Subject: [PATCH 08/17] Point coverage metadata at ColumnOptionTrait The CoversMethod attributes for getSqlInsertOffsets(), compareColumnOptions(), and normalizeColumnOption() still named the decorator classes, but the ColumnOptionTrait consolidation moved those methods into the trait, so php-code-coverage rejects them as targets - 78 PHPUnit warnings that fail the coverage CI leg under failOnWarning. Replaces them with CoversTrait(ColumnOptionTrait), which also credits the trait's other methods these tests exercise. Signed-off-by: Simon Mundy --- test/unit/Sql/Ddl/AlterTableDecoratorTest.php | 6 +++--- test/unit/Sql/Ddl/CreateTableDecoratorTest.php | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php index 5cb8834..62e4f21 100644 --- a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php @@ -10,11 +10,13 @@ use PhpDb\Mysql\AdapterPlatform; use PhpDb\Mysql\Pdo\Driver; use PhpDb\Mysql\Sql\Ddl\AlterTableDecorator; +use PhpDb\Mysql\Sql\Ddl\ColumnOptionTrait; use PhpDb\Sql\Ddl\AlterTable; use PhpDb\Sql\Ddl\Column; use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDbTest\Mysql\Sql\Ddl\TestAsset\ColumnOptionMatrix; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\CoversTrait; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; @@ -24,9 +26,7 @@ #[CoversMethod(AlterTableDecorator::class, 'setSubject')] #[CoversMethod(AlterTableDecorator::class, 'processAddColumns')] #[CoversMethod(AlterTableDecorator::class, 'processChangeColumns')] -#[CoversMethod(AlterTableDecorator::class, 'getSqlInsertOffsets')] -#[CoversMethod(AlterTableDecorator::class, 'compareColumnOptions')] -#[CoversMethod(AlterTableDecorator::class, 'normalizeColumnOption')] +#[CoversTrait(ColumnOptionTrait::class)] final class AlterTableDecoratorTest extends TestCase { protected AdapterPlatform $platform; diff --git a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php index 6c92b52..a772e27 100644 --- a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php @@ -9,6 +9,7 @@ use PhpDb\Adapter\Driver\Pdo\Statement; use PhpDb\Mysql\AdapterPlatform; use PhpDb\Mysql\Pdo\Driver; +use PhpDb\Mysql\Sql\Ddl\ColumnOptionTrait; use PhpDb\Mysql\Sql\Ddl\CreateTableDecorator; use PhpDb\Sql\Ddl\Column; use PhpDb\Sql\Ddl\Constraint; @@ -16,6 +17,7 @@ use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDbTest\Mysql\Sql\Ddl\TestAsset\ColumnOptionMatrix; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\CoversTrait; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; @@ -24,9 +26,7 @@ #[CoversMethod(CreateTableDecorator::class, 'setSubject')] #[CoversMethod(CreateTableDecorator::class, 'processColumns')] -#[CoversMethod(CreateTableDecorator::class, 'getSqlInsertOffsets')] -#[CoversMethod(CreateTableDecorator::class, 'compareColumnOptions')] -#[CoversMethod(CreateTableDecorator::class, 'normalizeColumnOption')] +#[CoversTrait(ColumnOptionTrait::class)] final class CreateTableDecoratorTest extends TestCase { protected AdapterPlatform $platform; From 1fc4d088cbd5e2bacbc2599d397e51b43ff24e6a Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 22:01:38 +1000 Subject: [PATCH 09/17] Credit enum coverage exercised by the DDL option tests The injection-rejection and option matrix tests drive both getOptionValue() paths of ColumnFormatEnum and StorageEnum, but with requireCoverageMetadata that execution earns no credit until a test class claims the enums - they reported 0% despite full exercise. Declares CoversClass for both on the two decorator test classes, taking each enum from 0/9 to 9/9 covered statements. Signed-off-by: Simon Mundy --- test/unit/Sql/Ddl/AlterTableDecoratorTest.php | 5 +++++ test/unit/Sql/Ddl/CreateTableDecoratorTest.php | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php index 62e4f21..7c040e2 100644 --- a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php @@ -9,12 +9,15 @@ use PhpDb\Adapter\Driver\Pdo\Statement; use PhpDb\Mysql\AdapterPlatform; use PhpDb\Mysql\Pdo\Driver; +use PhpDb\Mysql\Sql\ColumnFormatEnum; use PhpDb\Mysql\Sql\Ddl\AlterTableDecorator; use PhpDb\Mysql\Sql\Ddl\ColumnOptionTrait; +use PhpDb\Mysql\Sql\StorageEnum; use PhpDb\Sql\Ddl\AlterTable; use PhpDb\Sql\Ddl\Column; use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDbTest\Mysql\Sql\Ddl\TestAsset\ColumnOptionMatrix; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\CoversTrait; use PHPUnit\Framework\Attributes\DataProvider; @@ -27,6 +30,8 @@ #[CoversMethod(AlterTableDecorator::class, 'processAddColumns')] #[CoversMethod(AlterTableDecorator::class, 'processChangeColumns')] #[CoversTrait(ColumnOptionTrait::class)] +#[CoversClass(ColumnFormatEnum::class)] +#[CoversClass(StorageEnum::class)] final class AlterTableDecoratorTest extends TestCase { protected AdapterPlatform $platform; diff --git a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php index a772e27..0065343 100644 --- a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php @@ -9,13 +9,16 @@ use PhpDb\Adapter\Driver\Pdo\Statement; use PhpDb\Mysql\AdapterPlatform; use PhpDb\Mysql\Pdo\Driver; +use PhpDb\Mysql\Sql\ColumnFormatEnum; use PhpDb\Mysql\Sql\Ddl\ColumnOptionTrait; use PhpDb\Mysql\Sql\Ddl\CreateTableDecorator; +use PhpDb\Mysql\Sql\StorageEnum; use PhpDb\Sql\Ddl\Column; use PhpDb\Sql\Ddl\Constraint; use PhpDb\Sql\Ddl\CreateTable; use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDbTest\Mysql\Sql\Ddl\TestAsset\ColumnOptionMatrix; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\CoversTrait; use PHPUnit\Framework\Attributes\DataProvider; @@ -27,6 +30,8 @@ #[CoversMethod(CreateTableDecorator::class, 'setSubject')] #[CoversMethod(CreateTableDecorator::class, 'processColumns')] #[CoversTrait(ColumnOptionTrait::class)] +#[CoversClass(ColumnFormatEnum::class)] +#[CoversClass(StorageEnum::class)] final class CreateTableDecoratorTest extends TestCase { protected AdapterPlatform $platform; From 60e03817594a26e4696d374b71ffe01fc2749f3b Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 22:47:14 +1000 Subject: [PATCH 10/17] Close the coverage gaps in the driver classes Brings Connection, Driver, Result, Statement, and the Pdo counterparts to full statement coverage. Adds unit tests for the PDO-injected constructor path, failed-query and throwing-lastInsertId fallbacks, driverless execute, and unconnected rollback; integration tests for repeat connects, driver_options handling, invalid-SQL execution and preparation (with mysqli report mode restored), schema lookup without a selected database, statement re-preparation, failing prepared inserts, no-metadata statement results, unbuffered exhaustion, direct next(), undetermined buffering, and Pdo auto-connecting getCurrentSchema. Server-failure paths that first looked untestable are covered for real by killing the connection's thread from a second connection: a wide streaming result set (REPEAT over a self-join, so it always exceeds the socket buffers) provokes the fetch() error mid-iteration, and the same technique reaches getCurrentSchema's failed-query throw. The entry guard in loadDataFromMysqliStatement() is removed rather than excluded: both call sites dispatch on instanceof mysqli_stmt, unserialize() enforces the typed property union, and instanceof cannot be spoofed, so the check was unreachable from any path. A narrowing @var assignment documents the invariant for static analysis instead. The remaining structurally unreachable branches - a connect_error check a successful real_connect() cannot leave set, fetch_row() on an already-buffered result, the ext-mysqli check the suite itself requires, and the Pdo analyzer-narrowing guards - carry @codeCoverageIgnore blocks stating why. The annotation must sit alone on its comment line; a trailing reason stops php-code-coverage from recognising it. Project statement coverage: 91.05% -> 95.37%. Metadata/Source.php is the only file with uncovered lines left. Signed-off-by: Simon Mundy --- src/Connection.php | 7 + src/Driver.php | 4 + src/Pdo/Connection.php | 8 + src/Pdo/Driver.php | 5 + src/Result.php | 15 +- src/Sql/Ddl/AlterTableDecorator.php | 8 + src/Sql/Ddl/CreateTableDecorator.php | 4 + test/integration/Mysqli/ConnectionTest.php | 92 +++++++ .../Mysqli/StatementResultTest.php | 228 +++++++++++++++++- test/integration/Pdo/ConnectionTest.php | 17 ++ test/unit/ConnectionTest.php | 24 ++ test/unit/Pdo/ConnectionTest.php | 50 ++++ test/unit/Sql/Ddl/AlterTableDecoratorTest.php | 1 + .../unit/Sql/Ddl/CreateTableDecoratorTest.php | 36 +++ 14 files changed, 479 insertions(+), 20 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index 4ab3800..2fe0603 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -186,6 +186,9 @@ public function connect(): ConnectionInterface ); } + // real_connect() returning true never leaves connect_error populated; kept as a guard + // for exotic driver builds. + // @codeCoverageIgnoreStart if ($this->resource->connect_error) { throw new Exception\RuntimeException( 'Connection error', @@ -193,6 +196,7 @@ public function connect(): ConnectionInterface new Exception\ErrorException($this->resource->connect_error, $this->resource->connect_errno), ); } + // @codeCoverageIgnoreEnd if ('' !== ($p['charset'] ?? '')) { $this->resource->set_charset($p['charset']); @@ -264,9 +268,12 @@ public function getCurrentSchema(): string|false } $r = $result->fetch_row(); + // fetch_row() only returns false on a server failure between query and fetch. + // @codeCoverageIgnoreStart if (false === $r) { throw new Exception\RuntimeException($this->resource->error); } + // @codeCoverageIgnoreEnd /** @var array{0: string|null}|null $r */ if (null === $r || null === $r[0]) { diff --git a/src/Driver.php b/src/Driver.php index cbe8117..2084923 100644 --- a/src/Driver.php +++ b/src/Driver.php @@ -50,11 +50,15 @@ public function __construct( #[Override] public function checkEnvironment(): bool { + // The test suite itself requires ext-mysqli, so the missing-extension branch can never + // execute under coverage. + // @codeCoverageIgnoreStart if (! extension_loaded('mysqli')) { throw new Exception\RuntimeException( 'The Mysqli extension is required for this adapter but the extension is not loaded', ); } + // @codeCoverageIgnoreEnd return true; } diff --git a/src/Pdo/Connection.php b/src/Pdo/Connection.php index ad8dcfc..c783502 100644 --- a/src/Pdo/Connection.php +++ b/src/Pdo/Connection.php @@ -134,9 +134,13 @@ public function connect(): ConnectionInterface $this->driverName = strtolower((string) $this->resource->getAttribute(PDO::ATTR_DRIVER_NAME)); } catch (PDOException $e) { $code = $e->getCode(); + // pdo_mysql connect failures carry int codes; the string SQLSTATE form only occurs + // on other PDO drivers. + // @codeCoverageIgnoreStart if (! is_int($code)) { $code = 0; } + // @codeCoverageIgnoreEnd throw new Exception\RuntimeException("Connect Error: {$e->getMessage()}", $code, $e); } @@ -156,11 +160,15 @@ public function getCurrentSchema(): string|false $this->connect(); } + // connect() either assigns the resource or throws, so this guard exists to narrow the + // nullable property for static analysis. + // @codeCoverageIgnoreStart if (null === $this->resource) { throw new Exception\RuntimeException( 'Cannot query current schema without a connected resource; call connect() first.', ); } + // @codeCoverageIgnoreEnd $result = $this->resource->query('SELECT DATABASE()'); if (! $result instanceof PDOStatement) { diff --git a/src/Pdo/Driver.php b/src/Pdo/Driver.php index fa412d1..982a479 100644 --- a/src/Pdo/Driver.php +++ b/src/Pdo/Driver.php @@ -41,9 +41,14 @@ public function __construct( $this->statementPrototype->setDriver($this); // $features is not constructor promoted because $this->features is defined in the trait + // Driver does not implement DriverFeatureProviderInterface yet, so the branch cannot + // execute until feature support lands. + // @codeCoverageIgnoreStart if ([] !== $features && $this instanceof DriverFeatureProviderInterface) { $this->addFeatures($features); } + + // @codeCoverageIgnoreEnd } /** diff --git a/src/Result.php b/src/Result.php index d5bb03d..db9adb3 100644 --- a/src/Result.php +++ b/src/Result.php @@ -307,14 +307,13 @@ public function valid() */ protected function loadDataFromMysqliStatement(): bool { - if (! $this->resource instanceof mysqli_stmt) { - throw new Exception\RuntimeException('Expected resource to be an instance of mysqli_stmt'); - } + /** @var mysqli_stmt $statement Guaranteed by the instanceof dispatch in current() and valid(). */ + $statement = $this->resource; // build the default reference based bind structure, if it does not already exist if (null === $this->statementBindValues['keys']) { $this->statementBindValues['keys'] = []; - $resultResource = $this->resource->result_metadata(); + $resultResource = $statement->result_metadata(); if (false === $resultResource) { return $resultResource; } @@ -332,18 +331,18 @@ protected function loadDataFromMysqliStatement(): bool foreach ($this->statementBindValues['values'] as $i => &$f) { $refs[$i] = &$f; } - call_user_func_array([$this->resource, 'bind_result'], $this->statementBindValues['values']); + call_user_func_array([$statement, 'bind_result'], $this->statementBindValues['values']); } - if (($r = $this->resource->fetch()) === null) { + if (($r = $statement->fetch()) === null) { if (! $this->isBuffered) { - $this->resource->close(); + $statement->close(); } return false; } if (! $r) { - throw new Exception\RuntimeException($this->resource->error); + throw new Exception\RuntimeException($statement->error); } // dereference diff --git a/src/Sql/Ddl/AlterTableDecorator.php b/src/Sql/Ddl/AlterTableDecorator.php index 5531903..61aa12c 100644 --- a/src/Sql/Ddl/AlterTableDecorator.php +++ b/src/Sql/Ddl/AlterTableDecorator.php @@ -38,9 +38,13 @@ public function setSubject( #[Override] protected function processAddColumns(?PlatformInterface $adapterPlatform = null): array { + // AbstractSql substitutes a default platform before calling, so the guard only narrows + // the inherited nullable signature for static analysis. + // @codeCoverageIgnoreStart if (null === $adapterPlatform) { throw new Exception\RuntimeException('Cannot build column SQL without a platform.'); } + // @codeCoverageIgnoreEnd $sqls = []; @@ -70,9 +74,13 @@ protected function processAddColumns(?PlatformInterface $adapterPlatform = null) #[Override] protected function processChangeColumns(?PlatformInterface $adapterPlatform = null): array { + // AbstractSql substitutes a default platform before calling, so the guard only narrows + // the inherited nullable signature for static analysis. + // @codeCoverageIgnoreStart if (null === $adapterPlatform) { throw new Exception\RuntimeException('Cannot build column SQL without a platform.'); } + // @codeCoverageIgnoreEnd $sqls = []; diff --git a/src/Sql/Ddl/CreateTableDecorator.php b/src/Sql/Ddl/CreateTableDecorator.php index fd575b0..d9f4d38 100644 --- a/src/Sql/Ddl/CreateTableDecorator.php +++ b/src/Sql/Ddl/CreateTableDecorator.php @@ -42,9 +42,13 @@ protected function processColumns(?PlatformInterface $adapterPlatform = null): ? return null; } + // AbstractSql substitutes a default platform before calling, so the guard only narrows + // the inherited nullable signature for static analysis. + // @codeCoverageIgnoreStart if (null === $adapterPlatform) { throw new Exception\RuntimeException('Cannot build column SQL without a platform.'); } + // @codeCoverageIgnoreEnd $sqls = []; diff --git a/test/integration/Mysqli/ConnectionTest.php b/test/integration/Mysqli/ConnectionTest.php index e4c748a..efe3f99 100644 --- a/test/integration/Mysqli/ConnectionTest.php +++ b/test/integration/Mysqli/ConnectionTest.php @@ -5,6 +5,7 @@ namespace PhpDbIntegrationTest\Mysql\Mysqli; use mysqli; +use PhpDb\Adapter\Exception\InvalidQueryException; use PhpDb\Adapter\Exception\RuntimeException; use PhpDb\Mysql\Connection; use PhpDb\Mysql\Driver; @@ -16,6 +17,13 @@ use PHPUnit\Framework\TestCase; use function getenv; +use function mysqli_report; +use function usleep; + +use const MYSQLI_OPT_LOCAL_INFILE; +use const MYSQLI_REPORT_ERROR; +use const MYSQLI_REPORT_OFF; +use const MYSQLI_REPORT_STRICT; #[Group('integration')] #[Group('integration-mysqli')] @@ -81,6 +89,17 @@ public function connectAndDisconnect(): void static::assertFalse($connection->isConnected()); } + #[Test] + public function connectTwiceReturnsSameInstance(): void + { + $connection = new Connection($this->connectionParameters()); + new Driver($connection, new Statement(), new Result()); + + $connection->connect(); + + static::assertSame($connection, $connection->connect()); + } + #[Test] public function constructWithMysqliResource(): void { @@ -89,6 +108,24 @@ public function constructWithMysqliResource(): void static::assertTrue($connection->isConnected()); } + #[Test] + public function driverOptionsAreAppliedOnConnect(): void + { + $parameters = $this->connectionParameters(); + $parameters['driver_options'] = [ + 'MYSQLI_OPT_CONNECT_TIMEOUT' => 10, + 'NOT_A_MYSQLI_CONSTANT' => 1, + MYSQLI_OPT_LOCAL_INFILE => 0, + ]; + + $connection = new Connection($parameters); + new Driver($connection, new Statement(), new Result()); + + $connection->connect(); + + static::assertTrue($connection->isConnected()); + } + #[Test] public function executeAutoConnects(): void { @@ -111,6 +148,21 @@ public function executeInsertReturnsGeneratedValue(): void $connection->execute('DELETE FROM test WHERE name = \'generated\''); } + #[Test] + public function executeInvalidSqlThrowsInvalidQueryException(): void + { + $connection = $this->createConnection(); + $connection->connect(); + + mysqli_report(MYSQLI_REPORT_OFF); + try { + $this->expectException(InvalidQueryException::class); + $connection->execute('SELECT FROM WHERE'); + } finally { + mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); + } + } + #[Test] public function executeSelect(): void { @@ -149,6 +201,46 @@ public function getCurrentSchemaAutoConnects(): void static::assertTrue($connection->isConnected()); } + #[Test] + public function getCurrentSchemaOnKilledConnectionThrows(): void + { + $victim = $this->createMysqli(); + $killer = $this->createMysqli(); + + $connection = new Connection($victim); + new Driver($connection, new Statement(), new Result()); + + $killer->query("KILL {$victim->thread_id}"); + usleep(200_000); + + mysqli_report(MYSQLI_REPORT_OFF); + try { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Failed to query current schema'); + $connection->getCurrentSchema(); + } finally { + mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); + } + } + + #[Test] + public function getCurrentSchemaReturnsFalseWithoutSelectedDatabase(): void + { + $parameters = $this->connectionParameters(); + $mysqli = new mysqli( + $parameters['hostname'], + $parameters['username'], + $parameters['password'], + '', + $parameters['port'], + ); + + $connection = new Connection($mysqli); + new Driver($connection, new Statement(), new Result()); + + static::assertFalse($connection->getCurrentSchema()); + } + #[Test] public function rollbackWithoutTransactionThrows(): void { diff --git a/test/integration/Mysqli/StatementResultTest.php b/test/integration/Mysqli/StatementResultTest.php index 6bd92bb..8447c2c 100644 --- a/test/integration/Mysqli/StatementResultTest.php +++ b/test/integration/Mysqli/StatementResultTest.php @@ -8,12 +8,14 @@ use mysqli_result; use mysqli_stmt; use PhpDb\Adapter\Driver\ResultInterface; +use PhpDb\Adapter\Exception\InvalidQueryException; use PhpDb\Adapter\Exception\RuntimeException; use PhpDb\Adapter\ParameterContainer; use PhpDb\Mysql\Connection; use PhpDb\Mysql\Driver; use PhpDb\Mysql\Result; use PhpDb\Mysql\Statement; +use PhpDb\ResultSet\ResultSet; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; @@ -22,6 +24,12 @@ use function getenv; use function is_int; use function iterator_to_array; +use function mysqli_report; +use function usleep; + +use const MYSQLI_REPORT_ERROR; +use const MYSQLI_REPORT_OFF; +use const MYSQLI_REPORT_STRICT; #[Group('integration')] #[Group('integration-mysqli')] @@ -130,6 +138,17 @@ public function countOnNonQueryResultThrows(): void $this->executeNonQuery()->count(); } + #[Test] + public function createStatementConnectsTheConnection(): void + { + $connection = new Connection($this->connectionParameters()); + $driver = new Driver($connection, new Statement(), new Result()); + + $driver->createStatement('SELECT 1'); + + static::assertTrue($connection->isConnected()); + } + #[Test] public function createStatementFromMysqliStmtResource(): void { @@ -154,6 +173,22 @@ public function currentOnNonQueryResultThrows(): void $this->executeNonQuery()->current(); } + #[Test] + public function executeFailingPreparedStatementThrows(): void + { + $statement = $this->createDriver(false) + ->createStatement('INSERT INTO test (id, name, value) VALUES (?, ?, ?)'); + + mysqli_report(MYSQLI_REPORT_OFF); + try { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Duplicate entry'); + $statement->execute($this->createParameterContainer([1, 'dup', 'dup'])); + } finally { + mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); + } + } + #[Test] public function executeWithEmptyArray(): void { @@ -165,6 +200,97 @@ public function executeWithEmptyArray(): void static::assertTrue($result->isQueryResult()); } + #[Test] + public function fetchFailureMidIterationThrows(): void + { + $victim = $this->createMysqli(); + $killer = $this->createMysqli(); + + $driver = new Driver(new Connection($victim), new Statement(bufferResults: false), new Result()); + $result = $driver->createStatement( + "SELECT REPEAT('x', 65536) AS filler FROM test t1 JOIN test t2 JOIN test t3 JOIN test t4", + ) + ->execute([]); + + static::assertNotNull($result); + static::assertNotNull($result->current()); + + $killer->query("KILL {$victim->thread_id}"); + usleep(200_000); + + mysqli_report(MYSQLI_REPORT_OFF); + try { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessageMatches('/gone away|Lost connection/i'); + while ($result->valid()) { + $result->next(); + $result->current(); + } + } finally { + mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); + } + } + + #[Test] + public function getQueryResultClonesTheGivenPrototype(): void + { + $result = $this->createDriver(true) + ->createStatement('SELECT * FROM test WHERE id = ?') + ->execute($this->createParameterContainer([1])); + + static::assertNotNull($result); + static::assertInstanceOf(Result::class, $result); + + $prototype = new ResultSet(); + $resultSet = $result->getQueryResult($prototype); + + static::assertNotSame($prototype, $resultSet); + static::assertInstanceOf(ResultSet::class, $resultSet); + } + + #[Test] + public function getQueryResultOnNonQueryResultThrows(): void + { + $result = $this->executeNonQuery(); + static::assertInstanceOf(Result::class, $result); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + 'Cannot produce a query result set from a result that is not a query result', + ); + + $result->getQueryResult(); + } + + #[Test] + public function getQueryResultSeedsResultSetFromQueryResult(): void + { + $result = $this->createDriver(true) + ->createStatement('SELECT * FROM test WHERE id = ?') + ->execute($this->createParameterContainer([1])); + + static::assertNotNull($result); + static::assertInstanceOf(Result::class, $result); + + $resultSet = $result->getQueryResult(); + + static::assertSame(1, $resultSet->count()); + } + + #[Test] + public function initializeWithStatementDefaultsBufferedStateToUnknown(): void + { + $mysqli = $this->createMysqli(); + $stmt = $mysqli->prepare('SELECT * FROM test'); + static::assertInstanceOf(mysqli_stmt::class, $stmt); + $stmt->execute(); + + $result = new Result(); + $result->initialize($stmt, null); + + static::assertNull($result->isBuffered()); + } + #[Test] public function insertReturnsGeneratedValueAndAffectedRows(): void { @@ -182,6 +308,46 @@ public function insertReturnsGeneratedValueAndAffectedRows(): void ->execute($this->createParameterContainer(['new'])); } + #[Test] + public function nextBeforeAnyFetchAdvancesPosition(): void + { + $result = $this->createDriver(false) + ->createStatement('SELECT * FROM test WHERE id = ?') + ->execute($this->createParameterContainer([1])); + + static::assertNotNull($result); + + $result->next(); + + static::assertSame(1, $result->key()); + } + + #[Test] + public function prepareInvalidSqlThrowsInvalidQueryException(): void + { + $statement = $this->createDriver(false)->createStatement('SELECT FROM WHERE'); + + mysqli_report(MYSQLI_REPORT_OFF); + try { + $this->expectException(InvalidQueryException::class); + $this->expectExceptionMessage("Statement couldn't be produced"); + $statement->prepare(); + } finally { + mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); + } + } + + #[Test] + public function prepareTwiceThrows(): void + { + $statement = $this->createDriver(false)->createStatement('SELECT 1'); + $statement->prepare(); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('This statement has already been prepared'); + $statement->prepare(); + } + #[Test] public function rewindOnNonQueryResultThrows(): void { @@ -222,6 +388,28 @@ public function statementContainerAccessors(): void static::assertSame('SELECT 2', $statement->getSql()); } + #[Test] + public function statementResultWithoutMetadataYieldsNoRows(): void + { + $result = $this->createDriver(false) + ->createStatement('UPDATE test SET value = value WHERE id = ?') + ->execute($this->createParameterContainer([1])); + + static::assertNotNull($result); + static::assertNull($result->current()); + } + + #[Test] + public function unbufferedResultClosesStatementAfterFullIteration(): void + { + $result = $this->createDriver(false) + ->createStatement('SELECT * FROM test WHERE value = ?') + ->execute($this->createParameterContainer(['bar'])); + + static::assertNotNull($result); + static::assertCount(3, iterator_to_array($result, preserve_keys: false)); + } + #[Test] public function unbufferedResultCountThrows(): void { @@ -275,6 +463,28 @@ public function validReturnsTrueAfterCurrent(): void static::assertTrue($result->valid()); } + /** + * @return array{hostname: string, username: string, password: string, database: string, port: int} + */ + private function connectionParameters(): array + { + $host = (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_HOSTNAME'); + if ('' === $host) { + $host = 'localhost'; + } + + $port = (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_PORT'); + $port = '' === $port ? 3306 : (int) $port; + + return [ + 'hostname' => $host, + 'username' => (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_USERNAME'), + 'password' => (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_PASSWORD'), + 'database' => (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_DATABASE'), + 'port' => $port, + ]; + } + private function createDriver(bool $bufferResults = false): Driver { return new Driver( @@ -286,20 +496,14 @@ private function createDriver(bool $bufferResults = false): Driver private function createMysqli(): mysqli { - $host = (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_HOSTNAME'); - if ('' === $host) { - $host = 'localhost'; - } - - $port = (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_PORT'); - $port = '' === $port ? 3306 : (int) $port; + $parameters = $this->connectionParameters(); return new mysqli( - $host, - (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_USERNAME'), - (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_PASSWORD'), - (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_DATABASE'), - $port, + $parameters['hostname'], + $parameters['username'], + $parameters['password'], + $parameters['database'], + $parameters['port'], ); } diff --git a/test/integration/Pdo/ConnectionTest.php b/test/integration/Pdo/ConnectionTest.php index d959a00..7351543 100644 --- a/test/integration/Pdo/ConnectionTest.php +++ b/test/integration/Pdo/ConnectionTest.php @@ -157,6 +157,23 @@ public function getCurrentSchema(): void $connection->disconnect(); } + #[Test] + public function getCurrentSchemaAutoConnects(): void + { + $connection = new Connection([ + 'hostname' => (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_HOSTNAME'), + 'username' => (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_USERNAME'), + 'password' => (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_PASSWORD'), + 'dbname' => (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_DATABASE'), + 'port' => (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_PORT'), + ]); + + static::assertSame( + (string) getenv('TESTS_PHPDB_ADAPTER_MYSQL_DATABASE'), + $connection->getCurrentSchema(), + ); + } + #[Test] public function getLastGeneratedValue(): void { diff --git a/test/unit/ConnectionTest.php b/test/unit/ConnectionTest.php index def32ff..6d1897c 100644 --- a/test/unit/ConnectionTest.php +++ b/test/unit/ConnectionTest.php @@ -7,6 +7,7 @@ use Exception; use mysqli; use Override; +use PhpDb\Adapter\Exception\RuntimeException; use PhpDb\Mysql\Connection; use PhpDb\Mysql\Driver; use PhpDb\Mysql\Result; @@ -24,6 +25,8 @@ #[RequiresPhpExtension('mysqli')] #[CoversMethod(Connection::class, 'setDriver')] #[CoversMethod(Connection::class, 'connect')] +#[CoversMethod(Connection::class, 'execute')] +#[CoversMethod(Connection::class, 'rollback')] final class ConnectionTest extends TestCase { // fake test-only credential, not a real secret @@ -49,6 +52,19 @@ public function connectionFails(): void $connection->connect(); } + #[Test] + public function executeWithoutDriverThrows(): void + { + $mysqli = $this->createStub(mysqli::class); + $mysqli->method('query')->willReturn(true); + + $connection = new Connection($mysqli); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Cannot execute without a driver; call setDriver() first.'); + $connection->execute('SELECT 1'); + } + #[Test] public function getConnectionParameters(): void { @@ -75,6 +91,14 @@ public function nonSecureConnection(): void $connection->connect(); } + #[Test] + public function rollbackWithoutConnectionThrows(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Must be connected before you can rollback.'); + $this->connection->rollback(); + } + #[Test] public function setConnectionParameters(): void { diff --git a/test/unit/Pdo/ConnectionTest.php b/test/unit/Pdo/ConnectionTest.php index b31ef1d..29591e6 100644 --- a/test/unit/Pdo/ConnectionTest.php +++ b/test/unit/Pdo/ConnectionTest.php @@ -5,6 +5,8 @@ namespace PhpDbTest\Mysql\Pdo; use Override; +use PDO; +use PDOException; use PhpDb\Adapter\Exception\InvalidConnectionParametersException; use PhpDb\Adapter\Exception\RuntimeException; use PhpDb\Mysql\Pdo\Connection; @@ -16,8 +18,11 @@ use function sprintf; +#[CoversMethod(Connection::class, '__construct')] #[CoversMethod(Connection::class, 'connect')] #[CoversMethod(Connection::class, 'getDsnParameter')] +#[CoversMethod(Connection::class, 'getCurrentSchema')] +#[CoversMethod(Connection::class, 'getLastGeneratedValue')] final class ConnectionTest extends TestCase { protected Connection $connection; @@ -30,6 +35,7 @@ public static function unsafeDsnParameterProvider(): array 'host appends parameter' => ['host', '127.0.0.1;dbname=other', 'host'], 'charset appends parameter' => ['charset', 'utf8;dbname=other', 'charset'], 'unix_socket appends parameter' => ['unix_socket', '/tmp/mysql.sock;dbname=other', 'unix_socket'], + 'version appends parameter' => ['version', '5.7;dbname=other', 'version'], 'newline in host' => ['host', "127.0.0.1\nhost=attacker.example.com", 'host'], ]; } @@ -44,6 +50,7 @@ public function arrayOfConnectionParametersCreatesCorrectDsn(): void 'dbname' => 'foo', 'port' => '3306', 'unix_socket' => '/var/run/mysqld/mysqld.sock', + 'version' => '5.7', ]); try { $connection->connect(); @@ -58,6 +65,31 @@ public function arrayOfConnectionParametersCreatesCorrectDsn(): void static::assertStringContainsString('dbname=foo', $responseString); static::assertStringContainsString('port=3306', $responseString); static::assertStringContainsString('unix_socket=/var/run/mysqld/mysqld.sock', $responseString); + static::assertStringContainsString('version=5.7', $responseString); + } + + #[Test] + public function connectReturnsSelfWhenConstructedWithPdoInstance(): void + { + $pdo = $this->createStub(PDO::class); + $pdo->method('getAttribute')->willReturn('mysql'); + + $connection = new Connection($pdo); + + static::assertSame($connection, $connection->connect()); + static::assertSame($pdo, $connection->getResource()); + } + + #[Test] + public function getCurrentSchemaReturnsFalseWhenQueryProducesNoStatement(): void + { + $pdo = $this->createStub(PDO::class); + $pdo->method('getAttribute')->willReturn('mysql'); + $pdo->method('query')->willReturn(false); + + $connection = new Connection($pdo); + + static::assertFalse($connection->getCurrentSchema()); } /** @@ -79,6 +111,24 @@ public function getDsn(): void static::assertEquals($dsn, $responseString); } + #[Test] + public function getLastGeneratedValueReturnsFalseWhenDriverThrows(): void + { + $pdo = $this->createStub(PDO::class); + $pdo->method('getAttribute')->willReturn('mysql'); + $pdo->method('lastInsertId')->willThrowException(new PDOException('driver does not support lastInsertId')); + + $connection = new Connection($pdo); + + static::assertFalse($connection->getLastGeneratedValue()); + } + + #[Test] + public function getLastGeneratedValueReturnsFalseWithoutResource(): void + { + static::assertFalse($this->connection->getLastGeneratedValue()); + } + #[Test] public function hostnameAndUnixSocketThrowsInvalidConnectionParametersException(): void { diff --git a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php index 7c040e2..9bf3294 100644 --- a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php @@ -29,6 +29,7 @@ #[CoversMethod(AlterTableDecorator::class, 'setSubject')] #[CoversMethod(AlterTableDecorator::class, 'processAddColumns')] #[CoversMethod(AlterTableDecorator::class, 'processChangeColumns')] +#[CoversMethod(AlterTableDecorator::class, 'resolveAfterOption')] #[CoversTrait(ColumnOptionTrait::class)] #[CoversClass(ColumnFormatEnum::class)] #[CoversClass(StorageEnum::class)] diff --git a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php index 0065343..419817d 100644 --- a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php @@ -237,6 +237,34 @@ public function generatesExpectedSqlForColumnOptions(array $options, string $exp static::assertSame($expected, $this->buildSql($table)); } + #[Test] + public function optionInsertsBeforeInlinePrimaryKey(): void + { + $table = new CreateTable('test'); + $col = new Column\Integer('id'); + $col->addConstraint(new Constraint\PrimaryKey()); + $col->setOption('autoincrement', true); + $table->addColumn($col); + + $sql = $this->buildSql($table); + + static::assertStringContainsString('AUTO_INCREMENT PRIMARY KEY', $sql); + } + + #[Test] + public function optionInsertsBeforeInlineReferences(): void + { + $table = new CreateTable('test'); + $col = new Column\Integer('other_id'); + $col->addConstraint(new Constraint\ForeignKey('fk_other', 'other_id', 'other', 'id')); + $col->setOption('comment', 'linked'); + $table->addColumn($col); + + $sql = $this->buildSql($table); + + static::assertMatchesRegularExpression("/COMMENT 'linked'.*REFERENCES/s", $sql); + } + #[Test] #[DataProvider('unsafeColumnOptionProvider')] public function rejectsColumnOptionValueThatWouldInjectSql( @@ -266,6 +294,14 @@ public function storageOption(): void static::assertStringContainsString('STORAGE DISK', $this->buildSql($table)); } + #[Test] + public function tableWithoutColumnsRendersNoColumnDefinitions(): void + { + $sql = $this->buildSql(new CreateTable('test')); + + static::assertStringContainsString('CREATE TABLE `test`', $sql); + } + #[Test] public function unsignedOption(): void { From 9b005f607593a79b78160a8c055023f5a716a0e6 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 31 Aug 2026 23:01:38 +1000 Subject: [PATCH 11/17] Reduce coverage exclusions to the two irreducible branches Deletes the dead Pdo Driver feature branch and the unreachable second connect_error check, replaces the null-platform and null-resource guards with bare @var narrowings, rewrites the PDOException code fallback as a ternary, and strips explanatory prose from the surviving pragmas. Only the ext-mysqli check and the buffered fetch_row() failure branch remain excluded. Signed-off-by: Simon Mundy --- src/Connection.php | 13 ----------- src/Driver.php | 2 -- src/Pdo/Connection.php | 22 ++++--------------- src/Pdo/Driver.php | 12 +--------- src/Result.php | 2 +- src/Sql/Ddl/AlterTableDecorator.php | 33 ++++++++-------------------- src/Sql/Ddl/ColumnOptionTrait.php | 2 +- src/Sql/Ddl/CreateTableDecorator.php | 16 ++++---------- 8 files changed, 20 insertions(+), 82 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index 2fe0603..1069a9e 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -186,18 +186,6 @@ public function connect(): ConnectionInterface ); } - // real_connect() returning true never leaves connect_error populated; kept as a guard - // for exotic driver builds. - // @codeCoverageIgnoreStart - if ($this->resource->connect_error) { - throw new Exception\RuntimeException( - 'Connection error', - $this->resource->connect_errno, - new Exception\ErrorException($this->resource->connect_error, $this->resource->connect_errno), - ); - } - // @codeCoverageIgnoreEnd - if ('' !== ($p['charset'] ?? '')) { $this->resource->set_charset($p['charset']); } @@ -268,7 +256,6 @@ public function getCurrentSchema(): string|false } $r = $result->fetch_row(); - // fetch_row() only returns false on a server failure between query and fetch. // @codeCoverageIgnoreStart if (false === $r) { throw new Exception\RuntimeException($this->resource->error); diff --git a/src/Driver.php b/src/Driver.php index 2084923..2b0828d 100644 --- a/src/Driver.php +++ b/src/Driver.php @@ -50,8 +50,6 @@ public function __construct( #[Override] public function checkEnvironment(): bool { - // The test suite itself requires ext-mysqli, so the missing-extension branch can never - // execute under coverage. // @codeCoverageIgnoreStart if (! extension_loaded('mysqli')) { throw new Exception\RuntimeException( diff --git a/src/Pdo/Connection.php b/src/Pdo/Connection.php index c783502..6bef316 100644 --- a/src/Pdo/Connection.php +++ b/src/Pdo/Connection.php @@ -22,7 +22,6 @@ use function strtolower; // @mago-expect lint:cyclomatic-complexity -// @mago-expect lint:kan-defect final class Connection extends AbstractPdoConnection { // @mago-expect analysis:write-only-property - read by the parent's final AbstractPdoConnection::getDsn() @@ -134,13 +133,7 @@ public function connect(): ConnectionInterface $this->driverName = strtolower((string) $this->resource->getAttribute(PDO::ATTR_DRIVER_NAME)); } catch (PDOException $e) { $code = $e->getCode(); - // pdo_mysql connect failures carry int codes; the string SQLSTATE form only occurs - // on other PDO drivers. - // @codeCoverageIgnoreStart - if (! is_int($code)) { - $code = 0; - } - // @codeCoverageIgnoreEnd + $code = is_int($code) ? $code : 0; throw new Exception\RuntimeException("Connect Error: {$e->getMessage()}", $code, $e); } @@ -160,17 +153,10 @@ public function getCurrentSchema(): string|false $this->connect(); } - // connect() either assigns the resource or throws, so this guard exists to narrow the - // nullable property for static analysis. - // @codeCoverageIgnoreStart - if (null === $this->resource) { - throw new Exception\RuntimeException( - 'Cannot query current schema without a connected resource; call connect() first.', - ); - } - // @codeCoverageIgnoreEnd + /** @var PDO $resource */ + $resource = $this->resource; - $result = $this->resource->query('SELECT DATABASE()'); + $result = $resource->query('SELECT DATABASE()'); if (! $result instanceof PDOStatement) { return false; } diff --git a/src/Pdo/Driver.php b/src/Pdo/Driver.php index 982a479..ec123df 100644 --- a/src/Pdo/Driver.php +++ b/src/Pdo/Driver.php @@ -7,7 +7,6 @@ use Override; use PDO; use PDOStatement; -use PhpDb\Adapter\Driver\Feature\DriverFeatureProviderInterface; use PhpDb\Adapter\Driver\Pdo\AbstractPdo; use PhpDb\Adapter\Driver\Pdo\Result; use PhpDb\Adapter\Driver\Pdo\Statement; @@ -24,6 +23,7 @@ final class Driver extends AbstractPdo /** * @param array $features */ + // @mago-expect analysis:unused-parameter public function __construct( (PdoConnectionInterface&PdoDriverAwareInterface)|PDO $connection, StatementInterface&PdoDriverAwareInterface $statementPrototype = new Statement(), @@ -39,16 +39,6 @@ public function __construct( } $this->statementPrototype->setDriver($this); - - // $features is not constructor promoted because $this->features is defined in the trait - // Driver does not implement DriverFeatureProviderInterface yet, so the branch cannot - // execute until feature support lands. - // @codeCoverageIgnoreStart - if ([] !== $features && $this instanceof DriverFeatureProviderInterface) { - $this->addFeatures($features); - } - - // @codeCoverageIgnoreEnd } /** diff --git a/src/Result.php b/src/Result.php index db9adb3..b6f7c62 100644 --- a/src/Result.php +++ b/src/Result.php @@ -307,7 +307,7 @@ public function valid() */ protected function loadDataFromMysqliStatement(): bool { - /** @var mysqli_stmt $statement Guaranteed by the instanceof dispatch in current() and valid(). */ + /** @var mysqli_stmt $statement */ $statement = $this->resource; // build the default reference based bind structure, if it does not already exist diff --git a/src/Sql/Ddl/AlterTableDecorator.php b/src/Sql/Ddl/AlterTableDecorator.php index 61aa12c..1b9ac94 100644 --- a/src/Sql/Ddl/AlterTableDecorator.php +++ b/src/Sql/Ddl/AlterTableDecorator.php @@ -8,7 +8,6 @@ use PhpDb\Adapter\Platform\PlatformInterface; use PhpDb\Sql\Ddl\AlterTable; use PhpDb\Sql\Ddl\Column\ColumnInterface; -use PhpDb\Sql\Exception; use PhpDb\Sql\Platform\PlatformDecoratorInterface; use PhpDb\Sql\PreparableSqlInterface; use PhpDb\Sql\SqlInterface; @@ -32,19 +31,12 @@ public function setSubject( /** * @return array> - * - * @throws Exception\RuntimeException */ #[Override] protected function processAddColumns(?PlatformInterface $adapterPlatform = null): array { - // AbstractSql substitutes a default platform before calling, so the guard only narrows - // the inherited nullable signature for static analysis. - // @codeCoverageIgnoreStart - if (null === $adapterPlatform) { - throw new Exception\RuntimeException('Cannot build column SQL without a platform.'); - } - // @codeCoverageIgnoreEnd + /** @var PlatformInterface $platform */ + $platform = $adapterPlatform; $sqls = []; @@ -56,9 +48,9 @@ protected function processAddColumns(?PlatformInterface $adapterPlatform = null) $options = $column->getOptions(); $sqls[$i] = $this->processColumnOptions( - $this->processExpression($column, $adapterPlatform), + $this->processExpression($column, $platform), $options, - $adapterPlatform, + $platform, $this->resolveAfterOption(...), ); } @@ -68,19 +60,12 @@ protected function processAddColumns(?PlatformInterface $adapterPlatform = null) /** * @return array{0: list} - * - * @throws Exception\RuntimeException */ #[Override] protected function processChangeColumns(?PlatformInterface $adapterPlatform = null): array { - // AbstractSql substitutes a default platform before calling, so the guard only narrows - // the inherited nullable signature for static analysis. - // @codeCoverageIgnoreStart - if (null === $adapterPlatform) { - throw new Exception\RuntimeException('Cannot build column SQL without a platform.'); - } - // @codeCoverageIgnoreEnd + /** @var PlatformInterface $platform */ + $platform = $adapterPlatform; $sqls = []; @@ -92,11 +77,11 @@ protected function processChangeColumns(?PlatformInterface $adapterPlatform = nu $options = $column->getOptions(); $sqls[] = [ - $adapterPlatform->quoteIdentifier($name), + $platform->quoteIdentifier($name), $this->processColumnOptions( - $this->processExpression($column, $adapterPlatform), + $this->processExpression($column, $platform), $options, - $adapterPlatform, + $platform, ), ]; } diff --git a/src/Sql/Ddl/ColumnOptionTrait.php b/src/Sql/Ddl/ColumnOptionTrait.php index 5048910..c5c3432 100644 --- a/src/Sql/Ddl/ColumnOptionTrait.php +++ b/src/Sql/Ddl/ColumnOptionTrait.php @@ -113,7 +113,7 @@ protected function processColumnOptions( uksort($options, $this->compareColumnOptions(...)); - // @mago-expect analysis:mixed-assignment - option values are heterogeneous by design + // @mago-expect analysis:mixed-assignment foreach ($options as $name => $value) { if (! $value) { continue; diff --git a/src/Sql/Ddl/CreateTableDecorator.php b/src/Sql/Ddl/CreateTableDecorator.php index d9f4d38..8668233 100644 --- a/src/Sql/Ddl/CreateTableDecorator.php +++ b/src/Sql/Ddl/CreateTableDecorator.php @@ -8,7 +8,6 @@ use PhpDb\Adapter\Platform\PlatformInterface; use PhpDb\Sql\Ddl\Column\ColumnInterface; use PhpDb\Sql\Ddl\CreateTable; -use PhpDb\Sql\Exception; use PhpDb\Sql\Platform\PlatformDecoratorInterface; use PhpDb\Sql\PreparableSqlInterface; use PhpDb\Sql\SqlInterface; @@ -32,8 +31,6 @@ public function setSubject( /** * {@inheritDoc} - * - * @throws Exception\RuntimeException */ #[Override] protected function processColumns(?PlatformInterface $adapterPlatform = null): ?array @@ -42,13 +39,8 @@ protected function processColumns(?PlatformInterface $adapterPlatform = null): ? return null; } - // AbstractSql substitutes a default platform before calling, so the guard only narrows - // the inherited nullable signature for static analysis. - // @codeCoverageIgnoreStart - if (null === $adapterPlatform) { - throw new Exception\RuntimeException('Cannot build column SQL without a platform.'); - } - // @codeCoverageIgnoreEnd + /** @var PlatformInterface $platform */ + $platform = $adapterPlatform; $sqls = []; @@ -60,9 +52,9 @@ protected function processColumns(?PlatformInterface $adapterPlatform = null): ? $options = $column->getOptions(); $sqls[$i] = $this->processColumnOptions( - $this->processExpression($column, $adapterPlatform), + $this->processExpression($column, $platform), $options, - $adapterPlatform, + $platform, ); } From 78f2eb336d8bdb4a0d86b7034dafdb82e8fa461a Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 4 Sep 2026 11:06:09 +1000 Subject: [PATCH 12/17] Drive column option rendering from a single table Slot, emission order and SQL template for every column option now come from one COLUMN_OPTIONS constant in ColumnOptionTrait, replacing the separate sort-order property and the slot literals repeated in each match arm. The match only resolves the value to substitute, so an extra resolver such as AFTER returns the quoted identifier alone and can no longer disagree with the table about where it belongs. Keyword options lean on the enum directly: getColumnOptionKeyword() guards the type, then lets BackedEnum::from() validate and throw its own ValueError. The duplicated getOptionValue() helpers on ColumnFormatEnum and StorageEnum are removed. Tests for unknown keywords now expect ValueError, and non-string keyword values are covered for CREATE TABLE, ADD COLUMN and CHANGE COLUMN. --- src/Sql/ColumnFormatEnum.php | 30 ----- src/Sql/Ddl/AlterTableDecorator.php | 7 +- src/Sql/Ddl/ColumnOptionTrait.php | 116 ++++++++++++------ src/Sql/StorageEnum.php | 32 ----- test/unit/Sql/Ddl/AlterTableDecoratorTest.php | 116 ++++++++++++++++-- .../unit/Sql/Ddl/CreateTableDecoratorTest.php | 79 ++++++++++-- 6 files changed, 256 insertions(+), 124 deletions(-) diff --git a/src/Sql/ColumnFormatEnum.php b/src/Sql/ColumnFormatEnum.php index 1eea274..093353e 100644 --- a/src/Sql/ColumnFormatEnum.php +++ b/src/Sql/ColumnFormatEnum.php @@ -4,16 +4,6 @@ namespace PhpDb\Mysql\Sql; -use PhpDb\Sql\Exception\InvalidArgumentException; - -use function array_map; -use function get_debug_type; -use function implode; -use function is_string; -use function sprintf; -use function strtoupper; -use function trim; - /** * Keywords accepted by the COLUMN_FORMAT column option. * @@ -24,24 +14,4 @@ enum ColumnFormatEnum: string case Fixed = 'FIXED'; case Dynamic = 'DYNAMIC'; case Default = 'DEFAULT'; - - /** - * @return self The case whose value is emitted as the COLUMN_FORMAT keyword. - * @throws InvalidArgumentException If the value is not one of the declared keywords. - */ - public static function getOptionValue(mixed $value): self - { - $keyword = is_string($value) ? strtoupper(trim($value)) : ''; - $format = self::tryFrom($keyword); - - if (null === $format) { - throw new InvalidArgumentException(sprintf( - 'Invalid value for the "columnformat" column option; expected one of %s, received "%s"', - implode(', ', array_map(static fn(self $case): string => $case->value, self::cases())), - is_string($value) ? $value : get_debug_type($value), - )); - } - - return $format; - } } diff --git a/src/Sql/Ddl/AlterTableDecorator.php b/src/Sql/Ddl/AlterTableDecorator.php index 1b9ac94..9602678 100644 --- a/src/Sql/Ddl/AlterTableDecorator.php +++ b/src/Sql/Ddl/AlterTableDecorator.php @@ -89,15 +89,12 @@ protected function processChangeColumns(?PlatformInterface $adapterPlatform = nu return [$sqls]; } - /** - * @return array{string, int<0, 3>}|null - */ - private function resolveAfterOption(string $option, mixed $value, PlatformInterface $platform): ?array + private function resolveAfterOption(string $option, mixed $value, PlatformInterface $platform): ?string { if ('after' !== $option) { return null; } - return [" AFTER {$platform->quoteIdentifier((string) $value)}", 2]; + return $platform->quoteIdentifier((string) $value); } } diff --git a/src/Sql/Ddl/ColumnOptionTrait.php b/src/Sql/Ddl/ColumnOptionTrait.php index c5c3432..a749f8f 100644 --- a/src/Sql/Ddl/ColumnOptionTrait.php +++ b/src/Sql/Ddl/ColumnOptionTrait.php @@ -4,11 +4,17 @@ namespace PhpDb\Mysql\Sql\Ddl; +use BackedEnum; +use Closure; use PhpDb\Adapter\Platform\PlatformInterface; use PhpDb\Mysql\Sql\ColumnFormatEnum; use PhpDb\Mysql\Sql\StorageEnum; use PhpDb\Sql\Exception\InvalidArgumentException; +use ValueError; +use function array_flip; +use function array_key_exists; +use function array_keys; use function count; use function get_debug_type; use function is_string; @@ -19,7 +25,9 @@ use function strlen; use function strpos; use function strtolower; +use function strtoupper; use function substr_replace; +use function trim; use function uksort; /** @@ -41,20 +49,29 @@ trait ColumnOptionTrait { private const string NAME_PATTERN = '/^[A-Za-z0-9_]+$/'; - /** @var array $columnOptionSortOrder Order options are emitted in, lowest first. */ - protected array $columnOptionSortOrder = [ - 'unsigned' => 0, - 'zerofill' => 1, - 'charset' => 2, - 'collate' => 3, - 'identity' => 4, - 'serial' => 4, - 'autoincrement' => 4, - 'comment' => 5, - 'columnformat' => 6, - 'format' => 6, - 'storage' => 7, - 'after' => 8, + /** + * Column options in emission order, each mapped to its insert slot and SQL template. + * + * The slot indexes the array returned by {@see getSqlInsertOffsets()}, so an option's + * position here decides both where it lands in the definition and its order among + * the options sharing that slot. The template receives the resolved value as its + * sole sprintf argument, which flag options simply ignore. + * + * @var array, string}> + */ + private const array COLUMN_OPTIONS = [ + 'unsigned' => [0, ' UNSIGNED'], + 'zerofill' => [0, ' ZEROFILL'], + 'charset' => [0, ' CHARACTER SET %s'], + 'collate' => [0, ' COLLATE %s'], + 'identity' => [1, ' AUTO_INCREMENT'], + 'serial' => [1, ' AUTO_INCREMENT'], + 'autoincrement' => [1, ' AUTO_INCREMENT'], + 'comment' => [2, ' COMMENT %s'], + 'columnformat' => [2, ' COLUMN_FORMAT %s'], + 'format' => [2, ' COLUMN_FORMAT %s'], + 'storage' => [2, ' STORAGE %s'], + 'after' => [2, ' AFTER %s'], ]; /** @@ -100,8 +117,10 @@ protected function getSqlInsertOffsets(string $sql): array * Appends each option to $sql at the offset its keyword belongs to. * * @param array $options - * @param (callable(string, mixed, PlatformInterface): ?array{string, int<0, 3>})|null $resolveExtra - * Resolver for options only valid in the calling statement, tried before the common ones. + * @param (callable(string, mixed, PlatformInterface): ?string)|null $resolveExtra + * Value resolver for options only valid in the calling statement, tried before the common one. + * @throws InvalidArgumentException If an option value would not be safe to emit unquoted. + * @throws ValueError If a COLUMN_FORMAT or STORAGE value is not a keyword its enum declares. */ protected function processColumnOptions( string $sql, @@ -119,15 +138,22 @@ protected function processColumnOptions( continue; } - $option = $this->normalizeColumnOption($name); + $option = $this->normalizeColumnOption($name); + + if (! array_key_exists($option, self::COLUMN_OPTIONS)) { + continue; + } + $resolved = null === $resolveExtra ? null : $resolveExtra($option, $value, $platform); - $resolved ??= $this->resolveColumnOption($option, $value, $platform); + $resolved ??= $this->resolveColumnOptionValue($option, $value, $platform); if (null === $resolved) { continue; } - [$insert, $j] = $resolved; + [$j, $template] = self::COLUMN_OPTIONS[$option]; + + $insert = sprintf($template, $resolved); $length = strlen($insert); foreach ($insertStart as $slot => $offset) { @@ -148,15 +174,36 @@ protected function processColumnOptions( private function compareColumnOptions(string $columnA, string $columnB): int { - $columnA = $this->normalizeColumnOption($columnA); - $columnA = $this->columnOptionSortOrder[$columnA] ?? count($this->columnOptionSortOrder); + $sortOrder = array_flip(array_keys(self::COLUMN_OPTIONS)); + $unknown = count($sortOrder); - $columnB = $this->normalizeColumnOption($columnB); - $columnB = $this->columnOptionSortOrder[$columnB] ?? count($this->columnOptionSortOrder); + $columnA = $sortOrder[$this->normalizeColumnOption($columnA)] ?? $unknown; + $columnB = $sortOrder[$this->normalizeColumnOption($columnB)] ?? $unknown; return $columnA - $columnB; } + /** + * Backed enums match case-sensitively, so the value is upper-cased before it is handed to the enum. + * + * @param Closure(string): BackedEnum $from The enum's from() method, which validates the keyword. + * @return string The keyword to emit, as declared by the matching enum case. + * @throws InvalidArgumentException If the value is not a string. + * @throws ValueError If the value is not one of the declared keywords. + */ + private function getColumnOptionKeyword(string $option, Closure $from, mixed $value): string + { + if (! is_string($value)) { + throw new InvalidArgumentException(sprintf( + 'Invalid value for the "%s" column option; expected a keyword string, received "%s"', + $option, + get_debug_type($value), + )); + } + + return (string) $from(strtoupper(trim($value)))->value; + } + /** * @return string The validated name, unchanged. * @throws InvalidArgumentException If the value is not a bare character set or collation name. @@ -181,21 +228,20 @@ private function normalizeColumnOption(string $name): string } /** - * @return array{string, int<0, 3>}|null The SQL to insert and the offset index it belongs at. - * @throws InvalidArgumentException If the option value would not be safe to emit unquoted. + * @return string|null The value to substitute into the option's SQL template, empty for flags, + * or null when the option is only valid in a statement with its own resolver. + * @throws InvalidArgumentException If the value would not be safe to emit unquoted. + * @throws ValueError If a COLUMN_FORMAT or STORAGE value is not a keyword its enum declares. */ - private function resolveColumnOption(string $option, mixed $value, PlatformInterface $platform): ?array + private function resolveColumnOptionValue(string $option, mixed $value, PlatformInterface $platform): ?string { return match ($option) { - 'unsigned' => [' UNSIGNED', 0], - 'zerofill' => [' ZEROFILL', 0], - 'charset' => [" CHARACTER SET {$this->getColumnOptionName('charset', $value)}", 0], - 'collate' => [" COLLATE {$this->getColumnOptionName('collate', $value)}", 0], - 'identity', 'serial', 'autoincrement' => [' AUTO_INCREMENT', 1], - 'comment' => [" COMMENT {$platform->quoteValue((string) $value)}", 2], - 'columnformat', 'format' => [' COLUMN_FORMAT ' . ColumnFormatEnum::getOptionValue($value)->value, 2], - 'storage' => [' STORAGE ' . StorageEnum::getOptionValue($value)->value, 2], - default => null, + 'after' => null, + 'charset', 'collate' => $this->getColumnOptionName($option, $value), + 'comment' => $platform->quoteValue((string) $value), + 'columnformat', 'format' => $this->getColumnOptionKeyword($option, ColumnFormatEnum::from(...), $value), + 'storage' => $this->getColumnOptionKeyword($option, StorageEnum::from(...), $value), + default => '', }; } } diff --git a/src/Sql/StorageEnum.php b/src/Sql/StorageEnum.php index 5af1342..97059d0 100644 --- a/src/Sql/StorageEnum.php +++ b/src/Sql/StorageEnum.php @@ -4,16 +4,6 @@ namespace PhpDb\Mysql\Sql; -use PhpDb\Sql\Exception\InvalidArgumentException; - -use function array_map; -use function get_debug_type; -use function implode; -use function is_string; -use function sprintf; -use function strtoupper; -use function trim; - /** * Keywords accepted by the STORAGE column option. * @@ -23,26 +13,4 @@ enum StorageEnum: string { case Disk = 'DISK'; case Memory = 'MEMORY'; - - /** - * Backed enums match case-sensitively, so the option value is upper-cased before lookup. - * - * @return self The case whose value is emitted as the STORAGE keyword. - * @throws InvalidArgumentException If the value is not one of the declared keywords. - */ - public static function getOptionValue(mixed $value): self - { - $keyword = is_string($value) ? strtoupper(trim($value)) : ''; - $storage = self::tryFrom($keyword); - - if (null === $storage) { - throw new InvalidArgumentException(sprintf( - 'Invalid value for the "storage" column option; expected one of %s, received "%s"', - implode(', ', array_map(static fn(self $case): string => $case->value, self::cases())), - is_string($value) ? $value : get_debug_type($value), - )); - } - - return $storage; - } } diff --git a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php index 9bf3294..4c0931a 100644 --- a/test/unit/Sql/Ddl/AlterTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/AlterTableDecoratorTest.php @@ -4,6 +4,7 @@ namespace PhpDbTest\Mysql\Sql\Ddl; +use BackedEnum; use PhpDb\Adapter\Driver\Pdo\AbstractPdoConnection; use PhpDb\Adapter\Driver\Pdo\Result; use PhpDb\Adapter\Driver\Pdo\Statement; @@ -17,22 +18,21 @@ use PhpDb\Sql\Ddl\Column; use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDbTest\Mysql\Sql\Ddl\TestAsset\ColumnOptionMatrix; -use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\CoversTrait; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use ValueError; use function sprintf; +use function strtoupper; #[CoversMethod(AlterTableDecorator::class, 'setSubject')] #[CoversMethod(AlterTableDecorator::class, 'processAddColumns')] #[CoversMethod(AlterTableDecorator::class, 'processChangeColumns')] #[CoversMethod(AlterTableDecorator::class, 'resolveAfterOption')] #[CoversTrait(ColumnOptionTrait::class)] -#[CoversClass(ColumnFormatEnum::class)] -#[CoversClass(StorageEnum::class)] final class AlterTableDecoratorTest extends TestCase { protected AdapterPlatform $platform; @@ -65,21 +65,41 @@ public static function changeColumnMatrixProvider(): array ]); } + /** @return array */ + public static function keywordOptionProvider(): array + { + return [ + 'columnformat' => ['columnformat'], + 'storage' => ['storage'], + ]; + } + /** @return array */ public static function unsafeColumnOptionProvider(): array { return [ - 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], - 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], - 'collate statement terminator' => [ + 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], + 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], + 'collate statement terminator' => [ 'collate', 'utf8mb3_unicode_ci; DROP TABLE users; --', 'collate', ], - 'columnformat statement terminator' => ['column_format', 'FIXED; DROP TABLE users; --', 'columnformat'], - 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', 'columnformat'], - 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', 'storage'], - 'storage unknown keyword' => ['storage', 'TAPE', 'storage'], + ]; + } + + /** @return array}> */ + public static function unsafeKeywordOptionProvider(): array + { + return [ + 'columnformat statement terminator' => [ + 'column_format', + 'FIXED; DROP TABLE users; --', + ColumnFormatEnum::class, + ], + 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', ColumnFormatEnum::class], + 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', StorageEnum::class], + 'storage unknown keyword' => ['storage', 'TAPE', StorageEnum::class], ]; } @@ -187,6 +207,44 @@ public function addColumnFormat(): void static::assertStringContainsString('COLUMN_FORMAT FIXED', $this->buildSql($alter)); } + #[Test] + #[DataProvider('unsafeKeywordOptionProvider')] + public function addColumnRejectsKeywordOptionValueThatWouldInjectSql( + string $option, + string $value, + string $enum, + ): void { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, $value); + $alter->addColumn($col); + + $this->expectException(ValueError::class); + $this->expectExceptionMessage(sprintf( + '"%s" is not a valid backing value for enum %s', + strtoupper($value), + $enum, + )); + + $this->buildSql($alter); + } + + #[Test] + #[DataProvider('keywordOptionProvider')] + public function addColumnRejectsNonStringKeywordOptionValue(string $option): void + { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, true); + $alter->addColumn($col); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $option)); + $this->expectExceptionMessage('received "bool"'); + + $this->buildSql($alter); + } + #[Test] #[DataProvider('unsafeColumnOptionProvider')] public function addColumnRejectsOptionValueThatWouldInjectSql( @@ -330,6 +388,44 @@ public function changeColumnIdentity(): void static::assertStringContainsString('AUTO_INCREMENT', $this->buildSql($alter)); } + #[Test] + #[DataProvider('unsafeKeywordOptionProvider')] + public function changeColumnRejectsKeywordOptionValueThatWouldInjectSql( + string $option, + string $value, + string $enum, + ): void { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, $value); + $alter->changeColumn('name', $col); + + $this->expectException(ValueError::class); + $this->expectExceptionMessage(sprintf( + '"%s" is not a valid backing value for enum %s', + strtoupper($value), + $enum, + )); + + $this->buildSql($alter); + } + + #[Test] + #[DataProvider('keywordOptionProvider')] + public function changeColumnRejectsNonStringKeywordOptionValue(string $option): void + { + $alter = new AlterTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, true); + $alter->changeColumn('name', $col); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $option)); + $this->expectExceptionMessage('received "bool"'); + + $this->buildSql($alter); + } + #[Test] #[DataProvider('unsafeColumnOptionProvider')] public function changeColumnRejectsOptionValueThatWouldInjectSql( diff --git a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php index 419817d..17553e2 100644 --- a/test/unit/Sql/Ddl/CreateTableDecoratorTest.php +++ b/test/unit/Sql/Ddl/CreateTableDecoratorTest.php @@ -4,6 +4,7 @@ namespace PhpDbTest\Mysql\Sql\Ddl; +use BackedEnum; use PhpDb\Adapter\Driver\Pdo\AbstractPdoConnection; use PhpDb\Adapter\Driver\Pdo\Result; use PhpDb\Adapter\Driver\Pdo\Statement; @@ -18,20 +19,19 @@ use PhpDb\Sql\Ddl\CreateTable; use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDbTest\Mysql\Sql\Ddl\TestAsset\ColumnOptionMatrix; -use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\CoversTrait; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use ValueError; use function sprintf; +use function strtoupper; #[CoversMethod(CreateTableDecorator::class, 'setSubject')] #[CoversMethod(CreateTableDecorator::class, 'processColumns')] #[CoversTrait(ColumnOptionTrait::class)] -#[CoversClass(ColumnFormatEnum::class)] -#[CoversClass(StorageEnum::class)] final class CreateTableDecoratorTest extends TestCase { protected AdapterPlatform $platform; @@ -50,23 +50,43 @@ public static function columnOptionMatrixProvider(): array ]); } + /** @return array */ + public static function keywordOptionProvider(): array + { + return [ + 'columnformat' => ['columnformat'], + 'storage' => ['storage'], + ]; + } + /** @return array */ public static function unsafeColumnOptionProvider(): array { return [ - 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], - 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], - 'charset backtick' => ['charset', 'utf8mb3` DEFAULT `', 'charset'], - 'collate statement terminator' => [ + 'charset statement terminator' => ['charset', 'utf8mb3; DROP TABLE users; --', 'charset'], + 'charset quoted value' => ['charset', "'utf8mb3'", 'charset'], + 'charset backtick' => ['charset', 'utf8mb3` DEFAULT `', 'charset'], + 'collate statement terminator' => [ 'collate', 'utf8mb3_unicode_ci; DROP TABLE users; --', 'collate', ], - 'collate trailing clause' => ['collate', 'utf8mb3_unicode_ci COMMENT "x"', 'collate'], - 'columnformat statement terminator' => ['column_format', 'FIXED; DROP TABLE users; --', 'columnformat'], - 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', 'columnformat'], - 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', 'storage'], - 'storage unknown keyword' => ['storage', 'TAPE', 'storage'], + 'collate trailing clause' => ['collate', 'utf8mb3_unicode_ci COMMENT "x"', 'collate'], + ]; + } + + /** @return array}> */ + public static function unsafeKeywordOptionProvider(): array + { + return [ + 'columnformat statement terminator' => [ + 'column_format', + 'FIXED; DROP TABLE users; --', + ColumnFormatEnum::class, + ], + 'columnformat unknown keyword' => ['column_format', 'COMPRESSED', ColumnFormatEnum::class], + 'storage statement terminator' => ['storage', 'DISK; DROP TABLE users; --', StorageEnum::class], + 'storage unknown keyword' => ['storage', 'TAPE', StorageEnum::class], ]; } @@ -283,6 +303,41 @@ public function rejectsColumnOptionValueThatWouldInjectSql( $this->buildSql($table); } + #[Test] + #[DataProvider('unsafeKeywordOptionProvider')] + public function rejectsKeywordOptionValueThatWouldInjectSql(string $option, string $value, string $enum): void + { + $table = new CreateTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, $value); + $table->addColumn($col); + + $this->expectException(ValueError::class); + $this->expectExceptionMessage(sprintf( + '"%s" is not a valid backing value for enum %s', + strtoupper($value), + $enum, + )); + + $this->buildSql($table); + } + + #[Test] + #[DataProvider('keywordOptionProvider')] + public function rejectsNonStringKeywordOptionValue(string $option): void + { + $table = new CreateTable('test'); + $col = new Column\Varchar('name', 255); + $col->setOption($option, true); + $table->addColumn($col); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('Invalid value for the "%s" column option', $option)); + $this->expectExceptionMessage('received "bool"'); + + $this->buildSql($table); + } + #[Test] public function storageOption(): void { From ba9ad35f1df81d8fa5604644d0697b0d2c052268 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 4 Sep 2026 11:15:09 +1000 Subject: [PATCH 13/17] Regenerate the analyze baseline for Mago 1.47.5 CI installs the latest Mago, which moved from 1.47.4 to 1.47.5 since the last green run. The new release reports the Metadata\Source $data shape mismatch under invalid-property-assignment-value instead of property-type-coercion, and adds a possibly-undefined index warning on the mysqli Result's bind-value dereference. Both are pre-existing and unrelated to this branch; the baseline is regenerated so it matches under either release. --- analysis-baseline.toml | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/analysis-baseline.toml b/analysis-baseline.toml index a22bef2..66741a0 100644 --- a/analysis-baseline.toml +++ b/analysis-baseline.toml @@ -309,7 +309,13 @@ count = 1 [[issues]] file = "src/Metadata/Source.php" code = "invalid-property-assignment-value" -message = "Invalid type for property `$data`: expected `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`, but got `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_names': non-empty-list>>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`." +message = "Invalid type for property `$data`: expected `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`, but got `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_names': non-empty-list>>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`." +count = 1 + +[[issues]] +file = "src/Metadata/Source.php" +code = "invalid-property-assignment-value" +message = "Invalid type for property `$data`: expected `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`, but got `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints': non-empty-array, 'constraint_name'?: string, 'constraint_type'?: string, 'delete_rule'?: null|string, 'match_option'?: null|string, 'referenced_columns'?: list, 'referenced_table_name'?: null|string, 'referenced_table_schema'?: null|string, 'table_name'?: string, 'update_rule'?: null|string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}`." count = 1 [[issues]] @@ -384,12 +390,6 @@ code = "possibly-undefined-variable" message = "Variable `$name` might not have been defined on all execution paths leading to this point." count = 2 -[[issues]] -file = "src/Metadata/Source.php" -code = "property-type-coercion" -message = "A value of a less specific type `array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints': non-empty-array, 'constraint_name'?: string, 'constraint_type'?: string, 'delete_rule'?: null|string, 'match_option'?: null|string, 'referenced_columns'?: list, 'referenced_table_name'?: null|string, 'referenced_table_schema'?: null|string, 'table_name'?: string, 'update_rule'?: null|string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>}` is being assigned to property `$data` (array{'columns'?: array, 'is_nullable': bool|null, 'numeric_precision': int|null|string, 'numeric_scale': int|null|string, 'numeric_unsigned': bool|null, 'ordinal_position': int|null|string}>>>, 'constraint_keys'?: array>, 'constraint_references'?: array>, 'constraints'?: array, 'constraint_type'?: string, 'delete_rule'?: string, 'match_option'?: string, 'referenced_columns'?: list, 'referenced_table_name'?: string, 'referenced_table_schema'?: string, 'update_rule'?: string}>>>, 'schemas'?: list, 'table_names'?: array>, 'triggers'?: array>})." -count = 1 - [[issues]] file = "src/Metadata/Source.php" code = "redundant-comparison" @@ -486,6 +486,12 @@ code = "possibly-undefined-int-array-index" message = "Possibly undefined array index accessed on `list`." count = 1 +[[issues]] +file = "src/Result.php" +code = "possibly-undefined-int-array-index" +message = "Possibly undefined array key `non-negative-int` accessed on `array`." +count = 1 + [[issues]] file = "src/Result.php" code = "possibly-undefined-int-array-index" From b5facc2333014c1b231b210ba9dfd015cf10c92a Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 4 Sep 2026 11:24:45 +1000 Subject: [PATCH 14/17] Build the mysqli row with array_combine instead of an index loop The dereference loop indexed keys and values by a shared counter, which static analysis could not prove in range and which sat behind five baseline entries. The values array is filled to the length of keys when the bindings are created, so array_combine() expresses the same operation directly and the baseline entries are dropped. --- analysis-baseline.toml | 30 ------------------------------ src/Result.php | 10 ++++++---- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/analysis-baseline.toml b/analysis-baseline.toml index 66741a0..b2fc9a1 100644 --- a/analysis-baseline.toml +++ b/analysis-baseline.toml @@ -468,36 +468,6 @@ code = "missing-constructor" message = 'Class `PhpDb\Mysql\Result` has typed properties without default values but no constructor to initialize them.' count = 1 -[[issues]] -file = "src/Result.php" -code = "possibly-null-array-index" -message = "Possibly using `null` as an array index to access element." -count = 1 - -[[issues]] -file = "src/Result.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array index accessed on `list`." -count = 1 - -[[issues]] -file = "src/Result.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array index accessed on `list`." -count = 1 - -[[issues]] -file = "src/Result.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `non-negative-int` accessed on `array`." -count = 1 - -[[issues]] -file = "src/Result.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `non-negative-int` accessed on `array`." -count = 1 - [[issues]] file = "src/Result.php" code = "unused-property" diff --git a/src/Result.php b/src/Result.php index b6f7c62..d1e790e 100644 --- a/src/Result.php +++ b/src/Result.php @@ -16,6 +16,7 @@ // phpcs:ignore SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse use ReturnTypeWillChange; +use function array_combine; use function array_fill; use function call_user_func_array; use function count; @@ -345,10 +346,11 @@ protected function loadDataFromMysqliStatement(): bool throw new Exception\RuntimeException($statement->error); } - // dereference - for ($i = 0, $count = count($this->statementBindValues['keys']); $i < $count; $i++) { - $this->currentData[$this->statementBindValues['keys'][$i]] = $this->statementBindValues['values'][$i]; - } + // dereference: values was filled to the same length as keys when the bindings were built + $this->currentData = array_combine( + $this->statementBindValues['keys'], + $this->statementBindValues['values'], + ); $this->currentComplete = true; $this->nextComplete = true; $this->position++; From 2879a49c762b05ff2f207468261b3c5c254b6fa2 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 4 Sep 2026 11:30:22 +1000 Subject: [PATCH 15/17] Keep the mysqli row assignment on one line for the formatter --- src/Result.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Result.php b/src/Result.php index d1e790e..4b40b06 100644 --- a/src/Result.php +++ b/src/Result.php @@ -347,10 +347,8 @@ protected function loadDataFromMysqliStatement(): bool } // dereference: values was filled to the same length as keys when the bindings were built - $this->currentData = array_combine( - $this->statementBindValues['keys'], - $this->statementBindValues['values'], - ); + $this->currentData = array_combine($this->statementBindValues['keys'], $this->statementBindValues['values']); + $this->currentComplete = true; $this->nextComplete = true; $this->position++; From fbf9b383d802f936cb51852a4f4363d0275e61fe Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 4 Sep 2026 13:32:32 +1000 Subject: [PATCH 16/17] Type the keyword resolver against the concrete enums Narrowing the closure's return type to ColumnFormatEnum|StorageEnum lets the analyzer see that the case value is a string, so the cast on the return is no longer needed and the BackedEnum import goes with it. --- src/Sql/Ddl/ColumnOptionTrait.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Sql/Ddl/ColumnOptionTrait.php b/src/Sql/Ddl/ColumnOptionTrait.php index a749f8f..770dfd5 100644 --- a/src/Sql/Ddl/ColumnOptionTrait.php +++ b/src/Sql/Ddl/ColumnOptionTrait.php @@ -4,7 +4,6 @@ namespace PhpDb\Mysql\Sql\Ddl; -use BackedEnum; use Closure; use PhpDb\Adapter\Platform\PlatformInterface; use PhpDb\Mysql\Sql\ColumnFormatEnum; @@ -186,7 +185,7 @@ private function compareColumnOptions(string $columnA, string $columnB): int /** * Backed enums match case-sensitively, so the value is upper-cased before it is handed to the enum. * - * @param Closure(string): BackedEnum $from The enum's from() method, which validates the keyword. + * @param Closure(string): (ColumnFormatEnum|StorageEnum) $from The enum's from() method, which validates the keyword. * @return string The keyword to emit, as declared by the matching enum case. * @throws InvalidArgumentException If the value is not a string. * @throws ValueError If the value is not one of the declared keywords. @@ -201,7 +200,7 @@ private function getColumnOptionKeyword(string $option, Closure $from, mixed $va )); } - return (string) $from(strtoupper(trim($value)))->value; + return $from(strtoupper(trim($value)))->value; } /** From 1904ddac4a40ddd1af1dff94544cd3ee7070cb63 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 4 Sep 2026 13:53:11 +1000 Subject: [PATCH 17/17] Changed phpdb dependency to dev --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 55831d1..929684f 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ }, "require": { "php": "~8.3.0 || ~8.4.0 || ~8.5.0", - "php-db/phpdb": "^0.6.0" + "php-db/phpdb": "^0.6.x-dev" }, "require-dev": { "ext-mysqli": "*",