8000 Reworking the instanceof feature to act more like a "default" than li… · symfony/symfony@5279a96 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5279a96

Browse files
weaverryanweaverryan
committed
Reworking the instanceof feature to act more like a "default" than like a ChildDefinition
tl;dr Before, instanceof values would replace those explicitly set on services. Now, it acts more like a default: only setting a value on a service if that value wasn't explicitly set on that service (i.e. don't override). For things like tags and calls, they're merged intelligently and on a case-by-case basis (merging tags works different than merging calls)
1 parent 3a45a9d commit 5279a96

File tree

10 files changed

+148
-126
lines changed

10 files changed

+148
-126
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@ public function __construct()
4242
$this->beforeOptimizationPasses = array(
4343
100 => array(
4444
$resolveClassPass = new ResolveClassPass(),
45-
new ResolveDefinitionInheritancePass(),
4645
),
4746
);
4847

4948
$this->optimizationPasses = array(array(
5049
new ExtensionCompilerPass(),
5150
new ResolveDefinitionTemplatesPass(),
51+
new ResolveDefinitionInheritancePass(),
5252
new ServiceLocatorTagPass(),
5353
new DecoratorServicePass(),
5454
new ResolveParameterPlaceHoldersPass(),

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionInheritancePass.php

Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ protected function processValue($value, $isRoot = false)
2727
return parent::processValue($value, $isRoot);
2828
}
2929

30-
$class = $value instanceof ChildDefinition ? $this->resolveDefinition($value) : $value->getClass();
30+
$class = $value->getClass();
3131

3232
if (!$class || false !== strpos($class, '%') || !$instanceof = $value->getInstanceofConditionals()) {
3333
return parent::processValue($value, $isRoot);
@@ -39,57 +39,50 @@ protected function processValue($value, $isRoot = false)
3939
continue;
4040
}
4141
if ($interface === $class || is_subclass_of($class, $interface)) {
42-
$this->mergeDefinition($value, $definition);
42+
$this->mergeInstanceofDefinition($value, $definition);
4343
}
4444
}
4545

4646
return parent::processValue($value, $isRoot);
4747
}
4848

