diff --git a/README.md b/README.md index a7b1002..d668222 100644 --- a/README.md +++ b/README.md @@ -1,52 +1,63 @@ # Doctrine Deprecations -A small layer on top of `trigger_error(E_USER_DEPRECATED)` or PSR-3 logging -with options to disable all deprecations or selectively for packages. +A small (side-effect free by default) layer on top of +`trigger_error(E_USER_DEPRECATED)` or PSR-3 logging. -By default it does not log deprecations at runtime and needs to be configured -to log through either trigger_error or with a PSR-3 logger. This is done to -avoid side effects by deprecations on user error handlers that Doctrine has no -control over. +- no side-effects by default, making it a perfect fit for libraries that don't know how the error handler works they operate under +- options to avoid having to rely on error handlers global state by using PSR-3 logging +- deduplicate deprecation messages to avoid excessive triggering and reduce overhead + +We recommend to collect Deprecations using a PSR logger instead of relying on +the global error handler. ## Usage from consumer perspective: -Enable or Disable Doctrine deprecations to be sent as `trigger_error(E_USER_DEPRECATED)` +Enable Doctrine deprecations to be sent to a PSR3 logger: + +```php +\Doctrine\Deprecations\Deprecation::enableWithPsrLogger($logger); +``` + +Enable Doctrine deprecations to be sent as `@trigger_error($message, E_USER_DEPRECATED)` messages. ```php \Doctrine\Deprecations\Deprecation::enableWithTriggerError(); -\Doctrine\Deprecations\Deprecation::enableWithSuppressedTriggerError(); -\Doctrine\Deprecations\Deprecation::disable(); ``` -Enable Doctrine deprecations to be sent to a PSR3 logger: +If you only want to enable deprecation tracking, without logging or calling `trigger_error` then call: ```php -\Doctrine\Deprecations\Deprecation::enableWithPsrLogger($logger); +\Doctrine\Deprecations\Deprecation::enableTrackingDeprecations(); ``` -Disable deprecations from a package +Tracking is enabled with all three modes and provides access to all triggered +deprecations and their individual count: ```php -\Doctrine\Deprecations\Deprecation::ignorePackage("doctrine/orm"); +$deprecations = \Doctrine\Deprecations\Deprecation::getTriggeredDeprecations(); + +foreach ($deprecations as $identifier => $count) { + echo $identifier . " was triggered " . $count . " times\n"; +} ``` +### Suppressing Specific Deprecations + Disable triggering about specific deprecations: ```php \Doctrine\Deprecations\Deprecation::ignoreDeprecations("https://link/to/deprecations-description-identifier"); ``` -Access is provided to all triggered deprecations and their individual count: +Disable all deprecations from a package ```php -$deprecations = \Doctrine\Deprecations\Deprecation::getTriggeredDeprecations(); - -foreach ($deprecations as $identifier => $count) { - echo $identifier . " was triggered " . $count . " times\n"; -} +\Doctrine\Deprecations\Deprecation::ignorePackage("doctrine/orm"); ``` +### Other Operations + When used within PHPUnit or other tools that could collect multiple instances of the same deprecations the deduplication can be disabled: @@ -54,6 +65,12 @@ the deduplication can be disabled: \Doctrine\Deprecations\Deprecation::withoutDeduplication(); ``` +Disable deprecation tracking again: + +```php +\Doctrine\Deprecations\Deprecation::disable(); +``` + ## Usage from a library/producer perspective: When you want to unconditionally trigger a deprecation even when called @@ -93,7 +110,7 @@ then use: ``` Based on the issue link each deprecation message is only triggered once per -request, so it must be unique for each deprecation. +request. A limited stacktrace is included in the deprecation message to find the offending location. diff --git a/lib/Doctrine/Deprecations/Deprecation.php b/lib/Doctrine/Deprecations/Deprecation.php index 10e55bb..4375889 100644 --- a/lib/Doctrine/Deprecations/Deprecation.php +++ b/lib/Doctrine/Deprecations/Deprecation.php @@ -8,41 +8,43 @@ use function array_key_exists; use function array_reduce; -use function basename; use function debug_backtrace; use function sprintf; use function strpos; +use function strrpos; +use function substr; use function trigger_error; use const DEBUG_BACKTRACE_IGNORE_ARGS; +use const DIRECTORY_SEPARATOR; use const E_USER_DEPRECATED; /** * Manages Deprecation logging in different ways. * - * By default triggered exceptions are not logged, only the amount of - * deprecations triggered can be queried with `Deprecation::getUniqueTriggeredDeprecationsCount()`. + * By default triggered exceptions are not logged. * * To enable different deprecation logging mechanisms you can call the * following methods: * - * - Uses trigger_error with E_USER_DEPRECATED - * \Doctrine\Deprecations\Deprecation::enableWithTriggerError(); + * - Minimal collection of deprecations via getTriggeredDeprecations() + * \Doctrine\Deprecations\Deprecation::enableTrackingDeprecations(); * * - Uses @trigger_error with E_USER_DEPRECATED - * \Doctrine\Deprecations\Deprecation::enableWithSuppressedTriggerError(); + * \Doctrine\Deprecations\Deprecation::enableWithTriggerError(); * * - Sends deprecation messages via a PSR-3 logger * \Doctrine\Deprecations\Deprecation::enableWithPsrLogger($logger); * - * Packages that trigger deprecations should use the `trigger()` method. + * Packages that trigger deprecations should use the `trigger()` or + * `triggerIfCalledFromOutside()` methods. */ class Deprecation { - private const TYPE_NONE = 0; - private const TYPE_TRIGGER_ERROR = 1; - private const TYPE_TRIGGER_SUPPRESSED_ERROR = 2; - private const TYPE_PSR_LOGGER = 3; + private const TYPE_NONE = 0; + private const TYPE_TRACK_DEPRECATIONS = 1; + private const TYPE_TRIGGER_ERROR = 2; + private const TYPE_PSR_LOGGER = 4; /** @var int */ private static $type = self::TYPE_NONE; @@ -60,7 +62,7 @@ class Deprecation private static $deduplication = true; /** - * Trigger a deprecation for the given package, starting with given version. + * Trigger a deprecation for the given package and identfier. * * The link should point to a Github issue or Wiki entry detailing the * deprecation. It is additionally used to de-duplicate the trigger of the @@ -70,6 +72,10 @@ class Deprecation */ public static function trigger(string $package, string $link, string $message, ...$args): void { + if (self::$type === self::TYPE_NONE) { + return; + } + if (array_key_exists($link, self::$ignoredLinks)) { self::$ignoredLinks[$link]++; } else { @@ -80,12 +86,6 @@ public static function trigger(string $package, string $link, string $message, . return; } - // do not move this condition to the top, because we still want to - // count occcurences of deprecations even when we are not logging them. - if (self::$type === self::TYPE_NONE) { - return; - } - if (isset(self::$ignoredPackages[$package])) { return; } @@ -98,16 +98,32 @@ public static function trigger(string $package, string $link, string $message, . } /** + * Trigger a deprecation for the given package and identifier when called from outside. + * + * "Outside" means we assume that $package is currently installed as a + * dependency and the caller is not a file in that package. When $package + * is installed as a root package then deprecations triggered from the + * tests folder are also considered "outside". + * + * This deprecation method assumes that you are using Composer to install + * the dependency and are using the default /vendor/ folder and not a + * Composer plugin to change the install location. The assumption is also + * that $package is the exact composer packge name. + * + * Compared to {@link trigger()} this method causes some overhead when + * deprecation tracking is enabled even during deduplication, because it + * needs to call {@link debug_backtrace()} + * * @param mixed $args */ public static function triggerIfCalledFromOutside(string $package, string $link, string $message, ...$args): void { + if (self::$type === self::TYPE_NONE) { + return; + } + $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2); - // "outside" means we assume that $package is currently installed as a - // dependency and the caller is not a file in that package. - // When $package is installed as a root package, then this deprecation - // is always ignored // first check that the caller is not from a tests folder, in which case we always let deprecations pass if (strpos($backtrace[1]['file'], '/tests/') === false) { if (strpos($backtrace[0]['file'], '/vendor/' . $package . '/') === false) { @@ -129,12 +145,6 @@ public static function triggerIfCalledFromOutside(string $package, string $link, return; } - // do not move this condition to the top, because we still want to - // count occcurences of deprecations even when we are not logging them. - if (self::$type === self::TYPE_NONE) { - return; - } - if (isset(self::$ignoredPackages[$package])) { return; } @@ -149,56 +159,61 @@ public static function triggerIfCalledFromOutside(string $package, string $link, */ private static function delegateTriggerToBackend(string $message, array $backtrace, string $link, string $package): void { - if (self::$type === self::TYPE_TRIGGER_ERROR) { - $message .= sprintf( - ' (%s:%d called by %s:%d, %s, package %s)', - basename($backtrace[0]['file']), - $backtrace[0]['line'], - basename($backtrace[1]['file']), - $backtrace[1]['line'], - $link, - $package - ); - - trigger_error($message, E_USER_DEPRECATED); - } elseif (self::$type === self::TYPE_TRIGGER_SUPPRESSED_ERROR) { - $message .= sprintf( - ' (%s:%d called by %s:%d, %s, package %s)', - basename($backtrace[0]['file']), - $backtrace[0]['line'], - basename($backtrace[1]['file']), - $backtrace[1]['line'], - $link, - $package - ); - - @trigger_error($message, E_USER_DEPRECATED); - } elseif (self::$type === self::TYPE_PSR_LOGGER) { + if ((self::$type & self::TYPE_PSR_LOGGER) > 0) { $context = [ 'file' => $backtrace[0]['file'], 'line' => $backtrace[0]['line'], + 'package' => $package, + 'link' => $link, ]; - $context['package'] = $package; - $context['link'] = $link; - self::$logger->notice($message, $context); } + + if (! ((self::$type & self::TYPE_TRIGGER_ERROR) > 0)) { + return; + } + + $message .= sprintf( + ' (%s:%d called by %s:%d, %s, package %s)', + self::basename($backtrace[0]['file']), + $backtrace[0]['line'], + self::basename($backtrace[1]['file']), + $backtrace[1]['line'], + $link, + $package + ); + + @trigger_error($message, E_USER_DEPRECATED); } - public static function enableWithTriggerError(): void + /** + * A non-local-aware version of PHPs basename function. + */ + private static function basename(string $filename): string { - self::$type = self::TYPE_TRIGGER_ERROR; + $pos = strrpos($filename, DIRECTORY_SEPARATOR); + + if ($pos === false) { + return $filename; + } + + return substr($filename, $pos + 1); } - public static function enableWithSuppressedTriggerError(): void + public static function enableTrackingDeprecations(): void + { + self::$type |= self::TYPE_TRACK_DEPRECATIONS; + } + + public static function enableWithTriggerError(): void { - self::$type = self::TYPE_TRIGGER_SUPPRESSED_ERROR; + self::$type |= self::TYPE_TRIGGER_ERROR; } public static function enableWithPsrLogger(LoggerInterface $logger): void { - self::$type = self::TYPE_PSR_LOGGER; + self::$type |= self::TYPE_PSR_LOGGER; self::$logger = $logger; } diff --git a/lib/Doctrine/Deprecations/PHPUnit/VerifyDeprecations.php b/lib/Doctrine/Deprecations/PHPUnit/VerifyDeprecations.php index 8bca97f..4c3366a 100644 --- a/lib/Doctrine/Deprecations/PHPUnit/VerifyDeprecations.php +++ b/lib/Doctrine/Deprecations/PHPUnit/VerifyDeprecations.php @@ -26,6 +26,14 @@ public function expectNoDeprecationWithIdentifier(string $identifier): void $this->doctrineNoDeprecationsExpectations[$identifier] = Deprecation::getTriggeredDeprecations()[$identifier] ?? 0; } + /** + * @before + */ + public function enableDeprecationTracking(): void + { + Deprecation::enableTrackingDeprecations(); + } + /** * @after */ diff --git a/test_fixtures/src/Foo.php b/test_fixtures/src/Foo.php index 79e27ae..c4b8ebe 100644 --- a/test_fixtures/src/Foo.php +++ b/test_fixtures/src/Foo.php @@ -8,13 +8,13 @@ class Foo { - public function triggerDependencyWithDeprecation(): void + public static function triggerDependencyWithDeprecation(): void { $bar = new Bar(); $bar->oldFunc(); } - public function triggerDependencyWithDeprecationFromInside(): void + public static function triggerDependencyWithDeprecationFromInside(): void { $bar = new Bar(); $bar->newFunc(); diff --git a/tests/Doctrine/Deprecations/DeprecationTest.php b/tests/Doctrine/Deprecations/DeprecationTest.php index 438122d..e59c146 100644 --- a/tests/Doctrine/Deprecations/DeprecationTest.php +++ b/tests/Doctrine/Deprecations/DeprecationTest.php @@ -9,12 +9,14 @@ use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use Doctrine\Foo\Baz; use PHPUnit\Framework\Error\Deprecated; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use ReflectionProperty; use Throwable; use function method_exists; +use function set_error_handler; class DeprecationTest extends TestCase { @@ -30,6 +32,8 @@ public function setUp(): void $reflectionProperty = new ReflectionProperty(Deprecation::class, 'ignoredLinks'); $reflectionProperty->setAccessible(true); $reflectionProperty->setValue([]); + + Deprecation::enableTrackingDeprecations(); } public function expectDeprecation(): void @@ -50,36 +54,38 @@ public function expectDeprecationMessage(string $message): void } } + public function expectErrorHandler(string $expectedMessage, string $identifier, int $times = 1): void + { + set_error_handler(function ($type, $message) use ($expectedMessage, $identifier, $times): void { + $this->assertStringMatchesFormat( + $expectedMessage, + $message + ); + $this->assertEquals([$identifier => $times], Deprecation::getTriggeredDeprecations()); + }); + } + public function testDeprecation(): void { Deprecation::enableWithTriggerError(); - $this->expectDeprecation(); - $this->expectDeprecationMessage('this is deprecated foo 1234 (DeprecationTest.php'); - $this->expectDeprecationWithIdentifier('https://github.com/doctrine/deprecations/1234'); - $e = null; - try { - Deprecation::trigger( - 'doctrine/orm', - 'https://github.com/doctrine/deprecations/1234', - 'this is deprecated %s %d', - 'foo', - 1234 - ); + $this->expectErrorHandler( + 'this is deprecated foo 1234 (DeprecationTest.php:%d called by TestCase.php:%d, https://github.com/doctrine/deprecations/1234, package doctrine/orm)', + 'https://github.com/doctrine/deprecations/1234' + ); - $this->fail('Should never be reached because of deprecation exception'); - } catch (Throwable $e) { - $this->assertStringMatchesFormat( - 'this is deprecated foo 1234 (DeprecationTest.php:%d called by TestCase.php:%d, https://github.com/doctrine/deprecations/1234, package doctrine/orm)', - $e->getMessage() - ); - $this->assertEquals(1, Deprecation::getUniqueTriggeredDeprecationsCount()); - $this->assertEquals(['https://github.com/doctrine/deprecations/1234' => 1], Deprecation::getTriggeredDeprecations()); - } + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/deprecations/1234', + 'this is deprecated %s %d', + 'foo', + 1234 + ); + + $this->assertEquals(1, Deprecation::getUniqueTriggeredDeprecationsCount()); - // this is caught by deduplication and does not throw Deprecation::trigger( 'doctrine/orm', 'https://github.com/doctrine/deprecations/1234', @@ -89,8 +95,6 @@ public function testDeprecation(): void ); $this->assertEquals(2, Deprecation::getUniqueTriggeredDeprecationsCount()); - - throw $e; } public function testDeprecationWithoutDeduplication(): void @@ -98,35 +102,36 @@ public function testDeprecationWithoutDeduplication(): void Deprecation::enableWithTriggerError(); Deprecation::withoutDeduplication(); - try { - Deprecation::trigger( - 'doctrine/orm', - 'https://github.com/doctrine/deprecations/1234', - 'this is deprecated %s %d', - 'foo', - 1234 - ); + $this->expectErrorHandler( + 'this is deprecated foo 2222 (DeprecationTest.php:%d called by TestCase.php:%d, https://github.com/doctrine/deprecations/2222, package doctrine/orm)', + 'https://github.com/doctrine/deprecations/2222' + ); - $this->fail('Should never be reached because of deprecation exception'); - } catch (Throwable $e) { - $this->assertEquals(1, Deprecation::getUniqueTriggeredDeprecationsCount()); - $this->assertEquals(['https://github.com/doctrine/deprecations/1234' => 1], Deprecation::getTriggeredDeprecations()); - } + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/deprecations/2222', + 'this is deprecated %s %d', + 'foo', + 2222 + ); - try { - Deprecation::trigger( - 'doctrine/orm', - 'https://github.com/doctrine/deprecations/1234', - 'this is deprecated %s %d', - 'foo', - 1234 - ); + $this->assertEquals(1, Deprecation::getUniqueTriggeredDeprecationsCount()); - $this->fail('Should never be reached because of deprecation exception'); - } catch (Throwable $e) { - $this->assertEquals(2, Deprecation::getUniqueTriggeredDeprecationsCount()); - $this->assertEquals(['https://github.com/doctrine/deprecations/1234' => 2], Deprecation::getTriggeredDeprecations()); - } + $this->expectErrorHandler( + 'this is deprecated foo 2222 (DeprecationTest.php:%d called by TestCase.php:%d, https://github.com/doctrine/deprecations/2222, package doctrine/orm)', + 'https://github.com/doctrine/deprecations/2222', + 2 + ); + + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/deprecations/2222', + 'this is deprecated %s %d', + 'foo', + 2222 + ); + + $this->assertEquals(2, Deprecation::getUniqueTriggeredDeprecationsCount()); } public function testDeprecationResetsCounts(): void @@ -147,19 +152,28 @@ public function testDeprecationResetsCounts(): void } } - public function testDeprecationWithPsrLogger(): void + public function expectDeprecationMock(string $message, string $identifier, string $package): MockObject { - $this->expectDeprecationWithIdentifier('https://github.com/doctrine/deprecations/2222'); - $mock = $this->createMock(LoggerInterface::class); - $mock->method('notice')->with('this is deprecated foo 1234', $this->callback(function ($context) { - $this->assertEquals(__FILE__, $context['file']); - $this->assertEquals('doctrine/orm', $context['package']); - $this->assertEquals('https://github.com/doctrine/deprecations/2222', $context['link']); + $mock->method('notice')->with($message, $this->callback(function ($context) use ($identifier, $package) { + $this->assertEquals($package, $context['package']); + $this->assertEquals($identifier, $context['link']); return true; })); + return $mock; + } + + public function testDeprecationWithPsrLogger(): void + { + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/deprecations/2222'); + + $mock = $this->expectDeprecationMock( + 'this is deprecated foo 1234', + 'https://github.com/doctrine/deprecations/2222', + 'doctrine/orm' + ); Deprecation::enableWithPsrLogger($mock); Deprecation::trigger( @@ -192,18 +206,12 @@ public function testDeprecationIfCalledFromOutside(): void { Deprecation::enableWithTriggerError(); - try { - Foo::triggerDependencyWithDeprecation(); - - $this->fail('Not expected to get here'); - } catch (Throwable $e) { - $this->assertEquals( - 'Bar::oldFunc() is deprecated, use Bar::newFunc() instead. (Bar.php:16 called by Foo.php:14, https://github.com/doctrine/foo, package doctrine/foo)', - $e->getMessage() - ); + $this->expectErrorHandler( + 'Bar::oldFunc() is deprecated, use Bar::newFunc() instead. (Bar.php:16 called by Foo.php:14, https://github.com/doctrine/foo, package doctrine/foo)', + 'https://github.com/doctrine/foo' + ); - $this->assertEquals(['https://github.com/doctrine/foo' => 1], Deprecation::getTriggeredDeprecations()); - } + Foo::triggerDependencyWithDeprecation(); } public function testDeprecationIfCalledFromOutsideNotTriggeringFromInside(): void @@ -229,25 +237,15 @@ public function testDeprecationCalledFromOutsideInRoot(): void { Deprecation::enableWithTriggerError(); - $this->expectDeprecation(); - $this->expectDeprecationMessage('this is deprecated foo 1234 (RootDeprecation.php'); - $this->expectDeprecationWithIdentifier('https://github.com/doctrine/deprecations/4444'); - $e = null; - try { - RootDeprecation::run(); + $this->expectErrorHandler( + 'this is deprecated foo 1234 (RootDeprecation.php:%d called by DeprecationTest.php:%d, https://github.com/doctrine/deprecations/4444, package doctrine/orm)', + 'https://github.com/doctrine/deprecations/4444' + ); - $this->fail('Should never be reached because of deprecation exception'); - } catch (Throwable $e) { - $this->assertStringMatchesFormat( - 'this is deprecated foo 1234 (RootDeprecation.php:%d called by DeprecationTest.php:%d, https://github.com/doctrine/deprecations/4444, package doctrine/orm)', - $e->getMessage() - ); - $this->assertEquals(1, Deprecation::getUniqueTriggeredDeprecationsCount()); - $this->assertEquals(['https://github.com/doctrine/deprecations/4444' => 1], Deprecation::getTriggeredDeprecations()); + RootDeprecation::run(); - throw $e; - } + $this->assertEquals(1, Deprecation::getUniqueTriggeredDeprecationsCount()); } } diff --git a/tests/Doctrine/Deprecations/VerifyDeprecationsTest.php b/tests/Doctrine/Deprecations/VerifyDeprecationsTest.php index 9fef00c..8681f7d 100644 --- a/tests/Doctrine/Deprecations/VerifyDeprecationsTest.php +++ b/tests/Doctrine/Deprecations/VerifyDeprecationsTest.php @@ -7,17 +7,16 @@ use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use PHPUnit\Framework\TestCase; +use function set_error_handler; + class VerifyDeprecationsTest extends TestCase { use VerifyDeprecations; - /** - * @before - */ - public function setUpDisableDeprecations(): void + public function setUp(): void { - // prevent PHPUnit from throwing Deprecation exception in case trigger_error was enabled before - Deprecation::disable(); + set_error_handler(static function (): void { + }); } public function testExpectDeprecationWithIdentifier(): void