Distinguish between self, direct and indirect deprecations

[DominicLuidold]

Context

We've recently upgraded PHPUnit to version 10.5, up from 9.6, and also made the decision to remove the symfony/phpunit-bridge dev dependency. This choice was influenced by the insights from the "PHPUnit 10 for Symfony Developers" talk presented at SymfonyCon Brussels/SymfonyOnline, which indicated that the bridge is no longer necessary for reporting deprecations.

Our development involves a closed-source Symfony project for a client, and we've traditionally relied on the symfony/phpunit-bridge for deprecation reports. Specifically, we focus on identifying self and direct deprecations, such as instances where we call a method in our src directory from a third-party library in the vendor directory that will be removed in the next major version. This proactive approach helps us stay ahead during upgrades and the planning of our development tasks.

Previous approach using symfony/phpunit-bridge

With symfony/phpunit-bridge, distinguishing between self, direct, and indirect deprecations was possible, see here and here. We utilized this to make tests fail on self and especially direct deprecations while "ignoring" indirect deprecations, as they cannot directly be addressed by us.

Current possibilites

With PHPUnit 10.5, it is already possible to get reports on deprecations using the following configuration (inspired by slide #66 from the conference's talk).

<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.5/phpunit.xsd"
         bootstrap="tests/bootstrap.php"
         displayDetailsOnTestsThatTriggerDeprecations="true"
         failOnDeprecation="true">
    <!-- ... -->

    <source restrictDeprecations="true"
            ignoreSuppressionOfDeprecations="true">
        <include>
            <directory>src</directory>
            <file>vendor/symfony/deprecation-contracts/function.php</file>
        </include>
    </source>
</phpunit>

This will produce the following output when running tests that eventually trigger both direct and also indirect deprecations:

> XDEBUG_MODE=off vendor/bin/phpunit --testdox
PHPUnit 10.5.9 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.2
Configuration: /var/www/app/phpunit.xml.dist

DD                                                                  2 / 2 (100%)

Time: 00:00.294, Memory: 52.50 MB

Example (App\Tests\Functional\Example\Example)
 ⚠ Something
 ⚠ Something else

2 tests triggered 2 deprecations:

1) /var/www/app/vendor/symfony/deprecation-contracts/function.php:25
Since example/example 1.7.0: Direct deprecation triggered by calling a deprecated method located in vendor from the src directory

Triggered by:

* App\Tests\Functional\Example\ExampleTest::testSomething (3 times)
  /var/www/app/tests/Functional/Example/ExampleTest.php:14

* App\Tests\Functional\Example\ExampleTest::testSomethingElse (3 times)
  /var/www/app/tests/Functional/Example/ExampleTest.php:25

2) /var/www/app/vendor/symfony/deprecation-contracts/function.php:25
Since example2/example2 2.3.0: Indirect deprecation triggered by calling a deprecated method located in vendor library from a non-deprecated method located in another vendor library which is called by a method in the src directory

Triggered by:

* App\Tests\Functional\Example\ExampleTest::testSomething (3 times)
  /var/www/app/tests/Functional/Example/ExampleTest.php:14

* App\Tests\Functional\Example\ExampleTest::testSomethingElse (3 times)
  /var/www/app/tests/Functional/Example/ExampleTest.php:25

OK, but there were issues!
Tests: 2, Assertions: 8, Deprecations: 2.
Script XDEBUG_MODE=off vendor/bin/phpunit --testdox handling the test event returned with error code 1

Feature request

With PHPUnit's added functionalities througout 10.X, nearly all the functionalities of symfony/phpunit-bridge can be replaced. However, regardless of where the deprecation is triggered, it is reported as a deprecation and will, if configured, fail the respective test.

We propose adding the capability to distinguish whether a deprecation can be fixed by the project's maintainer (self & direct deprecations) or if a project is unable to do so because the deprecated code is called internally by a third-party library from another third-party library (indirect).

A possible configuration option could look like this:

<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.5/phpunit.xsd"
         bootstrap="tests/bootstrap.php"
+        reportDeprecations="self,direct"
         displayDetailsOnTestsThatTriggerDeprecations="true"
         failOnDeprecation="true">
    <!-- ... -->
</phpunit>

Big thanks to everyone who's been part of improving PHPUnit! As we suggest this feature, fingers crossed we haven't missed a config trick or an ongoing discussion about it. If there's more we can share — examples, insights, you name it — we're glad to assist. Thanks! 🚀

[sebastianbergmann]

Can you please elaborate what the differences between self, direct, and indirect deprecations are as well as why limiting the reporting of deprecations to only your code (excluding deprecations triggered by code in vendor) is not sufficient? And how can we detect whether an E_USER_DEPRECATED issue is self, direct, or indirect in the PHPUnit test runner's error handler? Thanks!

CC @stof @nicolas-grekas @derrabus

