PHPC-2124: Ensure that null is still accepted for optional parameters #1347
Conversation
| @@ -41,7 +41,7 @@ final class ClientEncryption | |||
|
|
|||
| final public function __construct(array $options) {} | |||
|
|
|||
| final public function createDataKey(string $kmsProvider, array $options = []): \MongoDB\BSON\Binary {} | |||
| final public function createDataKey(string $kmsProvider, ?array $options = null): \MongoDB\BSON\Binary {} | |||
There was a problem hiding this comment.
Was it necessary to change the default value from [] to null? Or was this mainly to be consistent with the use of Z_PARAM_ARRAY_OR_NULL in the method implementation?
I had used the following for in my proposed changes for #1339:
#if PHP_VERSION_ID >= 80000
final public function rewrapManyDataKey(array|object $filter, ?array $options = []): object {}
#else
/** @param array|object $filter */
final public function rewrapManyDataKey($filter, ?array $options = []): object {}
#endif
This satisfied a PHPT test I created with an explicit null passed as $options. I'm amenable to changing the stub file to use null as you have here to be consistent, but just wanted to confirm whether it was significant.
There was a problem hiding this comment.
This was mainly done for consistency, as omitting the parameter and passing an explicit null has the same effect.
| @@ -9,7 +9,7 @@ | |||
|
|
|||
| final class Decimal128 implements Decimal128Interface, \JsonSerializable, Type, \Serializable | |||
| { | |||
| final public function __construct(string $value = '') {} | |||
There was a problem hiding this comment.
I take it this was always inconsistent, as the constructor has always required 1 parameter and uses Z_PARAM_STRING.
There was a problem hiding this comment.
That is correct - this may be due to me being lazy and using PHPStorm's stubs as base for these and subsequently forgetting to double-check this method: https://github.com/JetBrains/phpstorm-stubs/blob/9d134fcc6c8d4cbb8ffd8e65e25a49fea18a7a0d/mongodb/mongodb.php#L2015
| @@ -66,7 +66,7 @@ ZEND_END_ARG_INFO() | |||
| ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_class_MongoDB_Driver_Manager_executeReadCommand, 0, 2, MongoDB\\Driver\\Cursor, 0) | |||
| ZEND_ARG_TYPE_INFO(0, db, IS_STRING, 0) | |||
| ZEND_ARG_OBJ_INFO(0, command, MongoDB\\Driver\\Command, 0) | |||
| ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, options, IS_ARRAY, 0, "[]") | |||
| ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, options, IS_ARRAY, 1, "null") | |||
There was a problem hiding this comment.
I was wondering why this diff was smaller than the stub file. Looks like gen_stub.php is smart enough to reuse arginfo_class_MongoDB_Driver_Manager_executeReadCommand for executeReadWriteCommand and executeWriteCommand.
PHPC-2124
While reviewing code for the docs, I realised that these stubs don't correspond to the actual ZPP info, which takes precedence as it enforces parsing rules.