Fix #74750 - Look for user code before checking for strict_types

[pmmaga]

Link for bugsnet: https://bugs.php.net/bug.php?id=74750

This fix allows the indirect check of strict_types skipping calls that are not user code. This way, calls made via a callback or reflection will check the place where the code was actually called by the user instead of checking the internal function that delegates the call.

This also fixes https://bugs.php.net/bug.php?id=75345 which covers the case of Reflection.

[jhdxr]

I like this. But I think it need a RFC.

[sebastianbergmann]

Why would this need an RFC? It's a bugfix.

[jhdxr]

@sebastianbergmann As cmb pointed out in the bug report, the behavior now is expected.

let's say we have two files. file1 is in strict mode while file2 not.

declare(strict_types=1);
//file1
function foo($foo) {
  bar($foo);
}
foo(123);

//file2
function bar($any){
	bar2($any);
}

function bar2(string $str) {
}

this won't trigger any error, because call to bar2 is in a file without strict mode. and so does array_map

[sebastianbergmann]

The behavior I described in https://bugs.php.net/bug.php?id=75345 is totally unexpected, IMO.

[sebastianbergmann]

@sgolemon @remicollet What do you think? I would love to see https://bugs.php.net/bug.php?id=75345 fixed in PHP 7.2.

[pmmaga]

@jhdxr while it is true that the strictness depends on the file that is making the call, currently there is no way for the user to specify the behavior of internal functions. Given that, I believe that it is more sane to look for user code to find the user intention rather than assuming something about code that the user has no control over.

[nikic]

Regardless of whether one considers this a bug, it's certainly a non-trivial backwards compatibility break, and as such warrants a discussion on the internals list.

I'm not sure whether this is really fixing the right issue. This kind-of works with internal calls, because they tend to be rather directly caused by a userland call, like in the case of the invokeArgs() call. However, this is not generally true. For example, an error handler may be called from pretty much anywhere, and whether that code happens to use strict or weak mode is anyone's guess.

To me this seems more related to the general issue of callback invocation, which also exists in userland. I've seen quite a few people express the expectation that if people define a closure in a non-strict file and it happens to be invoked in a strict file, it should still use non-strict semantics (and vice versa).

[jhdxr]

@pmmaga

currently there is no way for the user to specify the behavior of internal functions.

True, and it's why I say I like it. However, this PR is changing an expected behavior, and it nees a RFC.

Back to the problem itself, before we reach an agreement on dealing with the strictnss level of callbacks, I think we can propose something like global strict mode.

[pmmaga]

Link for discussion on internals: https://externals.io/message/100851

Generally, the idea is that the fix should not be based on what is user code or not but whether the call is done directly or dynamically.

About 75345, it's probably better to special-case the reflection API for now until we have a determined way of dealing with strictness on internal functions.