search

phpspec / prophecy

Highly opinionated mocking framework for PHP 5.3+
MIT License
8.53k stars 241 forks source link

Hotfix: case insensitive methods #442

Closed michalbundyra closed 4 years ago

michalbundyra commented 5 years ago

Fixes #165

Prophecy was case sensitive for methods, but PHP is not.

Because of that we have an issue in https://github.com/zendframework/zend-expressive-swoole/pull/70 as method name in the extension has been changed from rawcontent to rawContent.

stof commented 5 years ago

should we rather store MethodProphecy and Call objects with a lowercased method name, to avoid lowercasing them on each access (and changing getMethodProphecies from O(1) to O(n)) ?

michalbundyra commented 5 years ago

@stof This was my first approach, but then we need to change it in many places and the final messages are not going to be that clear as before. I would need also modify tests when I start storing lowercase method names.

In messages we are gonna see not what we have provided but lowercase method, for example:

expected calls were:
- findpropecymethodcalls(...)

instead of:

- findProphecyMethodCalls(...)

that's why I've implemented "less efficient" solution.

I can push the alternative solution to the other branch so you can have a look and decide.

stof commented 5 years ago

Well, we could have MethodProphecy and Call store the original case internally, usable in messages, but having ObjectProphecy::$methodProphecies be indexed by lowercase name for instance (to keep the O(1) access.

michalbundyra commented 5 years ago

@stof Updated now. Technically it's BC Break as getMethodProphecies gives different result now than before. I guess you've expected that. That's why I've modified some tests.

ciaranmcnulty commented 5 years ago

Does enabling case-insensitivity fit with our 'don't allow bad practices' philosophy?

michalbundyra commented 5 years ago

@ciaranmcnulty This is PHP internals and because of that it's really hard to write tests with swoole extension (across different versions).

ciaranmcnulty commented 4 years ago

Thanks, in retrospect I think it's good to mirror PHP's behaviour (although developers should be calling methods using the correct case)