Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ Alternatively, call:
\Doctrine\Deprecations\Deprecation::enableTrackingDeprecations();
```

To disable deduplication (useful for testing), set the `DOCTRINE_DEPRECATIONS_DEDUPLICATION`
environment variable to `false`. Alternatively, call:

```php
\Doctrine\Deprecations\Deprecation::withoutDeduplication();
```

Tracking is enabled with all three modes and provides access to all triggered
deprecations and their individual count:

Expand Down Expand Up @@ -182,8 +189,14 @@ deprecations triggered during the test suite execution.
Note that you can still trigger Deprecations in your code, provided you use the
`#[IgnoreDeprecations]` to ignore them for tests that call it.

At the moment, it is not possible to disable deduplication with an environment
variable, but you can use a bootstrap file to achieve that:
You can disable deduplication by setting the `DOCTRINE_DEPRECATIONS_DEDUPLICATION`
environment variable to `false`:

```bash
DOCTRINE_DEPRECATIONS_DEDUPLICATION=false vendor/bin/phpunit
```

Alternatively, you can use a bootstrap file to achieve the same effect:

```php
// tests/bootstrap.php
Expand Down
21 changes: 16 additions & 5 deletions src/Deprecation.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class Deprecation
/** @var array<string,bool> */
private static $ignoredLinks = [];

/** @var bool */
private static $deduplication = true;
/** @var bool|null */
private static $deduplication;

/**
* Trigger a deprecation for the given package and identfier.
Expand Down Expand Up @@ -93,7 +93,7 @@ public static function trigger(string $package, string $link, string $message, .
self::$triggeredDeprecations[$link] = 1;
}

if (self::$deduplication === true && self::$triggeredDeprecations[$link] > 1) {
if ((self::$deduplication ?? self::getDeduplicationFromEnv()) === true && self::$triggeredDeprecations[$link] > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self::$deduplication ?? self::getDeduplicationFromEnv() is a repeating pattern. I'd make a static function for it that returns self::$deduplication ?? ...whateverDerivedFromEnv...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self::$type ?? self::getTypeFromEnv(); is also a repeating pattern (even more repeating in fact), so although I had the same idea as you, I thought I would be consistent with the existing code

return;
}

Expand Down Expand Up @@ -160,7 +160,7 @@ public static function triggerIfCalledFromOutside(string $package, string $link,
self::$triggeredDeprecations[$link] = 1;
}

if (self::$deduplication === true && self::$triggeredDeprecations[$link] > 1) {
if ((self::$deduplication ?? self::getDeduplicationFromEnv()) === true && self::$triggeredDeprecations[$link] > 1) {
return;
}

Expand Down Expand Up @@ -250,7 +250,7 @@ public static function disable(): void
{
self::$type = self::TYPE_NONE;
self::$logger = null;
self::$deduplication = true;
self::$deduplication = null;
self::$ignoredLinks = [];

foreach (self::$triggeredDeprecations as $link => $count) {
Expand Down Expand Up @@ -306,4 +306,15 @@ private static function getTypeFromEnv(): int

return self::$type;
}

private static function getDeduplicationFromEnv(): bool
{
$envValue = $_SERVER['DOCTRINE_DEPRECATIONS_DEDUPLICATION'] ?? $_ENV['DOCTRINE_DEPRECATIONS_DEDUPLICATION'] ?? null;

if ($envValue === 'false' || $envValue === '0') {
return self::$deduplication = false;
}

return self::$deduplication = true;
Comment on lines +314 to +318
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this doing here? Flipping self::$deduplication to some value as a side effect?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 what do you mean "a side effect"? If this piece of code is reached, then it means there is either no env var defined, or it is neither false nor 0. In that case, we should fallback to the value before this PR, which was true.

}
}
83 changes: 83 additions & 0 deletions tests/DeprecationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,87 @@ public function testDeprecationTriggeredFromNativeCode(): void
restore_error_handler();
}
}

public function testDeprecationWithoutDeduplicationByEnv(): void
{
$_ENV['DOCTRINE_DEPRECATIONS_DEDUPLICATION'] = 'false';

Deprecation::enableWithTriggerError();

$errorCount = 0;
set_error_handler(static function () use (&$errorCount): bool {
$errorCount++;

return true;
});

try {
// Trigger the same deprecation multiple times
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/deprecations/issues/123',
'this is deprecated from env test'
);

Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/deprecations/issues/123',
'this is deprecated from env test'
);

Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/deprecations/issues/123',
'this is deprecated from env test'
);

$this->assertEquals(3, $errorCount, 'Expected 3 deprecation warnings to be triggered');
$this->assertEquals(3, Deprecation::getUniqueTriggeredDeprecationsCount());
} finally {
restore_error_handler();
unset($_ENV['DOCTRINE_DEPRECATIONS_DEDUPLICATION']);
}
}

public function testDeprecationWithDeduplicationEnabledByDefault(): void
{
// Ensure no environment variable is set
unset($_ENV['DOCTRINE_DEPRECATIONS_DEDUPLICATION']);

Deprecation::enableWithTriggerError();

$errorCount = 0;
set_error_handler(static function () use (&$errorCount): bool {
$errorCount++;

return true;
});

try {
// Trigger the same deprecation multiple times
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/deprecations/issues/123',
'this is deprecated default test'
);

Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/deprecations/issues/123',
'this is deprecated default test'
);

Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/deprecations/issues/123',
'this is deprecated default test'
);

// With deduplication enabled (default), only first should trigger
$this->assertEquals(1, $errorCount);
$this->assertEquals(3, Deprecation::getUniqueTriggeredDeprecationsCount());
} finally {
restore_error_handler();
}
}
}
Loading