Narrow down the return type of the Throwable::getTrace() method

[ste93cry]

As per title, with this PR I'm narrowing down the array shape returned by the Throwable::getTrace() method (and its children classes of course). At the beginning I though that it was enough to change the functionMap.php file, but the stub takes precedence so I had to edit both files

[staabm]

I like it.

would this also be usefull for debug_backtrace() ?

[ste93cry]

Done 😃 But I have a doubt, and I don't understand all the process to compute the backtrace found here: are the file and line keys always available or not?

[ste93cry]

Answer to myself: I've found this test case where the file and line keys are missing, so I'm changing both to be optional as well

function gen(): Generator
{
    yield 1;

    var_dump(debug_backtrace());
}

for ($gen = gen(); $gen->valid(); $gen->next()) {
    $gen->current();
}

https://3v4l.org/XOgoU

[ondrejmirtes]

Thanks! As you can see in the build failures, it might get annoying in some cases. Although technically true, a lot of existing code out there relies on the keys to always exist.

Do you have some real-world examples where the keys do not exist? Thanks!

[ste93cry]

As you can see in the build failures

Regarding the failures, I've already opened PRs to almost all involved projects to fix the problems so I expect the build to pass soon

a lot of existing code out there relies on the keys to always exist

I have the same feeling, however this is a clear error which will produce warnings/notices when accessing the undefined key, so I think it's appropriate for PHPStan to report it. Maybe this could be released in the next minor instead of a patch version to reduce the immediate impact of this change for all projects?

Do you have some real-world examples where the keys do not exist? Thanks!

Actually it's pretty easy to imagine them by just looking at the description of each key in the PHP docs. In any case, any well written code should take into account that most of the keys don't always exist and I expect that the majority of the issues will be around the fact that not everyone is aware that file and line are optional as well

[ondrejmirtes]

debug_backtrace() might be a bit different situation than Exception::getTrace(). You'd need to throw an exception that isn't thrown from inside a function and be able to catch it in order to dump the trace. Can you post an example on 3v4l.org that does that? It might be my weekend energy but I can't imagine it right now.

[ste93cry]

Here's an example of how I could get some stacktrace frame where the file and line keys are optional without explicitly throwing an exception myself (I think that these keys are the ones under scrutiny, since the rest are pretty obvious that are optional)

set_exception_handler(function (Throwable $exception) {
    var_dump($exception->getTrace());
});

class Foo
{
    private $foo;
    
    public function __toString()
    {
        return $this->foo;
    }
}

sprintf('%s', new Foo());

https://3v4l.org/lJ61X

At the moment I cannot came up with a test case with debug_backtrace() that specifically look up for the missing keys in a real-world example, but I strongly believe that it would not be PHPStan's fault to start complaining if the expectation is wrong

[ste93cry]

After some research, I found a test case in the PHP repo where debug_backtrace() returns a stacktrace without those keys set, and imho it somehow mimics a real-world example of what could happen inside an error handler. Of course the test case was not made for asserting whether or not certain keys are set, but its code is enough for our discussion

set_exception_handler(function () {
    ob_end_clean();
});

set_error_handler(function() {
    var_dump(debug_backtrace());

    throw new Exception();
});

$a['waa'];

https://3v4l.org/35MkN

[ondrejmirtes]

Okay, file and line are optional, but what about others? Can you get $e->getTrace() where function is missing?

And can you do it in a try-catch instead of an exception handler? I need the modification to be practical and not academical, so it's not worthy to set the key as optional if it can never happen in real world.

Can you also please link the PRs you opened here? Thanks.

[ste93cry]

what about others? Can you get $e->getTrace() where function is missing?

So, after looking again at the source code of PHP, I'm reasonably sure that there cannot be a stacktrace frame without the function key filled. For the other keys:

Key	Description
class	Missing when the frame comes from outside a method of a class
object	Present only when DEBUG_BACKTRACE_PROVIDE_OBJECT is used
type	Missing if the frame represents a function call
args	Missing when DEBUG_BACKTRACE_IGNORE_ARGS is used or when zend.exception_ignore_args = On

Can you also please link the PRs you opened here?

rectorphp/rector-src#1648, nextras/orm#552 and sebastianbergmann/phpunit#4857

[ste93cry]

@ondrejmirtes finally the CI passes and the PR is ready for review 🥳

[ondrejmirtes]

This was probably a larger effort than you anticipated :) Thank you very much. I'll /cc you when someone complains about the new signature :)