On Tue, Oct 10, 2017 at 3:41 PM, Sebastian Bergmann <sebast...@php.net> wrote:
> https://bugs.php.net/bug.php?id=75345 is about the fact that strict type > declarations are not enforced when a function or method is invoked via the > Reflection API. > > There is a pull request that addresses this (as well as > https://bugs.php.net/bug.php?id=74750) at > https://github.com/php/php-src/pull/2837. > > I consider this a serious bug that leads to unexpected, confusing problems > such as > https://github.com/sebastianbergmann/phpunit/issues/2796#issuecomment- > 335180273. > > I understand Nikita's point of view (see > https://github.com/php/php-src/pull/2837#issuecomment-335405067) that > changing this behavior (aka. fixing this bug) can be considered a > "non-trivial backwards compatibility break". Therefore I would like to > bring this issue to the attention of this list with this mail. > To repeat what I said in a comment on the pull request: I think this is fixing the wrong issue. The problem are not internal function calls, the problem are callbacks. In fact, the proposed fix does not actually fix the problem you encountered in PHPUnit, as it is going to use the strictness mode at the reflection call-site, not the strictness mode used by the file defining the data provider. The proposed patch is going to add a discrepancy between internal and userland code. It means that the internal array_map function will execute the callback using the strictness mode of the array_map caller, while a userland reimplementation of the same function will not be able to match that behavior and always either perform the call strictly or weakly, independent of the array_map caller. I believe that the proper way to fix this is to handle dynamic function invocations differently from direct invocations. Direct invocations should use the strictness level of the call-site, while dynamic invocations should use the strictness level of the declaration-site. This means that the internal array_map and a userland reimplementation will behave consistent. It also means that PHPUnit will be able to respect the strictness level of file defining the data provider, etc. I've seen a number of complaints about our handling of callbacks wrt strict_types, so I think it's worthwhile to at least consider making such a change. Regards, Nikita
