ext/sqlite3: SQLite3::close, SQLite3Stmt::close and SQLite3Result::fi… #10878
Conversation
UPGRADING
Outdated
| @@ -102,6 +102,10 @@ PHP 8.3 UPGRADE NOTES | |||
| argument is non empty with the class not having constructor. | |||
| . pg_insert now raises a ValueError instead of a WARNING when the table specified is invalid. | |||
|
|
|||
| - Sqlite: | |||
| . SQLite3::close, SQLite3Result::finalize and SQLite3Stmt::close are void. | |||
| Previously, they returned bool. | |||
There was a problem hiding this comment.
| Previously, they returned bool. | |
| Previously, they always returned true. |
Seems more accurate
ext/sqlite3/sqlite3.stub.php
Outdated
| @@ -312,7 +312,7 @@ public function __construct(string $filename, int $flags = SQLITE3_OPEN_READWRIT | |||
| public function open(string $filename, int $flags = SQLITE3_OPEN_READWRITE | SQLITE3_OPEN_CREATE, string $encryptionKey = ""): void {} | |||
|
|
|||
| /** @return bool */ | |||
There was a problem hiding this comment.
This comment needs to be removed and I think also replaced with /** @tentative-return-type */
Is this correct @kocsismate ?
There was a problem hiding this comment.
looking at another void function, you seem to be correct.
| @@ -436,5 +436,5 @@ public function fetchArray(int $mode = SQLITE3_BOTH): array|false {} | |||
| public function reset(): bool {} | |||
|
|
|||
| /** @return bool */ | |||
| public function finalize() {} // TODO make return type void | |||
| public function finalize(): void {} | |||
…nalize changed from bool return value to void.
devnexen
force-pushed
the
sqlite3_finalize_close_change
branch
from
66aae9d to
ffb1a1f
Compare
|
I'd love if the signatures in question could be fixed, but I'm also worried about the unsatisfactory upgrade path which I mentioned in a similar PR. I'm afraid we cannot silently change these signatures this fundamentally :( IMO at least there should be an RFC in order to provoke a discussion and in order to settle on a solution which the majority accepts. |
I agree. The |
|
Those are fair points indeed. |
I agree. This path is more reasonable. It will not change the logic, but will encourage people to change the code what will allow to change the return type to |
…nalize
changed from bool return value to void.