Skip to content

Commit

Permalink
Merge pull request doctrine#3348 from morozov/types
Browse files Browse the repository at this point in the history
Enforce parameter and return value types in the codebase
  • Loading branch information
morozov committed Aug 26, 2019
2 parents d0273e0 + efb900c commit 1fe5f35
Show file tree
Hide file tree
Showing 22 changed files with 99 additions and 185 deletions.
6 changes: 6 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Upgrade to 3.0

## BC BREAK: Changes in the `Doctrine\DBAL\Schema` API

- Column precision no longer defaults to 10. The default value is NULL.
- Asset names are no longer nullable. An empty asset name should be represented as an empty string.
- `Doctrine\DBAL\Schema\AbstractSchemaManager::_getPortableTriggersList()` and `::_getPortableTriggerDefinition()` have been removed.

## BC BREAK: Changes in the `Doctrine\DBAL\Event` API

- `SchemaAlterTableAddColumnEventArgs::addSql()` and the same method in other `SchemaEventArgs`-based classes no longer accept an array of SQL statements. They accept a variadic string.
Expand Down
6 changes: 3 additions & 3 deletions lib/Doctrine/DBAL/Schema/AbstractAsset.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
abstract class AbstractAsset
{
/** @var string */
protected $_name;
protected $_name = '';

/**
* Namespace of the asset. If none isset the default namespace is assumed.
Expand Down Expand Up @@ -138,7 +138,7 @@ public function getName() : string
return $this->_namespace . '.' . $this->_name;
}

return $this->_name ?? '';
return $this->_name;
}

/**
Expand Down Expand Up @@ -167,7 +167,7 @@ public function getQuotedName(AbstractPlatform $platform) : string
*/
protected function _generateIdentifierName(array $columnNames, string $prefix = '', int $maxSize = 30) : string
{
$hash = implode('', array_map(static function ($column) {
$hash = implode('', array_map(static function ($column) : string {
return dechex(crc32($column));
}, $columnNames));

Expand Down
31 changes: 1 addition & 30 deletions lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public function listTableNames() : array
*
* @return array<int, mixed>
*/
protected function filterAssetNames(array $assetNames)
protected function filterAssetNames(array $assetNames) : array
{
$filter = $this->_conn->getConfiguration()->getSchemaAssetsFilter();
if (! $filter) {
Expand Down Expand Up @@ -605,35 +605,6 @@ protected function getPortableNamespaceDefinition(array $namespace) : string
return array_shift($namespace);
}

/**
* @param array<int, array<int, mixed>> $triggers
*
* @return array<int, string>
*/
protected function _getPortableTriggersList(array $triggers)
{
$list = [];
foreach ($triggers as $value) {
$value = $this->_getPortableTriggerDefinition($value);

if (! $value) {
continue;
}

$list[] = $value;
}

return $list;
}

/**
* @param array<string|int, mixed> $trigger
*/
protected function _getPortableTriggerDefinition(array $trigger) : string
{
return array_shift($trigger);
}

/**
* @param array<int, array<string, mixed>> $sequences
*
Expand Down
8 changes: 2 additions & 6 deletions lib/Doctrine/DBAL/Schema/Column.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class Column extends AbstractAsset
protected $_length;

/** @var int|null */
protected $_precision = 10;
protected $_precision;

/** @var int */
protected $_scale = 0;
Expand Down Expand Up @@ -106,10 +106,6 @@ public function setLength(?int $length) : self

public function setPrecision(?int $precision) : self
{
if ($precision === null) {
$precision = 10; // defaults to 10 when no precision is given.
}

$this->_precision = $precision;

return $this;
Expand Down Expand Up @@ -195,7 +191,7 @@ public function getPrecision() : ?int
return $this->_precision;
}

public function getScale() : ?int
public function getScale() : int
{
return $this->_scale;
}
Expand Down
8 changes: 3 additions & 5 deletions lib/Doctrine/DBAL/Schema/ForeignKeyConstraint.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,12 @@ class ForeignKeyConstraint extends AbstractAsset implements Constraint
* @param array<int, string> $localColumnNames Names of the referencing table columns.
* @param Table|string $foreignTableName Referenced table.
* @param array<int, string> $foreignColumnNames Names of the referenced table columns.
* @param string|null $name Name of the foreign key constraint.
* @param string $name Name of the foreign key constraint.
* @param array<string, mixed> $options Options associated with the foreign key constraint.
*/
public function __construct(array $localColumnNames, $foreignTableName, array $foreignColumnNames, $name = null, array $options = [])
public function __construct(array $localColumnNames, $foreignTableName, array $foreignColumnNames, string $name = '', array $options = [])
{
if ($name !== null) {
$this->_setName($name);
}
$this->_setName($name);

$this->_localColumnNames = $this->createIdentifierMap($localColumnNames);

Expand Down
10 changes: 1 addition & 9 deletions lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public function determineExistingSchemaSearchPaths() : void
$names = $this->getSchemaNames();
$paths = $this->getSchemaSearchPaths();

$this->existingSchemaPaths = array_filter($paths, static function ($v) use ($names) {
$this->existingSchemaPaths = array_filter($paths, static function ($v) use ($names) : bool {
return in_array($v, $names);
});
}
Expand Down Expand Up @@ -161,14 +161,6 @@ protected function _getPortableTableForeignKeyDefinition(array $tableForeignKey)
);
}

/**
* {@inheritdoc}
*/
protected function _getPortableTriggerDefinition(array $trigger) : string
{
return $trigger['trigger_name'];
}

/**
* {@inheritdoc}
*/
Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/DBAL/Schema/SQLAnywhereSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ public function dropDatabase(string $database) : void
parent::dropDatabase($database);
}

public function startDatabase(string $database)
public function startDatabase(string $database) : void
{
assert($this->_platform instanceof SQLAnywherePlatform);
$this->_execSql($this->_platform->getStartDatabaseSQL($database));
}

public function stopDatabase(string $database)
public function stopDatabase(string $database) : void
{
assert($this->_platform instanceof SQLAnywherePlatform);
$this->_execSql($this->_platform->getStopDatabaseSQL($database));
Expand Down
2 changes: 0 additions & 2 deletions lib/Doctrine/DBAL/Schema/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,6 @@ public function visit(Visitor $visitor) : void

/**
* Cloning a Schema triggers a deep clone of all related assets.
*
* @return void
*/
public function __clone()
{
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Schema/SchemaDiff.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ protected function _toSql(AbstractPlatform $platform, bool $saveMode = false) :
}
}

if ($platform->supportsSequences() === true) {
if ($platform->supportsSequences()) {
foreach ($this->changedSequences as $sequence) {
$sql[] = $platform->getAlterSequenceSQL($sequence);
}
Expand Down
6 changes: 2 additions & 4 deletions lib/Doctrine/DBAL/Schema/Table.php
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ public function getColumns() : array

$colNames = array_unique(array_merge($pkCols, $fkCols, array_keys($columns)));

uksort($columns, static function ($a, $b) use ($colNames) {
uksort($columns, static function ($a, $b) use ($colNames) : bool {
return array_search($a, $colNames) >= array_search($b, $colNames);
});

Expand Down Expand Up @@ -580,7 +580,7 @@ public function getUniqueConstraints() : array
*
* @return array<string, ForeignKeyConstraint>
*/
public function getForeignKeys()
public function getForeignKeys() : array
{
return $this->_fkConstraints;
}
Expand Down Expand Up @@ -625,8 +625,6 @@ public function visit(Visitor $visitor) : void

/**
* Clone of a Table triggers a deep clone of all affected assets.
*
* @return void
*/
public function __clone()
{
Expand Down
16 changes: 8 additions & 8 deletions lib/Doctrine/DBAL/Schema/Visitor/SchemaDiffVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,25 @@ interface SchemaDiffVisitor
/**
* Visit an orphaned foreign key whose table was deleted.
*/
public function visitOrphanedForeignKey(ForeignKeyConstraint $foreignKey);
public function visitOrphanedForeignKey(ForeignKeyConstraint $foreignKey) : void;

/**
* Visit a sequence that has changed.
*/
public function visitChangedSequence(Sequence $sequence);
public function visitChangedSequence(Sequence $sequence) : void;

/**
* Visit a sequence that has been removed.
*/
public function visitRemovedSequence(Sequence $sequence);
public function visitRemovedSequence(Sequence $sequence) : void;

public function visitNewSequence(Sequence $sequence);
public function visitNewSequence(Sequence $sequence) : void;

public function visitNewTable(Table $table);
public function visitNewTable(Table $table) : void;

public function visitNewTableForeignKey(Table $table, ForeignKeyConstraint $foreignKey);
public function visitNewTableForeignKey(Table $table, ForeignKeyConstraint $foreignKey) : void;

public function visitRemovedTable(Table $table);
public function visitRemovedTable(Table $table) : void;

public function visitChangedTable(TableDiff $tableDiff);
public function visitChangedTable(TableDiff $tableDiff) : void;
}
8 changes: 0 additions & 8 deletions phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@
<exclude name="SlevomatCodingStandard.Classes.DisallowLateStaticBindingForConstants.DisallowedLateStaticBindingForConstant"/>
</rule>

<rule ref="SlevomatCodingStandard.TypeHints.TypeHintDeclaration.MissingParameterTypeHint">
<exclude-pattern>*/lib/*</exclude-pattern>
</rule>

<rule ref="SlevomatCodingStandard.TypeHints.TypeHintDeclaration.MissingReturnTypeHint">
<exclude-pattern>*/lib/*</exclude-pattern>
</rule>

<rule ref="PSR1.Classes.ClassDeclaration.MultipleClasses">
<exclude-pattern>*/tests/*</exclude-pattern>
</rule>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public function testFromConnectionParameters(array $params, string $expected) :
}

/**
* @return mixed[]
* @return iterable<string, array<int, mixed>>
*/
public static function connectionParametersProvider() : iterable
{
Expand Down
11 changes: 0 additions & 11 deletions tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -251,17 +251,6 @@ public function testTransactionalReturnValue() : void
self::assertEquals(42, $res);
}

/**
* Tests that the quote function accepts DBAL and PDO types.
*/
public function testQuote() : void
{
self::assertEquals(
$this->connection->quote('foo'),
$this->connection->quote('foo')
);
}

public function testPingDoesTriggersConnect() : void
{
$this->connection->close();
Expand Down
4 changes: 1 addition & 3 deletions tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -512,11 +512,9 @@ public function testNativeArrayListSupport() : void
}

/**
* @param string|false $char
*
* @dataProvider getTrimExpressionData
*/
public function testTrimExpression(string $value, int $position, $char, string $expectedResult) : void
public function testTrimExpression(string $value, int $position, ?string $char, string $expectedResult) : void
{
$sql = 'SELECT ' .
$this->connection->getDatabasePlatform()->getTrimExpression($value, $position, $char) . ' AS trimmed ' .
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,54 +293,54 @@ public function testListTableColumns() : void

self::assertArrayHasKey('test', $columns);
self::assertEquals(1, array_search('test', $columnsKeys));
self::assertEquals('test', strtolower($columns['test']->getname()));
self::assertInstanceOf(StringType::class, $columns['test']->gettype());
self::assertEquals(255, $columns['test']->getlength());
self::assertEquals(false, $columns['test']->getfixed());
self::assertEquals(false, $columns['test']->getnotnull());
self::assertEquals('expected default', $columns['test']->getdefault());
self::assertEquals('test', strtolower($columns['test']->getName()));
self::assertInstanceOf(StringType::class, $columns['test']->getType());
self::assertEquals(255, $columns['test']->getLength());
self::assertEquals(false, $columns['test']->getFixed());
self::assertEquals(false, $columns['test']->getNotnull());
self::assertEquals('expected default', $columns['test']->getDefault());
self::assertIsArray($columns['test']->getPlatformOptions());

self::assertEquals('foo', strtolower($columns['foo']->getname()));
self::assertEquals('foo', strtolower($columns['foo']->getName()));
self::assertEquals(2, array_search('foo', $columnsKeys));
self::assertInstanceOf(TextType::class, $columns['foo']->gettype());
self::assertEquals(false, $columns['foo']->getunsigned());
self::assertEquals(false, $columns['foo']->getfixed());
self::assertEquals(true, $columns['foo']->getnotnull());
self::assertEquals(null, $columns['foo']->getdefault());
self::assertInstanceOf(TextType::class, $columns['foo']->getType());
self::assertEquals(false, $columns['foo']->getUnsigned());
self::assertEquals(false, $columns['foo']->getFixed());
self::assertEquals(true, $columns['foo']->getNotnull());
self::assertEquals(null, $columns['foo']->getDefault());
self::assertIsArray($columns['foo']->getPlatformOptions());

self::assertEquals('bar', strtolower($columns['bar']->getname()));
self::assertEquals('bar', strtolower($columns['bar']->getName()));
self::assertEquals(3, array_search('bar', $columnsKeys));
self::assertInstanceOf(DecimalType::class, $columns['bar']->gettype());
self::assertEquals(null, $columns['bar']->getlength());
self::assertEquals(10, $columns['bar']->getprecision());
self::assertEquals(4, $columns['bar']->getscale());
self::assertEquals(false, $columns['bar']->getunsigned());
self::assertEquals(false, $columns['bar']->getfixed());
self::assertEquals(false, $columns['bar']->getnotnull());
self::assertEquals(null, $columns['bar']->getdefault());
self::assertInstanceOf(DecimalType::class, $columns['bar']->getType());
self::assertEquals(null, $columns['bar']->getLength());
self::assertEquals(10, $columns['bar']->getPrecision());
self::assertEquals(4, $columns['bar']->getScale());
self::assertEquals(false, $columns['bar']->getUnsigned());
self::assertEquals(false, $columns['bar']->getFixed());
self::assertEquals(false, $columns['bar']->getNotnull());
self::assertEquals(null, $columns['bar']->getDefault());
self::assertIsArray($columns['bar']->getPlatformOptions());

self::assertEquals('baz1', strtolower($columns['baz1']->getname()));
self::assertEquals('baz1', strtolower($columns['baz1']->getName()));
self::assertEquals(4, array_search('baz1', $columnsKeys));
self::assertInstanceOf(DateTimeType::class, $columns['baz1']->gettype());
self::assertEquals(true, $columns['baz1']->getnotnull());
self::assertEquals(null, $columns['baz1']->getdefault());
self::assertInstanceOf(DateTimeType::class, $columns['baz1']->getType());
self::assertEquals(true, $columns['baz1']->getNotnull());
self::assertEquals(null, $columns['baz1']->getDefault());
self::assertIsArray($columns['baz1']->getPlatformOptions());

self::assertEquals('baz2', strtolower($columns['baz2']->getname()));
self::assertEquals('baz2', strtolower($columns['baz2']->getName()));
self::assertEquals(5, array_search('baz2', $columnsKeys));
self::assertContains($columns['baz2']->gettype()->getName(), ['time', 'date', 'datetime']);
self::assertEquals(true, $columns['baz2']->getnotnull());
self::assertEquals(null, $columns['baz2']->getdefault());
self::assertContains($columns['baz2']->getType()->getName(), ['time', 'date', 'datetime']);
self::assertEquals(true, $columns['baz2']->getNotnull());
self::assertEquals(null, $columns['baz2']->getDefault());
self::assertIsArray($columns['baz2']->getPlatformOptions());

self::assertEquals('baz3', strtolower($columns['baz3']->getname()));
self::assertEquals('baz3', strtolower($columns['baz3']->getName()));
self::assertEquals(6, array_search('baz3', $columnsKeys));
self::assertContains($columns['baz3']->gettype()->getName(), ['time', 'date', 'datetime']);
self::assertEquals(true, $columns['baz3']->getnotnull());
self::assertEquals(null, $columns['baz3']->getdefault());
self::assertContains($columns['baz3']->getType()->getName(), ['time', 'date', 'datetime']);
self::assertEquals(true, $columns['baz3']->getNotnull());
self::assertEquals(null, $columns['baz3']->getDefault());
self::assertIsArray($columns['baz3']->getPlatformOptions());
}

Expand Down
Loading

0 comments on commit 1fe5f35

Please sign in to comment.