-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Conversation
@@ -83,7 +83,7 @@ public function getDatabase(Connection $conn) : ?string | |||
{ | |||
$params = $conn->getParams(); | |||
|
|||
return $params['path'] ?? null; | |||
return $params['path'] ?? ''; |
There was a problem hiding this comment.
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.'); |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Enforced parameter and return value types in Platform classes
Notes:
$database
argument inAbstractPlatform
methods is not final. It just reflects the current type. The type can be restricted to a non-nullablestring
in a subsequent PR.