8000 [DI] Prepare dropping "strict" handling in loaders by nicolas-grekas · Pull Request #20938 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Prepare dropping "strict" handling in loaders #20938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 26, 2016
Merged

[DI] Prepare dropping "strict" handling in loaders #20938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
[DI] Prepare dropping "strict" handling in loaders
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Dec 26, 2016
commit 243d1606158e7da6c87e79865f0581f6e76d7f28
8 changes: 1 addition & 7 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,13 +387,7 @@ private function getArgumentsAsPhp(\DOMElement $node, $name, $lowercase = true)
$invalidBehavior = ContainerInterface::NULL_ON_INVALID_REFERENCE;
}

if ($strict = $arg->getAttribute('strict')) {
$strict = XmlUtils::phpize($strict);
} else {
$strict = true;
}

$arguments[$key] = new Reference($arg->getAttribute('id'), $invalidBehavior, $strict);
$arguments[$key] = new Reference($arg->getAttribute('id'), $invalidBehavior);
Copy link
Member

Choose a reason for hiding this comment

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

I would still trigger a deprecation if the attribute is there, otherwise we cannot remove it from the XSD in 4.x

Copy link
Member Author

Choose a reason for hiding this comment

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

But there is no way to create any continuous upgrade path in XML files. That's why we didn't remove "strict" from the XSD, to allow cross 2x3 bundles. Should we consider this is not required anymore? Not sure on my side (because it doesn't hurt). WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

But the same is true for the YAML format, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

Well, you kept the attribute in the XSD, which is fine to allow the continuous migration. But you should now trigger the deprecation to allow the removal in 4.0

Copy link
Member

Choose a reason for hiding this comment

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

@xabbuh the YAML format does not have a schema restricting what gets written.

Copy link
Member

Choose a reason for hiding this comment

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

@stof I meant providing a file that works with both 2.x and 3.x. If we didn't trigger the deprecation with the XML format, it means that we assumed something did set the strict option to false (otherwise you wouldn't need to provide the option at all). However, in YAML that wouldn't be necessary at all.

Copy link
Member

Choose a reason for hiding this comment

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

@xabbuh setting a reference as not strict is deprecated in 2.8 already. The option is a no-op in 3.0.
So the way to make a file work in 2.8, 3.x and 4.0 is to skip the attribute entirely and avoiding the usage of non-strict references (which are a source of bugs anyway, as it means you bypass the scope restrictions)

Copy link
Member D95C

Choose a reason for hiding this comment

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

@stof I agree with you. But the same is true for the YAML format. So IMO it's inconsistent to deprecate this only in the YAML format. See #21058 to also trigger the deprecation with the XML format.

break;
case 'expression':
$arguments[$key] = new Expression($arg->nodeValue);
Expand Down
6 changes: 2 additions & 4 deletions src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,14 +471,12 @@ private function resolveServices($value)
}

if ('=' === substr($value, -1)) {
@trigger_error(sprintf('The "=" suffix that used to disable strict references in Symfony 2.x is deprecated since 3.3 and will be unsupported in 4.0. Remove it in "%s".', $value), E_USER_DEPRECATED);
$value = substr($value, 0, -1);
$strict = false;
} else {
$strict = true;
}

if (null !== $invalidBehavior) {
$value = new Reference($value, $invalidBehavior, $strict);
$value = new Reference($value, $invalidBehavior);
}
}

Expand Down
0