Fix access of undefined array index file when internally analyzing stack frames

[reinfi]

Q	A
Documentation	no
Bugfix	yes
BC Break	no
New Feature	no
RFC	no
QA	no

Description

When triggering events from within a controller plugin in laminas mvc the index is not a file because it is an invokable function.

4 = {array} [5] function = "__invoke" class = "Application\Controller\Plugin\ForwardToPlugin" object = {Application\Controller\Plugin\ForwardToPlugin} [3] type = "->" args = {array} [2]

I just checked if the index exists and return an empty string.

[Ocramius]

Needs test additions

[reinfi]

Needs test additions

Yeah would love to add a test but it is quite difficult because there are no existing tests for the class and mocking the native buildin functions is quite hard.

Any suggestions how to start this?

[Ocramius]

I think a backtrace is not available when the code is either internal, or eval()'d?

A test could do something like:

$provider = new EventContextProvider();

$file = eval(<<<'PHP'
return (function (): void {
    return $provider->getEventTriggerFile();
})();
PHP
);

self::assertSame('', $file);

Alternatively using array_map() could perhaps get the same result.

[reinfi]

I added a crazy testcase for this specific case 🙈

[Ocramius]

Test case makes sense, now we need to make CI green :D

Also: 100% sure this makes PHPUnit fail when your patch is not applied?

[samsonasik]

Target branch need to be changed to 2.7.x or 2.8.x and rebase.

[froschdesign]

Target branch need to be changed to 2.7.x or 2.8.x and rebase.

A bugfix goes to the current release branch (2.7.x):

What branch to issue the pull request against?

Which branch should you issue a pull request against?

• For fixes against the stable release, issue the pull request against the release branch matching the minor release you want to fix.
• For new features, or fixes that introduce new elements to the public API (such as new public methods or properties), issue the pull request against the default branch.

https://github.com/laminas/.github/blob/a1debce9a0842e3000ae9e01fbc60e32db62901d/CONTRIBUTING.md#what-branch-to-issue-the-pull-request-against

[reinfi]

Test case makes sense, now we need to make CI green :D

Also: 100% sure this makes PHPUnit fail when your patch is not applied?

Undefined array key "file" /Users/maddin/sites/laminas-developer-tools/src/EventLogging/EventContextProvider.php:124

Happens if I remove my patch.

Target branch need to be changed to 2.7.x or 2.8.x and rebase.

A bugfix goes to the current release branch (2.7.x):

What branch to issue the pull request against?

Which branch should you issue a pull request against?

• For fixes against the stable release, issue the pull request against the release branch matching the minor release you want to fix.
• For new features, or fixes that introduce new elements to the public API (such as new public methods or properties), issue the pull request against the default branch.

https://github.com/laminas/.github/blob/a1debce9a0842e3000ae9e01fbc60e32db62901d/CONTRIBUTING.md#what-branch-to-issue-the-pull-request-against

Will try to solve this as best as possible :-D

[reinfi]

If I try to fix psalm and add use function call_user_func; then the test case fails again because there is no more a inline call for whatever reason.

I think I'll just close this cause creating a test case for it is quite annoying 🙈

[reinfi]

Okay I think I'll not be able to solve the psalm errors without breaking the test case again. I'll get some sleep and may be next week I have a solution for the testcase 👼

[Ocramius]

Hmm, something went wrong with the rebase here - I can take over, if it's not clear :)

[reinfi]

was too late yesterday 🙈 Would be happy if you help cause I got somewhere stuck with the rebase, tought it was right but it seems not.

[Ocramius]

Currently checking this locally: interestingly, if use function call_user_func; is used, then PHP optimizes the call away, and the test no longer works as expected :D

[Ocramius]

Rewriting some of the test to use array_map() fixes that though 👍

[Ocramius]

I rewrote most of the patch, and GIT somehow lostsome ownership of the patch here, sorry :-(

Still, final result is much cleaner, I'd say.

[Ocramius]

Thanks @reinfi!

[froschdesign]

@reinfi
Thank you for your time and this contribution! And also for your library reinfi/zf-dependency-injection 👍🏻

[reinfi]

thank you guys <3