[stof]

self deprecations are deprecations triggered in your own code.
direct deprecations are deprecations triggered in vendor code with a caller in your own code (which is what happens when your own code calls a deprecated feature of some vendor code). Note that we have a special rule there where the trigger_deprecation function of symfony/deprecation-contracts is not considered in the call stack (instead, we use the caller of that wrapper function as the caller). And we might also have a special rule for the DebugClassLoader deprecations to consider the caller as being the place that triggers autoloading, but I'm not sure about that (maybe it is only an idea that we haven't implemented).
indirect deprecations are triggered in vendor code, called from other vendor code.

Excluding all deprecations triggered in vendor code would not report direct deprecations, which are still something that should be solved by the project.
The 3 categories of deprecations have different meaning regarding solving them:

• self: your own code triggers deprecated APIs of your own code. This should be avoided as it means that your users will never be able to do anything about them. this is mostly useful for libraries (as final projects are unlikely to implement their own deprecations in most cases). We want that to be reported in CI.
• direct: you need to fix those to prepare for compat with the next version of your dependencies. Many packages will try to be deprecation-free to avoid users of the package to see such deprecations. Those packages will want to report them in the CI
• indirect: you cannot do anything about them (assuming they are well classified, which is not always the case when a loader for a config file triggers a deprecation due to the content of a Yaml file being loaded, as the deprecation won't be triggered in the Yaml file itself that belongs to the project). You care about them once you actually want to migrate to the next major version of some package. But the fixes have to be done in your other dependencies. You often don't want them to be reported in CI.

[stof]

See https://github.com/symfony/phpunit-bridge/blob/6.3/DeprecationErrorHandler/Deprecation.php for the logic. $trace is the result of debug_backtrace() in the error handler

[sebastianbergmann]

Thank you, @stof, for explaining this.

[dkarlovi]

If implemented, @stof comment above can be used in the docs about the feature directly, nice!

[sebastianbergmann]

I am confident that this can and will be implemented.

[sebastianbergmann]

The 3 categories of deprecations have different meaning regarding solving them:

Sorry to bother you again, @stof, but I need to rephrase your explanation of self, direct, and indirect in my words to make sure that I understood you properly:

• self: your own code triggers an issue in your own code
• direct: your own code triggers an issue in third-party code
• indirect: third-party code triggers an issue either in your own code or in third-party code

In the above, your own code means code that is configured using <source> in PHPUnit's XML configuration and third-party code means any code that is not configured using <source>.

[sebastianbergmann]

Here is my first attempt at implementig the detection of the above:

phpunit/src/Runner/ErrorHandler.php

Lines 234 to 267 in ffc0078

	private function trigger(TestMethod $test): IssueTrigger
	{
	if (!$this->source->notEmpty()) {
	return IssueTrigger::unknown();
	}
	$trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 4);
	assert(isset($trace[2]['file']));
	assert(isset($trace[3]['file']));
	$triggeredInFirstPartyCode = false;
	$triggerCalledFromFirstPartyCode = false;
	if ($trace[2]['file'] === $test->file() ||
	$this->sourceFilter->includes($this->source, $trace[2]['file'])) {
	$triggeredInFirstPartyCode = true;
	}
	if ($trace[3]['file'] === $test->file() ||
	$this->sourceFilter->includes($this->source, $trace[3]['file'])) {
	$triggerCalledFromFirstPartyCode = true;
	}
	if ($triggerCalledFromFirstPartyCode) {
	if ($triggeredInFirstPartyCode) {
	return IssueTrigger::self();
	}
	return IssueTrigger::direct();
	}
	return IssueTrigger::indirect();
	}

[sebastianbergmann]

@stof I suspect that we need to make the stack frames we look at, 1 and 2 in the above, configurable to accommodate for indirections such as trigger_deprecation(). Correct?

[sebastianbergmann]

I think we need an optional callback to determine which stack frames need to be considered (2 and 3 in the code shown in #5689 (comment)).

[sebastianbergmann]

I think we need a configurable list of functions/methods for which stack frames should be skipped.

[sebastianbergmann]

This is the event log used in an end-to-end test for the current implementation:

phpunit/tests/end-to-end/event/user-deprecated-self-direct-indirect.phpt

Lines 22 to 54 in d0ad5b6

	PHPUnit Started (PHPUnit %s using %s)
	Test Runner Configured
	Bootstrap Finished (%sautoload.php)
	Test Suite Loaded (2 tests)
	Event Facade Sealed
	Test Runner Started
	Test Suite Sorted
	Test Runner Execution Started (2 tests)
	Test Suite Started (%sphpunit.xml, 2 tests)
	Test Suite Started (default, 2 tests)
	Test Suite Started (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest, 2 tests)
	Test Preparation Started (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testOne)
	Test Prepared (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testOne)
	Test Triggered Deprecation (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testOne, issue triggered by first-party code calling into third-party code, suppressed using operator)
	deprecation in third-party code
	Test Triggered Deprecation (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testOne, issue triggered by first-party code calling into first-party code, suppressed using operator)
	deprecation in first-party code
	Test Passed (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testOne)
	Test Finished (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testOne)
	Test Preparation Started (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testTwo)
	Test Prepared (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testTwo)
	Test Triggered Deprecation (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testTwo, issue triggered by first-party code calling into third-party code, suppressed using operator)
	deprecation in third-party code
	Test Triggered Deprecation (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testTwo, issue triggered by third-party code, suppressed using operator)
	deprecation in first-party code
	Test Passed (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testTwo)
	Test Finished (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest::testTwo)
	Test Suite Finished (PHPUnit\TestFixture\SelfDirectIndirect\FirstPartyClassTest, 2 tests)
	Test Suite Finished (default, 2 tests)
	Test Suite Finished (%sphpunit.xml, 2 tests)
	Test Runner Execution Finished
	Test Runner Finished
	PHPUnit Finished (Shell Exit Code: 0)

[derrabus]

I think we need a configurable list of functions/methods for which stack frames should be skipped.

The idea is that if a library uses a package like symfony/deprecation-contracts or doctrine/depreactions to trigger runtime deprecations, the usage of such a library is not falsely detected as indirect deprecation?

Symfony's error handler has a cleanTrace() method that kicks trigger_deprecation() out of the stack trace:

https://github.com/symfony/symfony/blob/099daf5a83dde73d1341c6555dea124fd8011009/src/Symfony/Component/ErrorHandler/ErrorHandler.php#L697-L734

Do you think, PHPUnit should ship a list of well-known deprecation functions that can be extended via phpunit.xml? Or should this list be empty by default, e.g. every project that uses any Symfony component has to configure trigger_deprecation() into that list?

[sebastianbergmann]

I think the list of deprecation functions should be empty by default and configurable through PHPUnit's XML configuration file.

[dkarlovi]

Empty by default sounds good, assuming it's possible for plugins to populate it? That way a Symfony plugin could populate the list with "well known deprecations in the Symfony world", Laravel could do the same for itself, etc. PHPUnit doesn't need to know the names.

[sebastianbergmann]

The XML configuration file should be the only means of configuring this. I have no plan to allow populating this list through plugins.

AFAIK, Symfony, for example, creates a PHPUnit XML configuration file already for new projects. Adding the relevant configuration to that should be trivial.

[sebastianbergmann]

Configuring deprecation triggering functions/methods is now implemented. Here is an example from the test suite:

• https://github.com/sebastianbergmann/phpunit/blob/10ac9a7d6ee68a70d7f4be6f98ca43e4ca12d5a0/tests/end-to-end/self-direct-indirect/deprecation-trigger-function.phpt
• https://github.com/sebastianbergmann/phpunit/tree/10ac9a7d6ee68a70d7f4be6f98ca43e4ca12d5a0/tests/end-to-end/self-direct-indirect/_files/deprecation-trigger-function

phpunit/tests/end-to-end/self-direct-indirect/_files/deprecation-trigger-function/phpunit.xml

Lines 17 to 19 in 10ac9a7

	<deprecationTrigger>
	<function>PHPUnit\TestFixture\SelfDirectIndirect\trigger_deprecation</function>
	</deprecationTrigger>

[sebastianbergmann]

The "only" thing left to implement is using the information whether a deprecation is self, direct, or indirect for selecting which deprecations to report.

Since PHPUnit 10, we have the restrictDeprecations attribute on the <source> element of PHPUnit's XML configuration file. Currently, this attribute allows true (only report deprecations triggered in your own code) and false (default; report all deprecations (except baselined, ...)).

I think we should deprecate the restrictDeprecations attribute in PHPUnit 11.1 and remove it in PHPUnit 12.

The new functionality that is the topic of this issue should be controlled through these new attributes:

• ignoreSelfDeprecations: boolean, default false
• ignoreDirectDeprecations: boolean, default false
• ignoreIndirectDeprecations: boolean, default true

Does that make sense?

[sebastianbergmann]

To avoid migration pain, the default value for all three attributes, ignoreSelfDeprecations, ignoreDirectDeprecations, and ignoreIndirectDeprecations, will be false.

The XML configuration file with best practice defaults that can be generated using --generate-configuration now has ignoreIndirectDeprecations="true" instead of restrictDeprecations="true".

The XML configuration file migrator replaces restrictDeprecations="true" with ignoreIndirectDeprecations="true".

[sebastianbergmann]

This has now been merged into main for PHPUnit 11.1.

A huge "Thank you!" to everyone who provided input and contributed feedback along the way.

[DominicLuidold]

@sebastianbergmann A huge thanks back for implementing this feature!