49-
/**
50-
* Populates the class and tags from parent definitions.
51-
*/
52-
private function resolveDefinition(ChildDefinition $definition)
49+
private function mergeInstanceofDefinition(Definition $def, ChildDefinition $instanceofDefinition)
5350
{
54-
if (!$this->container->has($parent = $definition->getParent())) {
55-
return;
51+
$configured = $def->getConfiguredParts();
52+
$changes = $instanceofDefinition->getChanges();
53+
if (!isset($configured['shared']) && isset($changes['shared'])) {
54+
$def->setShared($instanceofDefinition->isShared());
5655
}
57-
58-
$parentDef = $this->container->findDefinition($parent);
59-
$class = $parentDef instanceof ChildDefinition ? $this->resolveDefinition($parentDef) : $parentDef->getClass();
60-
$class = $definition->getClass() ?: $class;
61-
62-
// append parent tags when inheriting is enabled
63-
if ($definition->getInheritTags()) {
64-
$definition->setInheritTags(false);
65-
66-
foreach ($parentDef->getTags() as $k => $v) {
67-
foreach ($v as $v) {
68-
$definition->addTag($k, $v);
69-
}
70-
}
56+
if (!isset($configured['configurator']) && isset($changes['configurator'])) {
57+
$def->setConfigurator($instanceofDefinition->getConfigurator());
7158
}
72-
73-
return $class;
74-
}
75-
76-
private function mergeDefinition(Definition $def, ChildDefinition $definition)
77-
{
78-
$changes = $definition->getChanges();
79-
if (isset($changes['shared'])) {
80-
$def->setShared($definition->isShared());
59+
if (!isset($configured['public']) && isset($changes['public'])) {
60+
$def->setPublic($instanceofDefinition->isPublic());
8161
}
82-
if (isset($changes['abstract'])) {
83-
$def->setAbstract($definition->isAbstract());
62+
if (!isset($configured['lazy']) && isset($changes['lazy'])) {
63+
$def->setLazy($instanceofDefinition->isLazy());
8464
}
85-
86-
ResolveDefinitionTemplatesPass::mergeDefinition($def, $definition);
87-
88-
// prepend instanceof tags
89-
$tailTags = $def->getTags();
90-
if ($headTags = $definition->getTags()) {
91-
$def->setTags($headTags);
92-
foreach ($tailTags as $k => $v) {
65+
if (!isset($configured['autowired']) && isset($changes['autowired'])) {
66+
$def->setAutowired($instanceofDefinition->getAutowired());
67+
}
68+
// merge properties
69+
$properties = $def->getProperties();
70+
foreach ($instanceofDefinition->getProperties() as $k => $v) {
71+
// don't override properties set explicitly on the service
72+
if (!isset($properties[$k])) {
73+
$def->setProperty($k, $v);
74+
}
75+
}
76+
// append method calls
77+
if ($calls = $instanceofDefinition->getMethodCalls()) {
78+
$def->setMethodCalls(array_merge($def->getMethodCalls(), $calls));
79+
}
80+
// merge tags
81+
$tags = $def->getTags();
82+
foreach ($instanceofDefinition->getTags() as $k => $v) {
83+
// don't add a tag if one by that name was already added
84+
if (!isset($tags[$k])) {
85+
// loop over the tag attributes arrays, add each
9386
foreach ($v as $v) {
9487
$def->addTag($k, $v);
9588
}

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -102,26 +102,6 @@ private function doResolveDefinition(ChildDefinition $definition)
102102
$def->setLazy($parentDef->isLazy());
103103
$def->setAutowired($parentDef->getAutowired());
104104

105-
self::mergeDefinition($def, $definition);
106-
107-
// merge autowiring types
108-
foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
109-
$def->addAutowiringType($autowiringType);
110-
}
111-
112-
// these attributes are always taken from the child
113-
$def->setAbstract($definition->isAbstract());
114-
$def->setShared($definition->isShared());
115-
$def->setTags($definition->getTags());
116-
117-
return $def;
118-
}
119-
120-
/**
121-
* @internal
122-
*/
123-
public static function mergeDefinition(Definition $def, ChildDefinition $definition)
124-
{
125105
// overwrite with values specified in the decorator
126106
$changes = $definition->getChanges();
127107
if (isset($changes['class'])) {
@@ -182,5 +162,26 @@ public static function mergeDefinition(Definition $def, ChildDefinition $definit
182162
if ($calls = $definition->getMethodCalls()) {
183163
$def->setMethodCalls(array_merge($def->getMethodCalls(), $calls));
184164
}
165+
166+
// merge autowiring types
167+
foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
168+
$def->addAutowiringType($autowiringType);
169+
}
170+
171+
// these attributes are always taken from the child
172+
$def->setAbstract($definition->isAbstract());
173+
$def->setShared($definition->isShared());
174+
$def->setTags($definition->getTags());
175+
176+
// append parent tags when inheriting is enabled
177+
if ($definition->getInheritTags()) {
178+
foreach ($parentDef->getTags() as $k => $v) {
179+
foreach ($v as $v) {
180+
$def->addTag($k, $v);
181+
}
182+
}
183+
}
184+
185+
return $def;
185186
}
186187
}

‎src/Symfony/Component/DependencyInjection/Definition.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class Definition
4242
private $decoratedService;
4343
private $autowired = 0;
4444
private $autowiringTypes = array();
45+
private $configuredParts = array();
4546

4647
protected $arguments;
4748

@@ -70,6 +71,8 @@ public function setFactory($factory)
7071

7172
$this->factory = $factory;
7273

74+
$this->configuredParts['factory'] = true;
75+
7376
return $this;
7477
}
7578

@@ -100,6 +103,8 @@ public function setDecoratedService($id, $renamedId = null, $priority = 0)
100103
throw new InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id));
101104
}
102105

106+
$this->configuredParts['decorated_service'] = true;
107+
103108
if (null === $id) {
104109
$this->decoratedService = null;
105110
} else {
@@ -279,6 +284,7 @@ public function addMethodCall($method, array $arguments = array())
279284
if (empty($method)) {
280285
throw new InvalidArgumentException(sprintf('Method name cannot be empty.'));
281286
}
287+
282288
$this->calls[] = array($method, $arguments);
283289

284290
return $this;
@@ -455,6 +461,7 @@ public function clearTags()
455461
*/
456462
public function setFile($file)
457463
{
464+
$this->configuredParts['file'] = true;
458465
$this->file = $file;
459466

460467
return $this;
@@ -479,6 +486,7 @@ public function getFile()
479486
*/
480487
public function setShared($shared)
481488
{
489+
$this->configuredParts['shared'] = true;
482490
$this->shared = (bool) $shared;
483491

484492
return $this;
@@ -503,6 +511,7 @@ public function isShared()
503511
*/
504512
public function setPublic($boolean)
505513
{
514+
$this->configuredParts['public'] = true;
506515
$this->public = (bool) $boolean;
507516

508517
return $this;
@@ -527,6 +536,7 @@ public function isPublic()
527536
*/
528537
public function setLazy($lazy)
529538
{
539+
$this->configuredParts['lazy'] = true;
530540
$this->lazy = (bool) $lazy;
531541

532542
return $this;
@@ -552,6 +562,7 @@ public function isLazy()
552562
*/
553563
public function setSynthetic($boolean)
554564
{
565+
$this->configuredParts['synthetic'] = true;
555566
$this->synthetic = (bool) $boolean;
556567

557568
return $this;
@@ -578,6 +589,7 @@ public function isSynthetic()
578589
*/
579590
public function setAbstract($boolean)
580591
{
592+
$this->configuredParts['abstract'] = true;
581593
$this->abstract = (bool) $boolean;
582594

583595
return $this;
@@ -619,6 +631,7 @@ public function setDeprecated($status = true, $template = null)
619631
$this->deprecationTemplate = $template;
620632
}
621633

634+
$this->configuredParts['deprecated'] = true;
622635
$this->deprecated = (bool) $status;
623636

624637
return $this;
@@ -660,6 +673,7 @@ public function setConfigurator($configurator)
660673
$configurator = explode('::', $configurator, 2);
661674
}
662675

676+
$this->configuredParts['configurator'] = true;
663677
$this->configurator = $configurator;
664678

665679
return $this;
@@ -731,6 +745,8 @@ public function setAutowired($autowired)
731745
if ($autowired && self::AUTOWIRE_BY_TYPE !== $autowired && self::AUTOWIRE_BY_ID !== $autowired) {
732746
throw new InvalidArgumentException(sprintf('Invalid argument: Definition::AUTOWIRE_BY_TYPE (%d) or Definition::AUTOWIRE_BY_ID (%d) expected, %d given.', self::AUTOWIRE_BY_TYPE, self::AUTOWIRE_BY_ID, $autowired));
733747
}
748+
749+
$this->configuredParts['autowired'] = true;
734750
$this->autowired = $autowired;
735751

736752
return $this;
@@ -803,4 +819,14 @@ public function hasAutowiringType($type)
803819

804820
return isset($this->autowiringTypes[$type]);
805821
}
822+
823+
/**
824+
* Returns all parts of this Definition that were explicitly configured.
825+
*
826+
* @return array An array of configured parts for this Definition
827+
*/
828+
public function getConfiguredParts()
829+
{
830+
return $this->configuredParts;
831+
}
806832
}

‎src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,6 @@ class YamlFileLoader extends FileLoader
8181
'shared' => 'shared',
8282
'lazy' => 'lazy',
8383
'public' => 'public',
84-
'abstract' => 'abstract',
85-
'deprecated' => 'deprecated',
86-
'factory' => 'factory',
87-
'arguments' => 'arguments',
8884
'properties' => 'properties',
8985
'configurator' => 'configurator',
9086
'calls' => 'calls',

‎src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,7 @@
136136

137137
<xsd:complexType name="instanceof">
138138
<xsd:choice maxOccurs="unbounded">
139-
<xsd:element name="argument" type="argument" minOccurs="0" maxOccurs="unbounded" />
140139
<xsd:element name="configurator" type="callable" minOccurs="0" maxOccurs="1" />
141-
<xsd:element name="factory" type="callable" minOccurs="0" maxOccurs="1" />
142-
<xsd:element name="deprecated" type="xsd:string" minOccurs="0" maxOccurs="1" />
143140
<xsd:element name="call" type="call" minOccurs="0" maxOccurs="unbounded" />
144141
<xsd:element name="tag" type="tag" minOccurs="0" maxOccurs="unbounded" />
145142
<xsd:element name="property" type="property" minOccurs="0" maxOccurs="unbounded" />
@@ -148,7 +145,6 @@
148145
<xsd:attribute name="shared" type="boolean" />
149146
<xsd:attribute name="public" type="boolean" />
150147
<xsd:attribute name="lazy" type="boolean" />
151-
<xsd:attribute name="abstract" type="boolean" />
152148
<xsd:attribute name="autowire" type="autowire" />
153149
</xsd:complexType>
154150

0 commit comments

Comments
 (0)
0