From 6b5d226547f924f60c9630cfbf8ea82b15ca5410 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Thu, 28 Mar 2019 23:17:06 -0700 Subject: [PATCH] Reworked `AbstractSchemaManager::extractDoctrineTypeFromComment()` removed `::removeDoctrineTypeFromComment()` --- UPGRADE.md | 4 +++ .../DBAL/Schema/AbstractSchemaManager.php | 33 +++++-------------- lib/Doctrine/DBAL/Schema/DB2SchemaManager.php | 8 ++--- .../DBAL/Schema/MySqlSchemaManager.php | 9 ++--- .../DBAL/Schema/OracleSchemaManager.php | 5 ++- .../DBAL/Schema/PostgreSqlSchemaManager.php | 5 ++- .../DBAL/Schema/SQLAnywhereSchemaManager.php | 14 ++++---- .../DBAL/Schema/SQLServerSchemaManager.php | 5 ++- .../DBAL/Schema/SqliteSchemaManager.php | 10 ++---- .../SchemaManagerFunctionalTestCase.php | 26 +++++++-------- 10 files changed, 44 insertions(+), 75 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 96bd087010..9cd98d20af 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,9 @@ # Upgrade to 3.0 +## BC BREAK `AbstractSchemaManager::extractDoctrineTypeFromComment()` changed, `::removeDoctrineTypeFromComment()` removed + +`AbstractSchemaManager::extractDoctrineTypeFromComment()` made `protected`. It takes the comment by reference, removes the type annotation from it and returns the extracted Doctrine type. + ## BC BREAK `::errorCode()` and `::errorInfo()` removed from `Connection` and `Statement` APIs The error information is available in `DriverException` trown in case of an error. diff --git a/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php b/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php index a3edcdb220..d3e38a3d16 100644 --- a/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php @@ -23,7 +23,6 @@ use function is_array; use function is_callable; use function preg_match; -use function str_replace; use function strtolower; /** @@ -1099,35 +1098,19 @@ public function getSchemaSearchPaths() } /** - * Given a table comment this method tries to extract a typehint for Doctrine Type, or returns - * the type given as default. + * Given a table comment this method tries to extract a type hint for Doctrine Type. If the type hint is found, + * it's removed from the comment. * - * @param string|null $comment - * @param string $currentType - * - * @return string - */ - public function extractDoctrineTypeFromComment($comment, $currentType) - { - if ($comment !== null && preg_match('(\(DC2Type:(((?!\)).)+)\))', $comment, $match)) { - return $match[1]; - } - - return $currentType; - } - - /** - * @param string|null $comment - * @param string|null $type - * - * @return string|null + * @return string|null The extracted Doctrine type or NULL of the type hint was not found. */ - public function removeDoctrineTypeFromComment($comment, $type) + final protected function extractDoctrineTypeFromComment(?string &$comment) : ?string { - if ($comment === null) { + if ($comment === null || ! preg_match('/(.*)\(DC2Type:(((?!\)).)+)\)(.*)/', $comment, $match)) { return null; } - return str_replace('(DC2Type:' . $type . ')', '', $comment); + $comment = $match[1] . $match[4]; + + return $match[2]; } } diff --git a/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php b/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php index 9b8488398c..f6cce30df6 100644 --- a/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php @@ -56,12 +56,8 @@ protected function _getPortableTableColumnDefinition($tableColumn) $default = trim($tableColumn['default'], "'"); } - $type = $this->_platform->getDoctrineTypeMapping($tableColumn['typename']); - - if (isset($tableColumn['comment'])) { - $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type); - $tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type); - } + $type = $this->extractDoctrineTypeFromComment($tableColumn['comment']) + ?? $this->_platform->getDoctrineTypeMapping($tableColumn['typename']); switch (strtolower($tableColumn['typename'])) { case 'varchar': diff --git a/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php b/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php index ba140a6b4d..6dd2caacc5 100644 --- a/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php @@ -108,13 +108,8 @@ protected function _getPortableTableColumnDefinition($tableColumn) $scale = null; $precision = null; - $type = $this->_platform->getDoctrineTypeMapping($dbType); - - // In cases where not connected to a database DESCRIBE $table does not return 'Comment' - if (isset($tableColumn['comment'])) { - $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type); - $tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type); - } + $type = $this->extractDoctrineTypeFromComment($tableColumn['comment']) + ?? $this->_platform->getDoctrineTypeMapping($dbType); switch ($dbType) { case 'char': diff --git a/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php b/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php index 4bf88abc1c..658a7e1be3 100644 --- a/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php @@ -160,9 +160,8 @@ protected function _getPortableTableColumnDefinition($tableColumn) $scale = (int) $tableColumn['data_scale']; } - $type = $this->_platform->getDoctrineTypeMapping($dbType); - $type = $this->extractDoctrineTypeFromComment($tableColumn['comments'], $type); - $tableColumn['comments'] = $this->removeDoctrineTypeFromComment($tableColumn['comments'], $type); + $type = $this->extractDoctrineTypeFromComment($tableColumn['comments']) + ?? $this->_platform->getDoctrineTypeMapping($dbType); switch ($dbType) { case 'number': diff --git a/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php b/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php index 05d887fe77..8f2e752603 100644 --- a/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php @@ -366,9 +366,8 @@ protected function _getPortableTableColumnDefinition($tableColumn) $tableColumn['complete_type'] = $tableColumn['domain_complete_type']; } - $type = $this->_platform->getDoctrineTypeMapping($dbType); - $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type); - $tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type); + $type = $this->extractDoctrineTypeFromComment($tableColumn['comment']) + ?? $this->_platform->getDoctrineTypeMapping($dbType); switch ($dbType) { case 'smallint': diff --git a/lib/Doctrine/DBAL/Schema/SQLAnywhereSchemaManager.php b/lib/Doctrine/DBAL/Schema/SQLAnywhereSchemaManager.php index b99d68429e..df879a87ec 100644 --- a/lib/Doctrine/DBAL/Schema/SQLAnywhereSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/SQLAnywhereSchemaManager.php @@ -88,13 +88,13 @@ protected function _getPortableSequenceDefinition($sequence) */ protected function _getPortableTableColumnDefinition($tableColumn) { - $type = $this->_platform->getDoctrineTypeMapping($tableColumn['type']); - $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type); - $tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type); - $precision = null; - $scale = null; - $fixed = false; - $default = null; + $type = $this->extractDoctrineTypeFromComment($tableColumn['comment']) + ?? $this->_platform->getDoctrineTypeMapping($tableColumn['type']); + + $precision = null; + $scale = null; + $fixed = false; + $default = null; if ($tableColumn['default'] !== null) { // Strip quotes from default value. diff --git a/lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php b/lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php index 08aa1d1e31..9591dc4636 100644 --- a/lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php @@ -101,9 +101,8 @@ protected function _getPortableTableColumnDefinition($tableColumn) $fixed = true; } - $type = $this->_platform->getDoctrineTypeMapping($dbType); - $type = $this->extractDoctrineTypeFromComment($tableColumn['comment'], $type); - $tableColumn['comment'] = $this->removeDoctrineTypeFromComment($tableColumn['comment'], $type); + $type = $this->extractDoctrineTypeFromComment($tableColumn['comment']) + ?? $this->_platform->getDoctrineTypeMapping($dbType); $options = [ 'length' => $length === 0 || ! in_array($type, ['text', 'string']) ? null : $length, diff --git a/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php b/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php index c975fb1783..6640fd508b 100644 --- a/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php @@ -281,16 +281,10 @@ protected function _getPortableTableColumnList($table, $database, $tableColumns) $comment = $this->parseColumnCommentFromSQL($columnName, $createSql); - if ($comment === null) { - continue; - } + $type = $this->extractDoctrineTypeFromComment($comment); - $type = $this->extractDoctrineTypeFromComment($comment, ''); - - if ($type !== '') { + if ($type !== null) { $column->setType(Type::getType($type)); - - $comment = $this->removeDoctrineTypeFromComment($comment, $type); } $column->setComment($comment); diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php index 98995fa8ee..07b0cb6b03 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php @@ -32,6 +32,7 @@ use Doctrine\DBAL\Types\TextType; use Doctrine\DBAL\Types\Type; use Doctrine\Tests\DbalFunctionalTestCase; +use ReflectionMethod; use function array_filter; use function array_keys; use function array_map; @@ -1440,27 +1441,26 @@ public function testComparatorShouldNotAddCommentToJsonTypeSinceItIsTheDefaultNo * @dataProvider commentsProvider * @group 2596 */ - public function testExtractDoctrineTypeFromComment(string $comment, string $expected, string $currentType) : void + public function testExtractDoctrineTypeFromComment(?string $comment, ?string $expectedType) : void { - $result = $this->schemaManager->extractDoctrineTypeFromComment($comment, $currentType); + $re = new ReflectionMethod($this->schemaManager, 'extractDoctrineTypeFromComment'); + $re->setAccessible(true); - self::assertSame($expected, $result); + self::assertSame($expectedType, $re->invokeArgs($this->schemaManager, [&$comment])); } /** - * @return string[][] + * @return mixed[][] */ - public function commentsProvider() : array + public static function commentsProvider() : iterable { - $currentType = 'current type'; - return [ - 'invalid custom type comments' => ['should.return.current.type', $currentType, $currentType], - 'valid doctrine type' => ['(DC2Type:guid)', 'guid', $currentType], - 'valid with dots' => ['(DC2Type:type.should.return)', 'type.should.return', $currentType], - 'valid with namespace' => ['(DC2Type:Namespace\Class)', 'Namespace\Class', $currentType], - 'valid with extra closing bracket' => ['(DC2Type:should.stop)).before)', 'should.stop', $currentType], - 'valid with extra opening brackets' => ['(DC2Type:should((.stop)).before)', 'should((.stop', $currentType], + 'invalid custom type comments' => ['should.return.null', null], + 'valid doctrine type' => ['(DC2Type:guid)', 'guid'], + 'valid with dots' => ['(DC2Type:type.should.return)', 'type.should.return'], + 'valid with namespace' => ['(DC2Type:Namespace\Class)', 'Namespace\Class'], + 'valid with extra closing bracket' => ['(DC2Type:should.stop)).before)', 'should.stop'], + 'valid with extra opening brackets' => ['(DC2Type:should((.stop)).before)', 'should((.stop'], ]; }