8000 Stop relying on Type::__toString by greg0ire · Pull Request #2835 · doctrine/dbal · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Stop relying on Type::__toString #2835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,29 @@

namespace Doctrine\DBAL\Platforms;

use Doctrine\DBAL\DBALException;
use Doctrine\Common\EventManager;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Schema\Identifier;
use Doctrine\DBAL\Types;
use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\Event\SchemaAlterTableAddColumnEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableChangeColumnEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableRemoveColumnEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableRenameColumnEventArgs;
use Doctrine\DBAL\Event\SchemaCreateTableColumnEventArgs;
use Doctrine\DBAL\Event\SchemaCreateTableEventArgs;
use Doctrine\DBAL\Event\SchemaDropTableEventArgs;
use Doctrine\DBAL\Events;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\ColumnDiff;
use Doctrine\DBAL\Schema\Constraint;
use Doctrine\DBAL\Schema\ForeignKeyConstraint;
use Doctrine\DBAL\Schema\Identifier;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\Sequence;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\ForeignKeyConstraint;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\ColumnDiff;
use Doctrine\DBAL\Types;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Events;
use Doctrine\Common\EventManager;
use Doctrine\DBAL\Event\SchemaCreateTableEventArgs;
use Doctrine\DBAL\Event\SchemaCreateTableColumnEventArgs;
use Doctrine\DBAL\Event\SchemaDropTableEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableAddColumnEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableRemoveColumnEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableChangeColumnEventArgs;
use Doctrine\DBAL\Event\SchemaAlterTableRenameColumnEventArgs;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this com 8000 ment to others. Learn more.

I reordered the use statements. Is it good or did the previous order have some meaning I should keep?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries on reordering it, I usually do it on a separated commit to make it easier for people to review stuff 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I got lazy there, I'll do that.


