Skip to content

Conversation

camporter
Copy link
Contributor

Support constructing a ReflectionMethod from an object that has a get_method object handler that can be used to reflect methods that aren't in the class entry, such as PDO driver-specific methods.

Some downsides are that ReflectionClass::getMethods() used with a PDO object of a specific driver type still won't list the driver-specific methods. I'm also not sure this can be supported on ReflectionClass::getMethod().

This is just an experiment for trying to solve issues like reported in bug 78126.
Not using get_method to add driver specific methods (add them to the PDO class entry directly?) would be another way to solve the issue, but that's obviously a much larger change and has side effects.

Support constructing a ReflectionMethod from an object that has a get_method handler that can be used to reflect methods that aren't in the class entry, such as PDO driver-specific methods.
@camporter camporter force-pushed the use_get_method_for_reflectionmethod branch from 36d6902 to 9e7c5f5 Compare June 16, 2019 03:40
@morozov
Copy link
Contributor

morozov commented Jun 22, 2020

@nikic, any chance to get this reviewed? This inconsistency causes issues during static analysis of the code that uses this API (e.g. phpstan/phpstan#3527).

// a get_method object handler exists, so try to get the method from it
zend_string *method_string = zend_string_init(lcname, name_len, 0);

mptr = obj_handlers->get_method(&Z_OBJ_P(classname), method_string, NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this throw for inaccessible methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't touched this in a while, zend_std_get_method would? What would be the best way to reconcile either letting get_method handle throwing an error or otherwise throwing the exception below?

I might be misunderstanding though 🙂

@ramsey
Copy link
Member

ramsey commented May 27, 2022

@camporter What's the status of this PR?

@camporter
Copy link
Contributor Author

I'm not sure this is feasible, or at least I'm not familiar with how to make it feasible.
The driver-specific methods being called on the PDO class doesn't mesh well with reflection.

Can go ahead and close the PR.

@ramsey
Copy link
Member

ramsey commented May 28, 2022

Thanks!

@ramsey ramsey closed this May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants