Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot mock methods begining with underscore #108

Open
dantleech opened this issue Jun 16, 2014 · 10 comments · May be fixed by #553
Open

Cannot mock methods begining with underscore #108

dantleech opened this issue Jun 16, 2014 · 10 comments · May be fixed by #553
Labels

Comments

@dantleech
Copy link
Contributor

See: https://github.com/phpspec/prophecy/blob/master/src/Prophecy/Doubler/Generator/ClassMirror.php#L106

What is the reason for this?

@dantleech dantleech changed the title Cannot mock methods beining with underscore Cannot mock methods begining with underscore Jun 16, 2014
@stof
Copy link
Member

stof commented Jun 16, 2014

hmm, this is indeed weird. @everzet can you explain your choice there ?

@everzet
Copy link
Member

everzet commented Nov 29, 2014

@stof @dantleech I never seen methods starting from _ treated as public APIs by anyone. In all cases I've seen _ at the beginning of method name means it is part of the private API. Hence Prophecy ignores these methods.

@dantleech
Copy link
Contributor Author

@everzet I cannot remember now the use case, I do not program APIs with underscores. But it is perfectly valid - should prophecy really assume that nobody will ever need to do this?

@frankdejonge
Copy link

Ok, so the SoapClient has a __soapCall method, which is public, but due to this it can't be mocked. I think since 2016 is almost over people defining visibility with leading underscores just have to deal with the fact that tooling doesn't work with their conventions anymore, perhaps?

@boboudreau
Copy link

PECL RedisCluster::_masters() also is "public", in their documentation, but cannot be mocked. Don't mock what you don't own, I know. I'm building an adapter for it now.

@barryosull
Copy link

barryosull commented Jul 12, 2018

If this isn't going to be updated, how about we make it report an error when you try to mock a method beginning with an underscore? Or when you try to call one? It took me ages to figure out that Prophecy was just ignoring those definitions, an error would at least tell me what's happening.

@psincraian
Copy link

As @frankdejonge mentions the SoapClient has public methods starting with __, like __soapCall or __getLastResponse. This methods can't be mocked 👎

neclimdul added a commit to neclimdul/prophecy that referenced this issue Jun 10, 2022
Fixes phpspec#108

While it was a common convention to prefix "private" methods with an underscore
in earlier version of php, with support for method visibility this is no longer
common.

Additionally there are a number of exception such as the Soap library where
this convention did not hold and code was not mockable.
@neclimdul
Copy link
Contributor

well... dangit. I was using phpspec to tests a soap library. Specifically handing specific logging behaviors based on values stored in a SoapClient. Now I'm up a creek because SoapClient doesn't have an interface for settings values. Short term is the solution to use mockery or does someone have a workaround?

Its clearly documented there are exceptions inside PHP. Also everyone can just use native visibility if they really want something to be private. I was a good argument 6 years ago and its even better now. I can't remember the last time I've seen this convention used and I think we can let it die.

I'm going to optimistically open a PR. I hope we can change this so random people in the future don't run into this weird WTF an waste time on it.

@neclimdul neclimdul linked a pull request Jun 10, 2022 that will close this issue
@Grundik
Copy link

Grundik commented May 16, 2024

Аhead of the 10th anniversary of this issue, I'll humbly remind: it still actual. Moreover, since version 1.17, its no longer possible to use reflections as workarounds, since whitelisted method names are now constant, not property. But version 1.16 (last one, when whitelist overriding was possible) does not allows usage of PHP 8.3+, so this issue is starting to pressure.

neclimdul added a commit to neclimdul/prophecy that referenced this issue May 16, 2024
Fixes phpspec#108

While it was a common convention to prefix "private" methods with an underscore
in earlier version of php, with support for method visibility this is no longer
common.

Additionally there are a number of exception such as the Soap library where
this convention did not hold and code was not mockable.
@neclimdul
Copy link
Contributor

Thanks for the reminder. Looks like the merge request was out of date so I rebased. didn't get more then a passing review but maybe a bump on this issue will get some attention again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants