Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code inside eval triggers deprecation, results in ExcludeList::isExcluded(): Argument #1 ($file) must be of type string, null given #6043

Closed
ruudk opened this issue Nov 25, 2024 · 5 comments
Labels
type/bug Something is broken

Comments

@ruudk
Copy link
Contributor

ruudk commented Nov 25, 2024

Q A
PHPUnit version 11.5-dev
PHP version 8.4
Installation Method Composer

Summary

When eval'd code triggers a deprecation, the following error happens:

PHPUnit\Util\ExcludeList::isExcluded(): Argument #1 ($file) must be of type string, null given, called in /Volumes/CS/opensource/Twig/vendor/phpunit/phpunit/src/Runner/ErrorHandler.php on line 403")

Current behavior

This was introduced in 5d6dc50#diff-c59b939cd44edcda5d8a71c38f90461826dc2abbb5267f343fd7db0fa8fe1784

if ($excludeList->isExcluded($frame['file'])) {
continue;
}

Here, it should handle the fact that file cannot be there.

How to reproduce

PR where the issue is visible: twigphp/Twig#4472

When running the debugger, you can see that file is not part of the stack trace frame when the code is eval'd.

Screenshot 2024-11-25 at 09 52 26@2x

Expected behavior

No error.

@sebastianbergmann
Copy link
Owner

Thank you for your report.

Please provide a minimal, self-contained, reproducing test case that shows the problem you are reporting.

Without such a minimal, self-contained, reproducing test case I will not be able to investigate this issue.

@sebastianbergmann sebastianbergmann added the status/waiting-for-feedback Waiting for feedback from original reporter label Nov 25, 2024
@ruudk
Copy link
Contributor Author

ruudk commented Nov 25, 2024

I'm sorry, but I'm having a really hard time coming up with a minimal, self-contained, reproducing test case.

Any advice?

@ruudk
Copy link
Contributor Author

ruudk commented Nov 25, 2024

I am able to create a test that triggers a deprecation. But I cannot mimic the scenario that happens with Twig. Twig uses yield and generators on an eval'd template.

But even with this, I always receive the file:

final class Test extends TestCase
{
    public static function testEval(): void
    {
        $code = <<<'PHP'
function main() {
    yield fn() => \PHPUnit\TestFixture\Bug6043\trigger_deprecation('deprecation triggered from within eval', E_USER_DEPRECATED);
}
foreach (main() as $item) {
    echo $item();
}
PHP;

        eval($code);
    }
}

@sebastianbergmann
Copy link
Owner

The example given in #6043 (comment) is not self-contained, as it uses a function named \PHPUnit\TestFixture\Bug6043\trigger_deprecation which is not provided.

I have changed the example to make it run like so:

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

final class Test extends TestCase
{
    public function testEval(): void
    {
        $code = <<<'PHP'
function main() {
    yield fn() => trigger_error('message', E_USER_DEPRECATED);
}

foreach (main() as $item) {
    $item();
}
PHP;

        eval($code);

        $this->assertTrue(true);
    }
}

With the above, I get

❯ ./phpunit Test.php --display-deprecations
PHPUnit 11.5-g7e4dffb7d7 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.4.1
Configuration: /usr/local/src/phpunit/phpunit.xml

D                                                                   1 / 1 (100%)

Time: 00:00.028, Memory: 12.00 MB

1 test triggered 1 deprecation:

1) /usr/local/src/phpunit/Test.php(18) : eval()'d code:2
message

Triggered by:

* Test::testEval
  /usr/local/src/phpunit/Test.php:6

OK, but there were issues!
Tests: 1, Assertions: 1, Deprecations: 1.

This looks correct to me, but you mentioned that "even with this, [you] always receive the file", so that is probably expected and what you meant.

I implemented a safe guard to not use the file index of a stack frame array when it is not available in 0009ef0.

Please let me know whether this solves the issue for you. Thanks!

@sebastianbergmann sebastianbergmann removed the status/waiting-for-feedback Waiting for feedback from original reporter label Nov 25, 2024
@ruudk
Copy link
Contributor Author

ruudk commented Nov 25, 2024

I just ran the Twig suite on latest phpunit main again and can confirm the problem is gone now.

Thanks for the fast fix and pointing out the problem that I was using the dev dependency without knowing it.

@ruudk ruudk closed this as completed Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

2 participants