feature #35648 [Contracts/Deprecation] don't use assert(), rename to trigger_deprecation() (nicolas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[Contracts/Deprecation] don't use assert(), rename to trigger_deprecation()

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This PR is a follow up the discussion that happened in #35526.

I applied all the suggestions I received so far on this previous PR so that we can discuss them all together.

Here are the changes:
- the most important change is to *not* use `assert()` anymore. This fixes the objection from @ostrolucky and @derrabus that deprecations and assert() should not be bound to each other. Instead, in order to still make the function a no-op when wanted, it is suggested that apps declare an empty function. This will be more flexible in practice I believe and keeps all the benefits we envisioned by using assert();
- as suggested by @fabpot, the function is renamed `trigger_deprecation()` instead of `deprecated()`. Because the implementation is now not conditional anymore (from the package pov, since assert() is not used), my arguments to keep the previous name don't stand as strong as before to me so I'm fine renaming.
- type hints are added back (requiring PHP 7.1 to use `void`). As mentioned in the 1st point, ppl that really want deprecations to be no-ops will redeclare the function as empty, thus not using any types, thus reclaiming all the perf diff. And for ppl that do want to catch deprecations, the overhead of types is negligible compared to any processing of the deprecations themselves.

WDYT?

All points can be discussed separately if needed of course. I'm personally fine with merging the PR as is, with all 3 changes.

Commits
-------

0032b2a289 [Contracts/Deprecation] don't use assert(), rename to trigger_deprecation()
This commit is contained in:
Fabien Potencier 2020-02-08 14:30:44 +01:00
commit 55e2c5b37d
5 changed files with 38 additions and 42 deletions

View File

@ -3,9 +3,10 @@ Symfony Deprecation Contracts
A generic function and convention to trigger deprecation notices.
This package provides a single global function named `deprecated()`.
Its purpose is to trigger deprecations in a way that can be silenced on production environments
by using the `zend.assertions` ini setting and that can be caught during development to generate reports.
This package provides a single global function named `trigger_deprecation()` that triggers silenced deprecation notices.
By using a custom PHP error handler such as the one provided by the Symfony ErrorHandler component,
the triggered deprecations can be caught and logged for later discovery, both on dev and prod environments.
The function requires at least 3 arguments:
- the name of the Composer package that is triggering the deprecation
@ -15,8 +16,11 @@ The function requires at least 3 arguments:
Example:
```php
deprecated('symfony/blockchain', 8.9, 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin');
trigger_deprecation('symfony/blockchain', '8.9', 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin');
```
This will generate the following message:
`Since symfony/blockchain 8.9: Using "bitcoin" is deprecated, use "fabcoin" instead.`
While not necessarily recommended, the deprecation notices can be completely ignored by declaring an empty
`function trigger_deprecation() {}` in your application.

View File

@ -15,11 +15,11 @@
}
],
"require": {
"php": "^7.0"
"php": "^7.1"
},
"autoload": {
"files": [
"deprecated.php"
"function.php"
]
},
"minimum-stability": "dev",

View File

@ -1,35 +0,0 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
/**
* Triggers a deprecation.
*
* As recommended for prod, turn the "zend.assertions" ini setting to 0 or -1 to disable deprecation notices.
* Alternatively, provide your own implementation of the function and list "symfony/deprecation-contracts"
* in the "replace" section of your root composer.json if you need any custom behavior.
*
* The function doesn't use type hints to make it as fast as possible.
*
* @param string $package The name of the Composer package that is triggering the deprecation
* @param string $version The version of the package that introduced the deprecation
* @param string $message The message of the deprecation
* @param scalar ...$args Values to insert in the message using printf() formatting
*
* @author Nicolas Grekas <p@tchwork.com>
*/
function deprecated($package, $version, $message, ...$args)
{
assert(@trigger_error(
($package || $version ? "Since $package $version: " : '')
.($args ? vsprintf($message, $args) : $message),
E_USER_DEPRECATED
));
}

View File

@ -0,0 +1,27 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
if (!function_exists('trigger_deprecation')) {
/**
* Triggers a silenced deprecation notice.
*
* @param string $package The name of the Composer package that is triggering the deprecation
* @param string $version The version of the package that introduced the deprecation
* @param string $message The message of the deprecation
* @param mixed ...$args Values to insert in the message using printf() formatting
*
* @author Nicolas Grekas <p@tchwork.com>
*/
function trigger_deprecation(string $package, string $version, string $message, ...$args): void
{
@trigger_error(($package || $version ? "Since $package $version: " : '').($args ? vsprintf($message, $args) : $message), E_USER_DEPRECATED);
}
}

View File

@ -41,7 +41,7 @@
},
"autoload": {
"psr-4": { "Symfony\\Contracts\\": "" },
"files": [ "Deprecation/deprecated.php" ],
"files": [ "Deprecation/function.php" ],
"exclude-from-classmap": [
"**/Tests/"
]