Incorrect assumptions about mysqli object parameters

[jnvsor]

Example: https://psalm.dev/r/c8e212f816

Psalm assumes the PHP documentation is correct. It is not. Specifically the Mysqli object has a lot of shady stuff going on with strange states of the connection.

In PHP 7.4 sqlstate is false in an empty Mysqli object while client_info is a string. client_info will also be false when a connection is attempted and fails. To make matters worse, the behavior of the class in edge cases like this changes between PHP versions. (I know it's different in PHP 8 and in older PHP 5 versions)

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/c8e212f816

```php sqlstate)) { echo "We have sqlstate"; } else { echo "We have no sqlstate"; } $m = new mysqli('localhost', 'root', 'nottherightpassword'); if (\is_string(@$m->client_info)) { echo "We have client_info"; } else { echo "We have no client_info"; } ``` ``` Psalm output (using commit b8f5d16): ERROR: RedundantCondition - 5:5 - string always contains string ERROR: RedundantCondition - 13:5 - string always contains string ```

[orklah]

Would you happen to have examples for those behaviours in the different PHP versions? Psalm can be pretty detailed if needed, if we have the correct types, we should be able to enforce them a little better

[jnvsor]

I'm pretty sure it's not just these 2, but most properties that have unexpected types with an invalid mysqli object. Just going by the docs I tried it with affected_rows and got the same.

I think the properties not listed by var_dump are probably false:

class mysqli#1 (6) {
  public $client_info =>
  string(14) "mysqlnd 7.4.11"
  public $client_version =>
  int(70411)
  public $connect_errno =>
  int(2002)
  public $connect_error =>
  string(25) "No such file or directory"
  public $errno =>
  int(0)
  public $error =>
  string(0) ""
}
class mysqli#2 (3) {
  public $client_version =>
  int(70411)
  public $connect_errno =>
  int(2002)
  public $connect_error =>
  string(25) "No such file or directory"
}

Counter-intuitively, the larger object is the empty one, and the smaller one is the one that failed to connect.

I hit the different behaviors when my tests broke in php-next in CI a while back - I think the really old version was PHP 5.2 or earlier so you might as well ignore that, but in PHP 8 it changed from 7. I'm working on PHP8 support right now so I'll be able to give you an example either tomorrow or next weekend.

For the purposes of psalm you might be able to just stick |false on everything but client_version, connect_errno, and connect_error. Of course, accessing these values on a disconnected or empty object triggers a warning so perhaps psalm should warn about that too, though that's getting into complicated territory.

[orklah]

Well, for now, here's what Psalm have: https://github.com/vimeo/psalm/blob/master/dictionaries/PropertyMap.php#L251

Contrary to what I've said earlier, PropertyMap can't be detail version by php version (yet?) but feel free to push a PR to improve what's already there if you have examples to back it up.

[jnvsor]

I can confirm in php 7 the only properties that can't be false are client_version, connect_errno, and connect_error

In PHP 7 accessing non-existant properties triggers a warning and returns null, while accessing the false values triggers a different warning.

But in PHP 8, while accessing non-existant properties still triggers a warning and returns null, accessing the false values throws an Error - I think there really needs to be a separate test for these fields given they could start throwing errors around in php 8

[jnvsor]

As of 8.1 failed connections will throw exceptions, but you can still create empty Mysqli instances so this only clears up client_info, errno, and error