Merge pull request #21 from doctrine/GH-20-TriggerWhenNotFromPackage

[GH-20] triggerIfCalledFromOutside
doctrine · Mar 11, 2021 · db319bc · db319bc
2 parents 011e779 + 907ac04
commit db319bc
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 65 deletions.
diff --git a/README.md b/README.md
@@ -56,6 +56,9 @@ the deduplication can be disabled:
 
 ## Usage from a library/producer perspective:
 
+When you want to unconditionally trigger a deprecation even when called
+from the library itself then the `trigger` method is the way to go:
+
 ```php
 \Doctrine\Deprecations\Deprecation::trigger(
     "doctrine/orm",
@@ -77,6 +80,18 @@ the message.
 );
 ```
 
+When you want to trigger a deprecation only when it is called by a function
+outside of the current package, but not trigger when the package itself is the cause,
+then use:
+
+```php
+\Doctrine\Deprecations\Deprecation::triggerIfCalledFromOutside(
+    "doctrine/orm",
+    "https://link/to/deprecations-description",
+    "message"
+);
+```
+
 Based on the issue link each deprecation message is only triggered once per
 request, so it must be unique for each deprecation.
 
@@ -87,21 +102,6 @@ Note: A producer/library should never call `Deprecation::enableWith` methods
 and leave the decision how to handle deprecations to application and
 frameworks.
 
-Sometimes you cannot avoid to trigger a deprecation from the library itself,
-specifically when the new alternative functionality still calls the old API
-internally.
-
-In that case you can temporarily ignore a deprecation for 1 or more calls with
-this API:
-
-```php
-\Doctrine\Deprecations\Deprecation::ignoreDeprecationTemporarily(
-    'https://github.com/doctrine/orm/issue/1234'
-);
-```
-
-Pass the number of times to ignore as a second argument when it is more than once.
-
 ## Usage in PHPUnit tests
 
 There is a `VerifyDeprecations` trait that you can use to make assertions on

diff --git a/lib/Doctrine/Deprecations/Deprecation.php b/lib/Doctrine/Deprecations/Deprecation.php
@@ -11,6 +11,7 @@
 use function basename;
 use function debug_backtrace;
 use function sprintf;
+use function strpos;
 use function trigger_error;
 
 use const DEBUG_BACKTRACE_IGNORE_ARGS;
@@ -55,9 +56,6 @@ class Deprecation
     /** @var array<string,int> */
     private static $ignoredLinks = [];
 
-    /** @var array<string,int> */
-    private static $temporarilyIgnoredLinks = [];
-
     /** @var bool */
     private static $deduplication = true;
 
@@ -72,15 +70,49 @@ class Deprecation
      */
     public static function trigger(string $package, string $link, string $message, ...$args): void
     {
-        // Do not trigger this deprecation if it is temporarily ignored,
-        // because it is expected to be called for 1 or more times.
-        if (array_key_exists($link, self::$temporarilyIgnoredLinks)) {
-            self::$temporarilyIgnoredLinks[$link]--;
+        if (array_key_exists($link, self::$ignoredLinks)) {
+            self::$ignoredLinks[$link]++;
+        } else {
+            self::$ignoredLinks[$link] = 1;
+        }
+
+        if (self::$deduplication === true && self::$ignoredLinks[$link] > 1) {
+            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;
+        }
+
+        $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2);
+
+        $message = sprintf($message, ...$args);
+
+        self::delegateTriggerToBackend($message, $backtrace, $link, $package);
+    }
+
+    /**
+     * @param mixed $args
+     */
+    public static function triggerIfCalledFromOutside(string $package, string $link, string $message, ...$args): void
+    {
+        $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2);
 
-            if (self::$temporarilyIgnoredLinks[$link] <= 0) {
-                unset(self::$temporarilyIgnoredLinks[$link]);
-            }
+        // "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
+        if (strpos($backtrace[0]['file'], 'vendor/' . $package . '/') === false) {
+            return;
+        }
 
+        if (strpos($backtrace[1]['file'], 'vendor/' . $package . '/') !== false) {
             return;
         }
 
@@ -104,10 +136,16 @@ public static function trigger(string $package, string $link, string $message, .
             return;
         }
 
-        $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2);
-
         $message = sprintf($message, ...$args);
 
+        self::delegateTriggerToBackend($message, $backtrace, $link, $package);
+    }
+
+    /**
+     * @param array<mixed> $backtrace
+     */
+    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)',
@@ -175,8 +213,6 @@ public static function disable(): void
         foreach (self::$ignoredLinks as $link => $count) {
             self::$ignoredLinks[$link] = 0;
         }
-
-        self::$temporarilyIgnoredLinks = [];
     }
 
     public static function ignorePackage(string $packageName): void
@@ -191,11 +227,6 @@ public static function ignoreDeprecations(string ...$links): void
         }
     }
 
-    public static function ignoreDeprecationTemporarily(string $link, int $times = 1): void
-    {
-        self::$temporarilyIgnoredLinks[$link] = $times;
-    }
-
     public static function getUniqueTriggeredDeprecationsCount(): int
     {
         return array_reduce(self::$ignoredLinks, static function (int $carry, int $count) {

diff --git a/tests/Doctrine/Deprecations/DeprecationTest.php b/tests/Doctrine/Deprecations/DeprecationTest.php
@@ -184,35 +184,4 @@ public function testDeprecationWithIgnoredPackage(): void
         $this->assertEquals(1, Deprecation::getUniqueTriggeredDeprecationsCount());
         $this->assertEquals(['https://github.com/doctrine/orm/issue/1234' => 1], Deprecation::getTriggeredDeprecations());
     }
-
-    public function testDeprecationWithTemporarilyIgnored(): void
-    {
-        Deprecation::enableWithTriggerError();
-        Deprecation::ignoreDeprecationTemporarily('https://github.com/doctrine/orm/issue/3333');
-
-        Deprecation::trigger(
-            'doctrine/orm',
-            'https://github.com/doctrine/orm/issue/3333',
-            'this is deprecated %s %d',
-            'foo',
-            1234
-        );
-
-        $this->assertEquals(0, Deprecation::getUniqueTriggeredDeprecationsCount());
-
-        try {
-            Deprecation::trigger(
-                'doctrine/orm',
-                'https://github.com/doctrine/orm/issue/3333',
-                'this is deprecated %s %d',
-                'foo',
-                1234
-            );
-
-            $this->fail('Not expected');
-        } catch (Throwable $e) {
-            $this->assertEquals(1, Deprecation::getUniqueTriggeredDeprecationsCount());
-            $this->assertEquals(['https://github.com/doctrine/orm/issue/3333' => 1], Deprecation::getTriggeredDeprecations());
-        }
-    }
 }