Skip to content

Commit

Permalink
bug #37022 [DependencyInjection] Improve missing package/version depr…
Browse files Browse the repository at this point in the history
…ecation (acrobat)

This PR was merged into the 5.1 branch.

Discussion
----------

[DependencyInjection] Improve missing package/version deprecation

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       |
| License       | MIT
| Doc PR        |

After updating to symfony 5.1 I've got some deprecations related to the missing package/version attributes/options for `deprecated` on services.

But currently it's not clear which bundle/part of the code is triggering the deprecations. The only way for me to track down where they were coming from was by setting a xdebug breakpoint in the `XmlFileLoader` and check the `$file` variable.

So it seemed like a good idea to include the file path in the deprecation message, that way it will be easier for users to know if their code or a bundle (and which) is triggering this deprecation.

Before:
<img width="871" alt="Screenshot 2020-05-31 at 13 51 03" src="https://user-images.githubusercontent.com/1374857/83351609-d0d65600-a345-11ea-9785-3237a3ec2360.png">

After:
<img width="907" alt="Screenshot 2020-05-31 at 13 50 10" src="https://user-images.githubusercontent.com/1374857/83351606-cfa52900-a345-11ea-9617-60d07e46234b.png">

Commits
-------

f603317 [DependencyInjection] Improve missing package/version deprecation
  • Loading branch information
fabpot committed Jun 3, 2020
2 parents 773b4ef + f603317 commit e778ea6
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 12 deletions.
Expand Up @@ -205,11 +205,11 @@ private function parseDefinition(\DOMElement $service, string $file, Definition
$version = $deprecated[0]->getAttribute('version') ?: '';

if (!$deprecated[0]->hasAttribute('package')) {
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "package" of the node "deprecated" is deprecated.');
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "package" of the node "deprecated" in "%s" is deprecated.', $file);
}

if (!$deprecated[0]->hasAttribute('version')) {
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "version" of the node "deprecated" is deprecated.');
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "version" of the node "deprecated" in "%s" is deprecated.', $file);
}

$alias->setDeprecated($package, $version, $message);
Expand Down Expand Up @@ -265,11 +265,11 @@ private function parseDefinition(\DOMElement $service, string $file, Definition
$version = $deprecated[0]->getAttribute('version') ?: '';

if ('' === $package) {
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "package" of the node "deprecated" is deprecated.');
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "package" of the node "deprecated" in "%s" is deprecated.', $file);
}

if ('' === $version) {
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "version" of the node "deprecated" is deprecated.');
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "version" of the node "deprecated" in "%s" is deprecated.', $file);
}

$definition->setDeprecated($package, $version, $message);
Expand Down
Expand Up @@ -409,11 +409,11 @@ private function parseDefinition(string $id, $service, string $file, array $defa
$deprecation = \is_array($value) ? $value : ['message' => $value];

if (!isset($deprecation['package'])) {
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "package" of the "deprecated" option is deprecated.');
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "package" of the "deprecated" option in "%s" is deprecated.', $file);
}

if (!isset($deprecation['version'])) {
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "version" of the "deprecated" option is deprecated.');
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "version" of the "deprecated" option in "%s" is deprecated.', $file);
}

$alias->setDeprecated($deprecation['package'] ?? '', $deprecation['version'] ?? '', $deprecation['message']);
Expand Down Expand Up @@ -478,11 +478,11 @@ private function parseDefinition(string $id, $service, string $file, array $defa
$deprecation = \is_array($service['deprecated']) ? $service['deprecated'] : ['message' => $service['deprecated']];

if (!isset($deprecation['package'])) {
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "package" of the "deprecated" option is deprecated.');
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "package" of the "deprecated" option in "%s" is deprecated.', $file);
}

if (!isset($deprecation['version'])) {
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "version" of the "deprecated" option is deprecated.');
trigger_deprecation('symfony/dependency-injection', '5.1', 'Not setting the attribute "version" of the "deprecated" option in "%s" is deprecated.', $file);
}

$definition->setDeprecated($deprecation['package'] ?? '', $deprecation['version'] ?? '', $deprecation['message']);
Expand Down
Expand Up @@ -409,7 +409,7 @@ public function testDeprecated()
*/
public function testDeprecatedWithoutPackageAndVersion()
{
$this->expectDeprecation('Since symfony/dependency-injection 5.1: Not setting the attribute "package" of the node "deprecated" is deprecated.');
$this->expectDeprecation('Since symfony/dependency-injection 5.1: Not setting the attribute "package" of the node "deprecated" in "%s" is deprecated.');

$container = new ContainerBuilder();
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
Expand Down Expand Up @@ -442,7 +442,7 @@ public function testDeprecatedAliases()
*/
public function testDeprecatedAliaseWithoutPackageAndVersion()
{
$this->expectDeprecation('Since symfony/dependency-injection 5.1: Not setting the attribute "package" of the node "deprecated" is deprecated.');
$this->expectDeprecation('Since symfony/dependency-injection 5.1: Not setting the attribute "package" of the node "deprecated" in "%s" is deprecated.');

$container = new ContainerBuilder();
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
Expand Down
Expand Up @@ -238,8 +238,8 @@ public function testDeprecatedAliases()
*/
public function testDeprecatedAliasesWithoutPackageAndVersion()
{
$this->expectDeprecation('Since symfony/dependency-injection 5.1: Not setting the attribute "package" of the "deprecated" option is deprecated.');
$this->expectDeprecation('Since symfony/dependency-injection 5.1: Not setting the attribute "version" of the "deprecated" option is deprecated.');
$this->expectDeprecation('Since symfony/dependency-injection 5.1: Not setting the attribute "package" of the "deprecated" option in "%s" is deprecated.');
$this->expectDeprecation('Since symfony/dependency-injection 5.1: Not setting the attribute "version" of the "deprecated" option in "%s" is deprecated.');

$container = new ContainerBuilder();
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
Expand Down

0 comments on commit e778ea6

Please sign in to comment.