Update ibm_db2.stub.php

[phansys]

• Argument 2 for db2_autocommit() is optional, but not nullable;
• Argument 1 in db2_stmt_errormsg(), db2_stmt_error(), db2_conn_errormsg() and db2_conn_error() was explicitly declared as nullable;
• The resource type declaration was removed from argument 1 in db2_specialcolumns(), as is not available (see https://wiki.php.net/rfc/resource_typehint).

[tessus]

Suggested change

	function db2_autocommit($connection, int $value = null): int|bool {}
	function db2_autocommit($connection, int $value): int|bool {}

[phansys]

This parameter is optional. How do we make this explicit if we remove the default value?

pecl-database-ibm_db2/ibm_db2.stub.php

Line 5 in 07750cb

* XXX: How do we represent optionals without defaults?

[tessus]

if this is not nullable, we can't set the default to null, can we?

[phansys]

Currently, the only way in PHP to allow optional parameters is to defining default values for them. Generally, the default value used when the intention is to represent the absence of a value, is null.
See https://3v4l.org/2JIXi.

[tessus]

I get that, but as you can see (https://3v4l.org/utCc3), if you don't specify a value, the default will be NULL, which in turn would result in an error when called this way.

[tessus]

Ok, let me go through the stub again and check the other functions. I just wanted to make the function semantically correct for what is shown in the stub, because right now it is not.
According to the stub, one can send NULL, because NULL would be sent by default if no 2nd argument was passed. And this is just not true.

Maybe I have to take a step back and re-evaluate my understanding of the stub.

[NattyNarwhal]

I'll also note that my translation from the old way of declaring functions to the stub might not be perfect. I tested against the test suite and user reports, but there's definitely the possibility I got some of the signatures wrong.

[phansys]

I'll also note that my translation from the old way of declaring functions to the stub might not be perfect. I tested against the test suite and user reports, but there's definitely the possibility I got some of the signatures wrong.

As far as I can see, there are no problems about the way that the signatures are represented regarding the PHP syntax.

One thing to mention: Even if the language interpreter allows this behavior near nullable arguments without notice, I think we SHOULD use the available syntax following this convention:

1. Argument 1 MUST be passed, allowing int or null:

function test(?int $param) {};

2. Argument 1 is optional, and allows int or null:

function test(?int $param = null) {};

3. Argument 1 is optional, and allows only int (the case in db2_autocommit()):

function test(int $param = null) {};

[kocsismate]

If an optional parameter doesn't have a valid default value then you can use the UNKNOWN pseudo default value. php-src has quite a few of such parameters (although most of them will be phased out if my RFC in progress will be accepted).

[phansys]

Thank you so much for the help @kocsismate!
I've updated the PR with your suggestion.

[NattyNarwhal]

Per php/php-src#8931 - should we be setting the version stuff in the stub?

[kocsismate]

Per php/php-src#8931 - should we be setting the version stuff in the stub?

@NattyNarwhal The default is PHP 7.0, so if you have a minimum PHP version requirement other than this then you should set the version.

Although unfortunately only PHP 7.0 and PHP 8.x is accepted... So I guess then you are not required to set it for now. It may come handy though for future reference if the requirements are bumped to 8.x+.

[NattyNarwhal]

lgtm, though maybe we'd want to wait for helmut's suggested change for autocommit argument handling?

[phansys]

@NattyNarwhal, what do you think about this flow?

1. Merge this PR in order to warranty the stubs are up to date and the headers are in sync with it;
2. Merge [CI] Check that headers are up to date in the CI pipeline #35 to ensure any future change in the stub is up to date with the headers;
3. Merge any other change, like the one proposed by @tessus.

[NattyNarwhal]

That seems reasonable to me.