8000 [DI] Rework config hierarchy: defaults > instanceof > service config by weaverryan · Pull Request #22294 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Rework config hierarchy: defaults > instanceof > service config #22294

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

Closed
wants to merge 19 commits into from
Closed

[DI] Rework config hierarchy: defaults > instanceof > service config #22294

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
8698228
Reworking the instanceof feature to act more like a "default" than li…
weaverryan Apr 5, 2017
db96c89
Moving changes tracking from ChildDefinition to Definition and fixing…
weaverryan Apr 5, 2017
24a6a83
Adding a test for the config overriding order
weaverryan Apr 6, 2017
6202909
Fixing a bug where instanceof was not copied into the new Definition …
weaverryan Apr 6, 2017
28d7d8d
Adding a test for instanceof behavior with parent-child services
weaverryan Apr 6, 2017
de0f718
Adding more tests
weaverryan Apr 6, 2017
8fd3738
Updating calls logic to merge/override with instanceof
weaverryan Apr 6, 2017
5e39c5d
Using Definition instead of ChildDefinition for instanceofConditionals
weaverryan Apr 6, 2017
132201e
Fixing outdated type-hint
weaverryan Apr 6, 2017
d7c4836
Fixing method name after rebase
weaverryan Apr 6, 2017
a44d92b
Making instanceof method call matching case insensitive
weaverryan Apr 6, 2017
a2aea3b
Fixing an edge case with instanceof property overriding when value is…
weaverryan Apr 6, 2017
5f9ca7c
A few tweaks thanks to nicolas
weaverryan Apr 6, 2017
2021675
Fixing missing use
weaverryan Apr 6, 2017
a95de6c
Thanks fabbot!
weaverryan Apr 6, 2017
34ad21a
Updating more code for ChildDefinition -> Definition change
weaverryan Apr 6, 2017
371fec2
Fixing failing tests: some of the compiler passes caused false "chang…
weaverryan Apr 6, 2017
82fa3c0
Updating DI tags logic to merge, but not replace tags
weaverryan Apr 6, 2017
1f2ce11
thanks fabbot!
weaverryan Apr 6, 2017
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
Prev Previous commit
Next Next commit
A few tweaks thanks to nicolas
  • Loading branch information
@weaverryan
weaverryan committed Apr 6, 2017
commit 5f9ca7cccc7248fb591af16d5a7f3dda25aa881d
26 changes: 13 additions & 13 deletions src/Symfony/Component/DependencyInjection/Definition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function setFactory($factory)
$factory = explode('::', $factory, 2);
}

$this->trackChange('factory');
$this->recordChange('factory');
$this->factory = $factory;

return $this;
Expand Down Expand Up @@ -102,7 +102,7 @@ public function setDecoratedService($id, $renamedId = null, $priority = 0)
throw new InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id));
}

$this->trackChange('decorated_service');
$this->recordChange('decorated_service');

if (null === $id) {
$this->decoratedService = null;
Expand Down Expand Up @@ -132,7 +132,7 @@ public function getDecoratedService()
*/
public function setClass($class)
{
$this->trackChange('class');
$this->recordChange('class');
$this->class = $class;

return $this;
Expand Down Expand Up @@ -456,7 +456,7 @@ public function clearTags()
*/
public function setFile($file)
{
$this->trackChange('file');
$this->recordChange('file');
$this->file = $file;

return $this;
Expand All @@ -481,7 +481,7 @@ public function getFile()
*/
public function setShared($shared)
{
$this->trackChange('shared');
$this->recordChange('shared');
$this->shared = (bool) $shared;

return $this;
Expand All @@ -506,7 +506,7 @@ public function isShared()
*/
public function setPublic($boolean)
{
$this->trackChange('public');
$this->recordChange('public');
$this->public = (bool) $boolean;

return $this;
Expand All @@ -531,7 +531,7 @@ public function isPublic()
*/
public function setLazy($lazy)
{
$this->trackChange('lazy');
$this->recordChange('lazy');
$this->lazy = (bool) $lazy;

return $this;
Expand All @@ -557,7 +557,7 @@ public function isLazy()
*/
public function setSynthetic($boolean)
{
$this->trackChange('synthetic');
$this->recordChange('synthetic');
$this->synthetic = (bool) $boolean;

return $this;
Expand All @@ -584,7 +584,7 @@ public function isSynthetic()
*/
public function setAbstract($boolean)
{
$this->trackChange('abstract');
$this->recordChange('abstract');
$this->abstract = (bool) $boolean;

return $this;
Expand Down Expand Up @@ -626,7 +626,7 @@ public function setDeprecated($status = true, $template = null)
$this->deprecationTemplate = $template;
}

$this->trackChange('deprecated');
$this->recordChange('deprecated');
$this->deprecated = (bool) $status;

return $this;
Expand Down Expand Up @@ -668,7 +668,7 @@ public function setConfigurator($configurator)
$configurator = explode('::', $configurator, 2);
}

$this->trackChange('configurator');
$this->recordChange('configurator');
$this->configurator = $configurator;

return $this;
Expand Down Expand Up @@ -725,7 +725,7 @@ public function isAutowired()
*/
public function setAutowired($autowired)
{
$this->trackChange('autowired');
$this->recordChange('autowired');
$this->autowired = (bool) $autowired;

return $this;
Expand Down Expand Up @@ -823,7 +823,7 @@ public function setTrackChanges($trackChanges)
return $this;
}

private function trackChange($type)
private function recordChange($type)
{
if ($this->trackChanges) {
$this->changes[$type] = true;
Expand Down
7 changes: 3 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 @@ -497,10 +497,9 @@ private function parseDefinition($id, $service, $file, array $defaults)
if ($autowireDefaultsUsed) {
Copy link
Member

Choose a reason for hiding this comment

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

"if" not needed, call setTrackChanges(!$autowireDefaultsUsed)

$definition->setTrackChanges(false);
}
$definition->setAutowired($autowire);
if ($autowireDefaultsUsed) {
$definition->setTrackChanges(true);
}
$definition->setAutowired($autowire)
->setTrackChanges(true)
;
Copy link
Contributor

Choose a reason for hiding this comment

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

What about updating this to:

        if (isset($service['autowire'])) {
            $definition->setAutowired($service['autowire']);
        } elseif (isset($defaults['autowire'])) {
            $definition
                ->setTrackChanges(false)
                ->setAutowired($defaults['autowire'])
                ->setTrackChanges(true);
        }

}

if (isset($service['autowiring_types'])) {
Expand Down
0