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/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/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/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/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; 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..dd26f090 100644 --- a/test/unit/Sql/Ddl/Column/IntegerTest.php +++ b/test/unit/Sql/Ddl/Column/IntegerTest.php @@ -6,18 +6,91 @@ 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')] #[CoversMethod(Integer::class, 'getExpressionData')] +#[CoversMethod(Integer::class, 'normaliseDisplayWidth')] #[CoversMethod(Column::class, 'getExpressionData')] #[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 +137,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 +153,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');