/**
* Base class for all DatabasePlatforms. The DatabasePlatforms are the central
Expand Down Expand Up @@ -1556,7 +1556,7 @@ public function getCreateTableSQL(Table $table, $createFlags = self::CREATE_INDE
$columnData['version'] = $column->hasPlatformOption("version") ? $column->getPlatformOption('version') : false;
$columnData['comment'] = $this->getColumnComment($column);

if (strtolower($columnData['type']) == "string" && $columnData['length'] === null) {
if ($columnData['type'] instanceof Types\StringType && $columnData['length'] === null) {
$columnData['length'] = 255;
}

Expand Down Expand Up @@ -2279,15 +2279,17 @@ public function getDefaultValueDeclarationSQL($field)
if (isset($field['default'])) {
$default = " DEFAULT '".$field['default']."'";
if (isset($field['type'])) {
if (in_array((string) $field['type'], ["Integer", "BigInt", "SmallInt"])) {
$type = $field['type'];
if ($type instanceof Types\PhpIntegerMappingType) {
$default = " DEFAULT ".$field['default'];
} elseif (in_array((string) $field['type'], ['DateTime', 'DateTimeTz']) && $field['default'] == $this->getCurrentTimestampSQL()) {
} elseif ($type instanceof Types\PhpDateTimeMappingType
&& $field['default'] == $this->getCurrentTimestampSQL()) {
$default = " DEFAULT ".$this->getCurrentTimestampSQL();
} elseif ((string) $field['type'] == 'Time' && $field['default'] == $this->getCurrentTimeSQL()) {
} elseif ($type instanceof Types\TimeType && $field['default'] == $this->getCurrentTimeSQL()) {
$default = " DEFAULT ".$this->getCurrentTimeSQL();
} elseif ((string) $field['type'] == 'Date' && $field['default'] == $this->getCurrentDateSQL()) {
} elseif ($type instanceof Types\DateType && $field['default'] == $this->getCurrentDateSQL()) {
$default = " DEFAULT ".$this->getCurrentDateSQL();
} elseif ((string) $field['type'] == 'Boolean') {
} elseif ($type instanceof Types\BooleanType) {
$default = " DEFAULT '" . $this->convertBooleans($field['default']) . "'";
}
}
Expand Down
10 changes: 7 additions & 3 deletions lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Types;

/**
* The SQLServerPlatform provides the behavior, features and SQL dialect of the
Expand Down Expand Up @@ -1554,15 +1555,18 @@ public function getDefaultValueDeclarationSQL($field)
return " DEFAULT '" . $field['default'] . "'";
}

if (in_array((string) $field['type'], ['Integer', 'BigInt', 'SmallInt'])) {
$type = $field['type'];

if ($type instanceof Types\PhpIntegerMappingType) {
return " DEFAULT " . $field['default'];
}

if (in_array((string) $field['type'], ['DateTime', 'DateTimeTz']) && $field['default'] == $this->getCurrentTimestampSQL()) {
if ($type instanceof Types\PhpDateTimeMappingType
&& $field['default'] == $this->getCurrentTimestampSQL()) {
return " DEFAULT " . $this->getCurrentTimestampSQL();
}

if ((string) $field['type'] == 'Boolean') {
if ($type instanceof Types\BooleanType) {
return " DEFAULT '" . $this->convertBooleans($field['default']) . "'";
}

Expand Down
17 changes: 9 additions & 8 deletions lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Types;

/**
* The SqlitePlatform class describes the specifics and dialects of the SQLite
Expand Down Expand Up @@ -904,7 +905,7 @@ private function getSimpleAlterTableSQL(TableDiff $diff)
if ( ! $columnDiff->fromColumn instanceof Column ||
! $columnDiff->column instanceof Column ||
! $columnDiff->column->getAutoincrement() ||
! (string) $columnDiff->column->getType() === 'Integer'
! $columnDiff->column->getType() instanceof Types\IntegerType
) {
continue;
}
Expand All @@ -915,9 +916,9 @@ private function getSimpleAlterTableSQL(TableDiff $diff)
continue;
}

$fromColumnType = (string) $columnDiff->fromColumn->getType();
$fromColumnType = $columnDiff->fromColumn->getType();

if ($fromColumnType === 'SmallInt' || $fromColumnType === 'BigInt') {
if ($fromColumnType instanceof Types\SmallIntType || $fromColumnType instanceof Types\BigIntType) {
unset($diff->changedColumns[$oldColumnName]);
}
}
Expand All @@ -942,17 +943,17 @@ private function getSimpleAlterTableSQL(TableDiff $diff)
}

$field = array_merge(['unique' => null, 'autoincrement' => null, 'default' => null], $column->toArray());
$type = (string) $field['type'];
$type = $field['type'];
switch (true) {
case isset($field['columnDefinition']) || $field['autoincrement'] || $field['unique']:
case $type == 'DateTime' && $field['default'] == $this->getCurrentTimestampSQL():
case $type == 'Date' && $field['default'] == $this->getCurrentDateSQL():
case $type == 'Time' && $field['default'] == $this->getCurrentTimeSQL():
case $type instanceof Types\DateTimeType && $field['default'] == $this->getCurrentTimestampSQL():
case $type instanceof Types\DateType && $field['default'] == $this->getCurrentDateSQL():
case $type instanceof Types\TimeType && $field['default'] == $this->getCurrentTimeSQL():
return false;
}

$field['name'] = $column->getQuotedName($this);
if (strtolower($field['type']) == 'string' && $field['length'] === null) {
if ($type instanceof Types\StringType && $field['length'] === null) {
$field['length'] = 255;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Types/BigIntType.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* @author robo
* @since 2.0
*/
class BigIntType extends Type
class BigIntType extends Type implements PhpIntegerMappingType
{
/**
* {@inheritdoc}
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Types/DateTimeType.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
*
* @since 2.0
*/
class DateTimeType extends Type
class DateTimeType extends Type implements PhpDateTimeMappingType
{
/**
* {@inheritdoc}
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Types/DateTimeTzType.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
* @author Jonathan Wage <jonwage@gmail.com>
* @author Roman Borschel <roman@code-factory.org>
*/
class DateTimeTzType extends Type
class DateTimeTzType extends Type implements PhpDateTimeMappingType
{
/**
* {@inheritdoc}
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Types/IntegerType.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* @author Roman Borschel <roman@code-factory.org>
* @since 2.0
*/
class IntegerType extends Type
class IntegerType extends Type implements PhpIntegerMappingType
{
/**
* {@inheritdoc}
Expand Down
30 changes: 30 additions & 0 deletions lib/Doctrine/DBAL/Types/PhpDateTimeMappingType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
/*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PR 10000 OFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* This software consists of voluntary contributions made by many individuals
* and is licensed under the MIT license. For more information, see
* <http://www.doctrine-project.org>.
*/

namespace Doctrine\DBAL\Types;

use Doctrine\DBAL\Platforms\AbstractPlatform;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the point for this use statement?


/**
* Implementations should map a database type to a PHP DateTimeInterface instance.
* @internal
*/
interface PhpDateTimeMappingType
{
}
30 changes: 30 additions & 0 deletions lib/Doctrine/DBAL/Types/PhpIntegerMappingType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
/*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* This software consists of voluntary contributions made by many individuals
* and is licensed under the MIT license. For more information, see
* <http://www.doctrine-project.org>.
*/

namespace Doctrine\DBAL\Types;

use Doctrine\DBAL\Platforms\AbstractPlatform;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the point for this use statement?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None I'm afraid 😓


/**
* Implementations should map a database type to a PHP integer.
* @internal
*/
interface PhpIntegerMappingType
{
}
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Types/SmallIntType.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
*
* @author robo
*/
class SmallIntType extends Type
class SmallIntType extends Type implements PhpIntegerMappingType
{
/**
* {@inheritdoc}
Expand Down
3 changes: 2 additions & 1 deletion tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL461Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Doctrine\Tests\DBAL\Functional\Ticket;

use Doctrine\DBAL\Schema\SQLServerSchemaManager;
use Doctrine\DBAL\Types\DecimalType;

/**
* @group DBAL-461
Expand Down Expand Up @@ -31,6 +32,6 @@ public function testIssue()
'comment' => null,
));

self::assertEquals('Decimal', (string)$column->getType());
$this->assertInstanceOf(DecimalType::class, $column->getType());
}
}
0