Skip to content
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

Enforced parameter and return value types in Platform classes #3562

Merged
merged 1 commit into from
May 28, 2019

Conversation

morozov
Copy link
Member

@morozov morozov commented May 28, 2019

Q A
Type improvement
BC Break yes

Notes:

  1. The nullability of the $database argument in AbstractPlatform methods is not final. It just reflects the current type. The type can be restricted to a non-nullable string in a subsequent PR.
  2. Some mocks have been reworked from using the PHPUnit mock API instead of Prophecy because the latter doesn't respect the return types of the mocked methods.

@morozov morozov added the WIP label May 28, 2019
@morozov morozov added this to the 3.0.0 milestone May 28, 2019
@@ -83,7 +83,7 @@ public function getDatabase(Connection $conn) : ?string
{
$params = $conn->getParams();

return $params['path'] ?? null;
return $params['path'] ?? '';
Copy link
Member Author

@morozov morozov May 28, 2019

Choose a reason for hiding this comment

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

The returned NULL implies that there's no database currently selected, however, SQLite doesn't support databases and one is always selected implicitly. Some of the platform methods do not allow NULL for $database which previously was only documented but not enforced.

The method documentation will be updated in a subsequent pull request.

*/
public function testReturnsDatabaseNameWithoutDatabaseNameParameter()
{
$this->markTestSkipped('SQLite does not support the concept of a database.');
Copy link
Member Author

@morozov morozov May 28, 2019

Choose a reason for hiding this comment

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

Previously, this test would only pass with an in-memory SQLite database for which Driver::getDatabase() would return NULL. The test is invalid with SQLite by design.

@@ -884,10 +875,6 @@ public function testGeneratesSequenceSqlCommands()
'SELECT myseq.NEXTVAL',
$this->platform->getSequenceNextValSQL('myseq')
);
self::assertEquals(
'SELECT sequence_name, increment_by, start_with, min_value FROM SYS.SYSSEQUENCE',
$this->platform->getListSequencesSQL(null)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is no longer valid.

Copy link
Member

@jwage jwage left a comment

Choose a reason for hiding this comment

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

💯

@morozov morozov merged commit 7124688 into doctrine:develop May 28, 2019
@morozov morozov deleted the platform-types branch May 28, 2019 16:17
morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit to morozov/dbal that referenced this pull request May 31, 2019
Enforced parameter and return value types in Platform classes
@morozov morozov self-assigned this Jun 7, 2019
morozov added a commit that referenced this pull request Jun 13, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit that referenced this pull request Jun 27, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit that referenced this pull request Jun 27, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit that referenced this pull request Jun 27, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit to morozov/dbal that referenced this pull request Aug 26, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit to morozov/dbal that referenced this pull request Aug 27, 2019
Enforced parameter and return value types in Platform classes
morozov added a commit that referenced this pull request Nov 2, 2019
Enforced parameter and return value types in Platform classes
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants