mysqli_get_client_info doesn't take any parameters

[kamil-tekiela]

mysqli_get_client_info() doesn't take any parameters and it never did. This seems to have been a regression in PHP 8.0.

As documented in PHP manual mysqli_get_client_info doesn't need the connection link. Prior to PHP 8.0 the function didn't take any arguments, so it makes no sense to add it now. This function is just a wrapper around a constant in the client library. Mysqlnd always has this constant set (duh) but I am unable to test against libmysql. However, if it is not set in libmysql then that would have been noticed ages ago since we have a test case for this already https://github.com/php/php-src/blob/master/ext/mysqli/tests/068.phpt

This PR reverts the previous commit and adjusts the stub file.

[cmb69]

That has been done for backwards compatibility, see https://bugs.php.net/78790.

[kamil-tekiela]

@cmb69 Backwards compatibility with what? This function never accepted any parameters. I wrote my fix based on that bug report you shared. I really do not understand why this parameter was added.

[cmb69]

The function hasn't had any ZPP, so it accepted arbitrary argument lists (but completely ignored all the arguments). Originally, it was changed to accept no parameter, but due to the bug report, that restriction was lifted.

[kamil-tekiela]

That bug report should have been declined. If we accept any kind of such bug reports then people will force us to add any number of useless parameters to all kinds of functions. The function never accepted any parameters so there is no BC. The parameter should not have been added.

[cmb69]

The function did accept arbitrary arguments, and it is not even unreasonable to call a function which correlates to a non-static method with an object instance as argument. Of course, the method should have been static in the first place. And let's not forget, that there is still the non-static property ::$client_info:

php > $mysqli = mysqli_init();
php > var_dump($mysqli->client_info);
string(17) "mysqlnd 8.0.2-dev"
php > var_dump(mysqli::$client_info);
Uncaught Error: Access to undeclared static property mysqli::$client_info

[kamil-tekiela]

mysqli doesn't use static properties or functions. I assume it was not possible back in the days of PHP 5. Given that this is not a real property, it should have been a constant. I agree with that much, but changing it to a constant would be a BC.

Many standard functions accepted any number of arguments in PHP 7. The problem is not restricted to only this function. I believe the idea was to restrict this so that only the functions that explicitly allow variadic arguments can take more than is specified in the signature. This is one of them. People used to pass arbitrary arguments to the function that didn't have a use for them.

There are a number of functions that don't take mysqli instance as an argument and belong to mysqli API. This is one of them. We don't have such thing for mysqli_connect_error($mysqli) and yet people complain about it. Why did we single out mysqli_get_client_info() based on a single complaint?

I just answered a question on SO about the BC with mysqli_connect_error($mysqli): https://stackoverflow.com/questions/65566596/fatal-error-uncaught-argumentcounterror-mysqli-connect-errno-expects-exactly

We should stay consistent. Either we disallow passing arbitrary useless arguments to functions that don't need them, or we allow passing any argument to any PHP built-in function.

[cmb69]

Many standard functions accepted any number of arguments in PHP 7.

Yes, but that usually issues E_WARNING. mysqli_get_client_info(1, 2, 3) didn't issue any warning, so it appears to be harsh to suddently fail with a TypeError. This has also been discussed in the respective PR (note that the function is still documented to accept an optional argument).

It might be prudent to deprecate passing arguments to this function ASAP. As they say, "Rome wasn't build in a day". :)

[kamil-tekiela]

I see this really as a bug. I don't see any need for a deprecation message just like we didn't have any for other functions. Don't listen to the mysqli documentation, it is riddled with mistakes. I don't think it's harsh to start throwing an error in PHP 8 for a rarely used function that had a bug. The bug got fixed and now if you try to use it incorrectly you get an error. If we keep this optional parameter it will only make the confusion worse. After a year we might even forget about this and it will never be fixed properly.

When there is a missing error for an incorrect function usage then it is a bug not a feature. We fixed this bug for mysqli_connect_error, mysqli_init() and other functions, and we should fix it for this one as well. I see absolutely no benefit in keeping this incorrect behaviour. Especially that this was done in PHP 8, not in some minor version.

[nikic]

I think the ship for just dropping the parameter in PHP 8 has sailed at this point. It would suggest to change this PR to throw a deprecation notice if the parameter is passed, to make sure that we can at least drop it in the future.

[kamil-tekiela]

Agreed. PR is on its way.