From b42f80f460753f3ab3d4497fb32e0eee4f9bfae1 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 7 Sep 2026 19:59:09 +1000 Subject: [PATCH 1/3] Render Integer display width directly after the type The "length" option was concatenated onto the specification string after Column::getExpressionData() had already appended NOT NULL and DEFAULT, so the display width landed after the column attributes and MySQL rejected the statement with error 1064. It was also the only column attribute to reach SQL by string concatenation rather than as an Argument. Insert "(%s)" directly after the type and splice the width in as a Literal, matching AbstractLengthColumn. The option accepts an int or a string of digits and anything else throws InvalidArgumentException, so no raw option value reaches the SQL. setOption() now accepts int, as the constructor options array and the documentation already did. Fixes #178 Signed-off-by: Simon Mundy --- docs/book/sql-ddl/columns.md | 9 +- src/Sql/Ddl/Column/Column.php | 2 +- src/Sql/Ddl/Column/Integer.php | 47 +++++++++- test/unit/Sql/Ddl/Column/BigIntegerTest.php | 15 +++ test/unit/Sql/Ddl/Column/ColumnTest.php | 12 +++ test/unit/Sql/Ddl/Column/IntegerTest.php | 94 ++++++++++++++++++- test/unit/Sql/Ddl/Column/SmallIntegerTest.php | 15 +++ 7 files changed, 187 insertions(+), 7 deletions(-) diff --git a/docs/book/sql-ddl/columns.md b/docs/book/sql-ddl/columns.md index 2d60e9ae..2fb1e08c 100644 --- a/docs/book/sql-ddl/columns.md +++ b/docs/book/sql-ddl/columns.md @@ -14,11 +14,16 @@ use PhpDb\Sql\Ddl\Column\Integer; $column = new Integer('user_id'); $column = new Integer('count', false, 0); // NOT NULL with default 0 -// With display length (platform-specific) +// With display width (platform-specific) $column = new Integer('user_id'); -$column->setOption('length', 11); +$column->setOption('length', 11); // INTEGER(11) ``` +The `length` option is a display width rendered directly after the type, as `INTEGER(11)`. It +accepts an `int` or a string of digits. MySQL deprecated integer display widths in 8.0.17, and the +MySQL platform decorator drops the attribute (see phpdb-mysql#81); other platforms render it as +given. + **Constructor:** ```php diff --git a/src/Sql/Ddl/Column/Column.php b/src/Sql/Ddl/Column/Column.php index 4fec18d3..7e1c41de 100644 --- a/src/Sql/Ddl/Column/Column.php +++ b/src/Sql/Ddl/Column/Column.php @@ -130,7 +130,7 @@ public function setNullable(bool $nullable): static return $this; } - public function setOption(string $name, bool|string $value): static + public function setOption(string $name, bool|int|string $value): static { $this->options[$name] = $value; return $this; diff --git a/src/Sql/Ddl/Column/Integer.php b/src/Sql/Ddl/Column/Integer.php index 4c517515..c816d4e6 100644 --- a/src/Sql/Ddl/Column/Integer.php +++ b/src/Sql/Ddl/Column/Integer.php @@ -5,20 +5,61 @@ namespace PhpDb\Sql\Ddl\Column; use Override; +use PhpDb\Sql\Argument\Literal; +use PhpDb\Sql\Exception\InvalidArgumentException; + +use function array_splice; +use function ctype_digit; +use function is_int; +use function is_string; +use function sprintf; +use function strlen; +use function substr; class Integer extends Column { - /** @inheritDoc */ + /** + * Renders the display width from the "length" option in parentheses directly after the type, + * ahead of the nullability and default clauses. + * + * @inheritDoc + * @throws InvalidArgumentException When the "length" option is not a non-negative integer. + */ #[Override] public function getExpressionData(): array { $expressionData = parent::getExpressionData(); $options = $this->getOptions(); - if (isset($options['length'])) { - $expressionData['spec'] .= " ({$options['length']})"; + if (! isset($options['length'])) { + return $expressionData; } + $displayWidth = $this->normaliseDisplayWidth($options['length']); + $attributes = substr($expressionData['spec'], strlen($this->specification)); + + $expressionData['spec'] = "{$this->specification}(%s){$attributes}"; + array_splice($expressionData['values'], offset: 2, length: 0, replacement: [new Literal($displayWidth)]); + return $expressionData; } + + /** + * @throws InvalidArgumentException When the value is not a non-negative integer. + */ + private function normaliseDisplayWidth(mixed $length): string + { + if (is_int($length) && $length >= 0) { + return (string) $length; + } + + if (is_string($length) && ctype_digit($length)) { + return $length; + } + + throw new InvalidArgumentException(sprintf( + 'Column "%s" length option must be a non-negative integer', + $this->name, + )); + } } diff --git a/test/unit/Sql/Ddl/Column/BigIntegerTest.php b/test/unit/Sql/Ddl/Column/BigIntegerTest.php index 55b330e1..3fc7959b 100644 --- a/test/unit/Sql/Ddl/Column/BigIntegerTest.php +++ b/test/unit/Sql/Ddl/Column/BigIntegerTest.php @@ -7,13 +7,28 @@ use PhpDb\Sql\Argument; use PhpDb\Sql\Ddl\Column\BigInteger; use PhpDb\Sql\Ddl\Column\Column; +use PhpDb\Sql\Ddl\Column\Integer; +use PhpDb\Sql\Ddl\CreateTable; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; #[CoversMethod(BigInteger::class, '__construct')] #[CoversMethod(Column::class, 'getExpressionData')] +#[CoversMethod(Integer::class, 'getExpressionData')] +#[Group('unit')] final class BigIntegerTest extends TestCase { + #[Test] + public function rendersLengthDirectlyAfterType(): void + { + $createTable = new CreateTable('t'); + $createTable->addColumn(new BigInteger('i', false, null, ['length' => 20])); + + static::assertSame("CREATE TABLE \"t\" ( \n \"i\" BIGINT(20) NOT NULL \n)", $createTable->getSqlString()); + } + public function testGetExpressionData(): void { $column = new BigInteger('foo'); diff --git a/test/unit/Sql/Ddl/Column/ColumnTest.php b/test/unit/Sql/Ddl/Column/ColumnTest.php index 2db3fa68..0e496f88 100644 --- a/test/unit/Sql/Ddl/Column/ColumnTest.php +++ b/test/unit/Sql/Ddl/Column/ColumnTest.php @@ -11,6 +11,7 @@ use PhpDb\Sql\Ddl\Constraint\PrimaryKey; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; #[CoversMethod(Column::class, '__construct')] @@ -28,6 +29,17 @@ #[Group('unit')] final class ColumnTest extends TestCase { + #[Test] + public function setOptionAcceptsIntegerValue(): void + { + $column = new Column(); + + $result = $column->setOption('length', 11); + + static::assertSame($column, $result); + static::assertSame(['length' => 11], $column->getOptions()); + } + public function testAddConstraintAppendsConstraintToColumn(): void { $column = new Column('id'); diff --git a/test/unit/Sql/Ddl/Column/IntegerTest.php b/test/unit/Sql/Ddl/Column/IntegerTest.php index 0404d88b..e842b989 100644 --- a/test/unit/Sql/Ddl/Column/IntegerTest.php +++ b/test/unit/Sql/Ddl/Column/IntegerTest.php @@ -6,10 +6,16 @@ use PhpDb\Sql\Argument; use PhpDb\Sql\Ddl\Column\Column; +use PhpDb\Sql\Ddl\Column\ColumnInterface; use PhpDb\Sql\Ddl\Column\Integer; use PhpDb\Sql\Ddl\Constraint\PrimaryKey; +use PhpDb\Sql\Ddl\CreateTable; +use PhpDb\Sql\Exception\InvalidArgumentException; +use PhpDbTest\TestAsset\TrustingSql92Platform; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; #[CoversMethod(Integer::class, '__construct')] @@ -18,6 +24,72 @@ #[Group('unit')] final class IntegerTest extends TestCase { + /** + * @return array + */ + public static function invalidLengthProvider(): array + { + return [ + 'boolean' => [true], + 'negative integer' => [-1], + 'non-digit string' => ['abc'], + ]; + } + + /** + * Asserts what a column renders to inside CREATE TABLE, with values quoted. + */ + private static function assertColumnRenders(string $expected, ColumnInterface $column): void + { + $createTable = new CreateTable('t'); + $createTable->addColumn($column); + + static::assertSame( + "CREATE TABLE \"t\" ( \n {$expected} \n)", + $createTable->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function getExpressionDataPlacesLengthDirectlyAfterType(): void + { + $expressionData = (new Integer('i', false, null, ['length' => 11]))->getExpressionData(); + + static::assertSame('%s %s(%s) NOT NULL', $expressionData['spec']); + static::assertEquals( + [ + Argument::identifier('i'), + Argument::literal('INTEGER'), + Argument::literal('11'), + ], + $expressionData['values'], + ); + } + + #[Test] + public function rendersLengthBeforeNullabilityAndDefault(): void + { + static::assertColumnRenders( + '"i" INTEGER(11) NULL DEFAULT \'7\'', + new Integer('i', true, 7, ['length' => 11]), + ); + } + + #[Test] + public function rendersLengthDirectlyAfterType(): void + { + static::assertColumnRenders('"i" INTEGER(11) NOT NULL', new Integer('i', false, null, ['length' => 11])); + } + + #[Test] + public function rendersLengthSetAsIntegerOption(): void + { + $column = new Integer('i'); + $column->setOption('length', 11); + + static::assertColumnRenders('"i" INTEGER(11) NOT NULL', $column); + } + public function testGetExpressionData(): void { $column = new Integer('foo'); @@ -64,7 +136,15 @@ public function testGetExpressionDataIncludesLengthWhenOptionSet(): void $expressionData = $column->getExpressionData(); - self::assertStringContainsString('(11)', $expressionData['spec']); + static::assertSame('%s %s(%s) NOT NULL', $expressionData['spec']); + static::assertEquals( + [ + Argument::identifier('id'), + Argument::literal('INTEGER'), + Argument::literal('11'), + ], + $expressionData['values'], + ); } public function testObjectConstruction(): void @@ -72,4 +152,16 @@ public function testObjectConstruction(): void $integer = new Integer('foo'); self::assertEquals('foo', $integer->getName()); } + + #[Test] + #[DataProvider('invalidLengthProvider')] + public function throwsWhenLengthOptionIsNotANonNegativeInteger(bool|int|string $length): void + { + $column = new Integer('i', false, null, ['length' => $length]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Column "i" length option must be a non-negative integer'); + + $column->getExpressionData(); + } } diff --git a/test/unit/Sql/Ddl/Column/SmallIntegerTest.php b/test/unit/Sql/Ddl/Column/SmallIntegerTest.php index 62593edb..1218b4da 100644 --- a/test/unit/Sql/Ddl/Column/SmallIntegerTest.php +++ b/test/unit/Sql/Ddl/Column/SmallIntegerTest.php @@ -6,14 +6,29 @@ use PhpDb\Sql\Argument; use PhpDb\Sql\Ddl\Column\Column; +use PhpDb\Sql\Ddl\Column\Integer; use PhpDb\Sql\Ddl\Column\SmallInteger; +use PhpDb\Sql\Ddl\CreateTable; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; #[CoversMethod(SmallInteger::class, '__construct')] #[CoversMethod(Column::class, 'getExpressionData')] +#[CoversMethod(Integer::class, 'getExpressionData')] +#[Group('unit')] final class SmallIntegerTest extends TestCase { + #[Test] + public function rendersLengthDirectlyAfterType(): void + { + $createTable = new CreateTable('t'); + $createTable->addColumn(new SmallInteger('i', false, null, ['length' => 6])); + + static::assertSame("CREATE TABLE \"t\" ( \n \"i\" SMALLINT(6) NOT NULL \n)", $createTable->getSqlString()); + } + public function testGetExpressionData(): void { $column = new SmallInteger('foo'); From e3a2a62f0f68a5d33a09aa87094463d8e94ba4c9 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 7 Sep 2026 21:14:53 +1000 Subject: [PATCH 2/3] Keep the QA toolchain green under Mago 1.47.6 CI installs the newest Mago, and 1.47.5 changed how the analyzer handles nested array shapes: prepareDataHierarchy() re-binds a reference into the MetadataData shape on every loop iteration, which no longer completes in useful time. Re-type the reference as a plain array so the shape is not re-derived per iteration, and adjust the expected findings accordingly. Regenerate the analyzer baseline for the reworded findings in Select, PredicateSet, Profiler and AbstractSql (the issues are unchanged, the messages are not), and reduce the continuation indent of two multi-line conditions in AbstractTableGateway to match the 1.47.4+ formatter. Signed-off-by: Simon Mundy --- analyzer-baseline.toml | 54 +++++------------------ src/Metadata/Source/AbstractSource.php | 10 +++-- src/TableGateway/AbstractTableGateway.php | 8 ++-- 3 files changed, 22 insertions(+), 50 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 26938f84..ae0b6959 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1173,7 +1173,7 @@ count = 1 [[issues]] file = "src/Adapter/Profiler/Profiler.php" code = "invalid-property-assignment-value" -message = '''Invalid type for property `$profiles`: expected `array`, but got `array`.''' +message = '''Invalid type for property `$profiles`: expected `array`, but got `non-empty-array`.''' count = 1 [[issues]] @@ -1194,24 +1194,6 @@ code = "missing-property-type" message = "Property `$profiles` is missing a type hint." count = 1 -[[issues]] -file = "src/Adapter/Profiler/Profiler.php" -code = "possibly-null-operand" -message = "Right operand in arithmetic operation might be `null` (type `float|null`)." -count = 1 - -[[issues]] -file = "src/Adapter/Profiler/Profiler.php" -code = "possibly-undefined-int-array-index" -message = '''Possibly undefined array key `int` accessed on `array`.''' -count = 1 - -[[issues]] -file = "src/Adapter/Profiler/Profiler.php" -code = "possibly-undefined-string-array-index" -message = '''Possibly undefined array key accessed on `array{'elapse': float|null, 'end': float, 'parameters': PhpDb\Adapter\ParameterContainer|null, 'sql': string, 'start': float}|array{'end': float}`.''' -count = 1 - [[issues]] file = "src/Adapter/Profiler/Profiler.php" code = "unhandled-thrown-type" @@ -2289,7 +2271,7 @@ count = 1 [[issues]] file = "src/Sql/AbstractSql.php" code = "less-specific-nested-argument-type" -message = "Argument type mismatch for argument #2 of `vsprintf`: expected `array`, but provided type `array{}|non-empty-list` is less specific." +message = "Argument type mismatch for argument #2 of `vsprintf`: expected `array`, but provided type `list` is less specific." count = 1 [[issues]] @@ -3321,7 +3303,13 @@ count = 1 [[issues]] file = "src/Sql/Ddl/CreateTable.php" code = "possibly-undefined-string-array-index" -message = "Possibly undefined array key `string('combinedBy')` accessed on `array|string>`." +message = "Possibly undefined array key `string('combinedBy')` accessed on `array>`." +count = 1 + +[[issues]] +file = "src/Sql/Ddl/CreateTable.php" +code = "possibly-undefined-string-array-index" +message = "Possibly undefined array key `string('combinedBy')` accessed on `array`." count = 1 [[issues]] @@ -4353,7 +4341,7 @@ count = 1 [[issues]] file = "src/Sql/Predicate/PredicateSet.php" code = "less-specific-nested-argument-type" -message = "Argument type mismatch for argument #2 of `implode`: expected `array|null`, but provided type `array{}|non-empty-list` is less specific." +message = "Argument type mismatch for argument #2 of `implode`: expected `array|null`, but provided type `list` is less specific." count = 1 [[issues]] @@ -4365,7 +4353,7 @@ count = 1 [[issues]] file = "src/Sql/Predicate/PredicateSet.php" code = "less-specific-nested-return-statement" -message = '''Returned type `array{'spec': string, 'values': array{}|list}` is less specific than the declared return type `array{'spec': string, 'values': array}` for function `PhpDb\Sql\Predicate\PredicateSet::getExpressionData` due to nested 'mixed'.''' +message = '''Returned type `array{'spec': string, 'values': list}` is less specific than the declared return type `array{'spec': string, 'values': array}` for function `PhpDb\Sql\Predicate\PredicateSet::getExpressionData` due to nested 'mixed'.''' count = 1 [[issues]] @@ -4623,7 +4611,7 @@ count = 1 [[issues]] file = "src/Sql/Select.php" code = "invalid-property-assignment-value" -message = "Invalid type for property `$specifications`: expected `array>|array`, but got `array|string>`." +message = "Invalid type for property `$specifications`: expected `array>|array`, but got `non-empty-array|string>`." count = 1 [[issues]] @@ -5898,30 +5886,12 @@ code = "class-must-be-final" message = 'Class `PhpDb\TableGateway\Feature\GlobalAdapterFeature` should be declared `final`.' count = 1 -[[issues]] -file = "src/TableGateway/Feature/GlobalAdapterFeature.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\TableGateway\Feature\GlobalAdapterFeature::getStaticAdapter`: expected `PhpDb\Adapter\AdapterInterface`, but found `PhpDb\Adapter\AdapterInterface|null`.' -count = 1 - [[issues]] file = "src/TableGateway/Feature/GlobalAdapterFeature.php" code = "missing-constructor" message = 'Class `PhpDb\TableGateway\Feature\GlobalAdapterFeature` has typed properties without default values but no constructor to initialize them.' count = 1 -[[issues]] -file = "src/TableGateway/Feature/GlobalAdapterFeature.php" -code = "nullable-return-statement" -message = 'Function `PhpDb\TableGateway\Feature\GlobalAdapterFeature::getStaticAdapter` is declared to return `PhpDb\Adapter\AdapterInterface` but possibly returns a nullable value (inferred as `PhpDb\Adapter\AdapterInterface|null`).' -count = 1 - -[[issues]] -file = "src/TableGateway/Feature/GlobalAdapterFeature.php" -code = "possibly-undefined-string-array-index" -message = '''Possibly undefined array key `class-string('PhpDb\TableGateway\Feature\GlobalAdapterFeature')` accessed on `array`.''' -count = 1 - [[issues]] file = "src/TableGateway/Feature/MasterSlaveFeature.php" code = "class-must-be-final" diff --git a/src/Metadata/Source/AbstractSource.php b/src/Metadata/Source/AbstractSource.php index 1c6c333e..7552e872 100644 --- a/src/Metadata/Source/AbstractSource.php +++ b/src/Metadata/Source/AbstractSource.php @@ -693,15 +693,17 @@ protected function loadTriggerData(string $schema): void * Prepare data hierarchy * * The by-reference walk builds arbitrary depths of the hierarchy, which - * cannot be expressed against the MetadataData shape. + * cannot be expressed against the MetadataData shape. The reference is + * re-typed as a plain array so the analyzer does not re-derive the shape + * on every iteration, which does not terminate in useful time on Mago + * 1.47.5 and later. * - * @mago-expect analysis:possibly-undefined-string-array-index - * @mago-expect analysis:possibly-undefined-int-array-index - * @mago-expect analysis:possibly-null-array-access + * @mago-expect analysis:mixed-assignment * @mago-expect lint:no-isset */ protected function prepareDataHierarchy(string $type, string ...$keys): void { + /** @var array $data */ $data = &$this->data; foreach ([$type, ...$keys] as $key) { if (! isset($data[$key])) { diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index 808a1a72..73c38bbf 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -347,8 +347,8 @@ protected function executeSelect(Select $select): ResultSetInterface if ( isset($selectState['columns']) - && [Select::SQL_STAR] === $selectState['columns'] - && [] !== $this->columns + && [Select::SQL_STAR] === $selectState['columns'] + && [] !== $this->columns ) { $select->columns($this->columns); } @@ -430,8 +430,8 @@ public function __clone(): void $this->table = clone $this->table; } elseif ( is_array($this->table) - && count($this->table) === 1 - && is_object(reset($this->table)) + && count($this->table) === 1 + && is_object(reset($this->table)) ) { foreach ($this->table as &$tableObject) { $tableObject = clone $tableObject; From 6107c1ba4dd554412194aac76e2b74afa569d3bf Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 7 Sep 2026 21:20:57 +1000 Subject: [PATCH 3/3] Attribute normaliseDisplayWidth() coverage to IntegerTest The tests declare their covered units with CoversMethod, so the private helper added to Integer was executed but credited to nothing, and codecov reported the patch as 40% covered. Signed-off-by: Simon Mundy --- test/unit/Sql/Ddl/Column/IntegerTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/Sql/Ddl/Column/IntegerTest.php b/test/unit/Sql/Ddl/Column/IntegerTest.php index e842b989..dd26f090 100644 --- a/test/unit/Sql/Ddl/Column/IntegerTest.php +++ b/test/unit/Sql/Ddl/Column/IntegerTest.php @@ -20,6 +20,7 @@ #[CoversMethod(Integer::class, '__construct')] #[CoversMethod(Integer::class, 'getExpressionData')] +#[CoversMethod(Integer::class, 'normaliseDisplayWidth')] #[CoversMethod(Column::class, 'getExpressionData')] #[Group('unit')] final class IntegerTest extends TestCase