Skip to content

ReflectionClass can't detect readonly properties for mysqli #15309

@RV7PR

Description

@RV7PR

Description

The following code:

<?php
$reflection = new ReflectionClass(mysqli::class);
$props = [];
foreach ($reflection->getProperties() as $property) {
    $props[] = $property->getName() . ' | readonly: ' . (int) $property->isReadOnly();
}
print_r($props);

Resulted in this output:

Array
(
    [0] => affected_rows | readonly: 0
    [1] => client_info | readonly: 0
    [2] => client_version | readonly: 0
    [3] => connect_errno | readonly: 0
    [4] => connect_error | readonly: 0
    [5] => errno | readonly: 0
    [6] => error | readonly: 0
    [7] => error_list | readonly: 0
    [8] => field_count | readonly: 0
    [9] => host_info | readonly: 0
    [10] => info | readonly: 0
    [11] => insert_id | readonly: 0
    [12] => server_info | readonly: 0
    [13] => server_version | readonly: 0
    [14] => sqlstate | readonly: 0
    [15] => protocol_version | readonly: 0
    [16] => thread_id | readonly: 0
    [17] => warning_count | readonly: 0
)

But I expected this output instead:

Array
(
    [0] => affected_rows | readonly: 1
    [1] => client_info | readonly: 1
    [2] => client_version | readonly: 1
    [3] => connect_errno | readonly: 1
    [4] => connect_error | readonly: 1
    [5] => errno | readonly: 1
    [6] => error | readonly: 1
    [7] => error_list | readonly: 1
    [8] => field_count | readonly: 1
    [9] => host_info | readonly: 1
    [10] => info | readonly: 1
    [11] => insert_id | readonly: 1
    [12] => server_info | readonly: 1
    [13] => server_version | readonly: 1
    [14] => sqlstate | readonly: 1
    [15] => protocol_version | readonly: 1
    [16] => thread_id | readonly: 1
    [17] => warning_count | readonly: 1
)

PHP Version

PHP 8.3.8

Operating System

No response

Activity

iluuu1994

iluuu1994 commented on Aug 9, 2024

@iluuu1994
Member

This is not bug, because these properties are not really readonly.

/** @readonly */
public string $client_info;

The @readonly is only added for documentation purposes. mysqli handles property writes through custom object handlers that predate the readonly keyword, and they have not been migrated over. We can treat this as a feature request though.

nielsdos

nielsdos commented on Aug 10, 2024

@nielsdos
Member

The way I see it is that it's more like a "get" hook without a "set" hook. Reason being that the extension can still change the value. So I think that isReadOnly should actually return false.

RV7PR

RV7PR commented on Aug 10, 2024

@RV7PR
Author

Well, internally that may be true but on the user side it's still readonly so I think it should return true

iluuu1994

iluuu1994 commented on Aug 10, 2024

@iluuu1994
Member

Readonly currently also means that the value never changes. If the value can change then readonly is not the correct flag to use.

cmb69

cmb69 commented on Aug 23, 2024

@cmb69
Member

The way I see it is that it's more like a "get" hook without a "set" hook. Reason being that the extension can still change the value. So I think that isReadOnly should actually return false.

So ReflectionProperty::getHooks() and friends should ideally be implemented; at least, gen_stubs.php (and PhD) should be improved to handle @property-read annotations. Is there already someone working on this (or is it planned)?

iluuu1994

iluuu1994 commented on Aug 23, 2024

@iluuu1994
Member

It gets a bit worse, since we can have a missing set operation (from the users pov) through various mechanisms:

  • readonly.
  • Hooks with no set, but only when the property is virtual.
  • Hooks implemented through object handlers. The set really just throws an exception.
  • Soon, private(set).

Maybe we should have something like: is{Readable,Writeable}(?string $fromScope = null). It's not completely clear whether the default should refer to the current scope, or private scope (i.e. the one used for {get,set}Value().

cmb69

cmb69 commented on Aug 23, 2024

@cmb69
Member

Okay, than leave this ticket for Reflection improvements/fixes, and have #15553 regarding the stubs.

added a commit that references this issue on Oct 3, 2024
47d23c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @iluuu1994@cmb69@nielsdos@RV7PR

        Issue actions

          ReflectionClass can't detect readonly properties for mysqli · Issue #15309 · php/php-src