Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit cc5e582

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[BC BREAK][DI] Always autowire "by id" instead of using reflection against all existing services
1 parent 12bb392 commit cc5e582
Copy full SHA for cc5e582

29 files changed

+212
-345
lines changed

‎UPGRADE-3.3.md

Copy file name to clipboardExpand all lines: UPGRADE-3.3.md
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ Debug
8080
DependencyInjection
8181
-------------------
8282

83+
* [BC BREAK] autowiring now happens only when a type-hint matches its corresponding FQCN id or alias. Please follow the suggestions provided by the exceptions thrown at compilation to upgrade your service configuration.
84+
8385
* [BC BREAK] `_defaults` and `_instanceof` are now reserved service names in Yaml configurations. Please rename any services with that names.
8486

8587
* [BC BREAK] non-numeric keys in methods and constructors arguments have never been supported and are now forbidden. Please remove them if you happen to have one.

‎UPGRADE-4.0.md

Copy file name to clipboardExpand all lines: UPGRADE-4.0.md
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ Debug
7373
DependencyInjection
7474
-------------------
7575

76+
* Autowiring now happens only when a type-hint matches its corresponding FQCN id or alias.
77+
7678
* `_defaults` and `_instanceof` are now reserved service names in Yaml configurations. Please rename any services with that names.
7779

7880
* Non-numeric keys in methods and constructors arguments have never been supported and are now forbidden. Please remove them if you happen to have one.

‎src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ private function getContainerDefinitionData(Definition $definition, $omitTags =
221221
'lazy' => $definition->isLazy(),
222222
'shared' => $definition->isShared(),
223223
'abstract' => $definition->isAbstract(),
224-
'autowire' => $definition->isAutowired() ? (Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id') : false,
224+
'autowire' => $definition->isAutowired(),
225225
);
226226

227227
foreach ($definition->getAutowiringTypes(false) as $autowiringType) {

‎src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ protected function describeContainerDefinition(Definition $definition, array $op
182182
."\n".'- Lazy: '.($definition->isLazy() ? 'yes' : 'no')
183183
."\n".'- Shared: '.($definition->isShared() ? 'yes' : 'no')
184184
."\n".'- Abstract: '.($definition->isAbstract() ? 'yes' : 'no')
185-
."\n".'- Autowired: '.($definition->isAutowired() ? (Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id') : 'no')
185+
."\n".'- Autowired: '.($definition->isAutowired() ? 'yes' : 'no')
186186
;
187187

188188
foreach ($definition->getAutowiringTypes(false) as $autowiringType) {

‎src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ protected function describeContainerDefinition(Definition $definition, array $op
295295
$tableRows[] = array('Lazy', $definition->isLazy() ? 'yes' : 'no');
296296
$tableRows[] = array('Shared', $definition->isShared() ? 'yes' : 'no');
297297
$tableRows[] = array('Abstract', $definition->isAbstract() ? 'yes' : 'no');
298-
$tableRows[] = array('Autowired', $definition->isAutowired() ? (Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id') : 'no');
298+
$tableRows[] = array('Autowired', $definition->isAutowired() ? 'yes' : 'no');
299299

300300
if ($autowiringTypes = $definition->getAutowiringTypes(false)) {
301301
$tableRows[] = array('Autowiring Types', implode(', ', $autowiringTypes));

‎src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ private function getContainerDefinitionDocument(Definition $definition, $id = nu
371371
$serviceXML->setAttribute('lazy', $definition->isLazy() ? 'true' : 'false');
372372
$serviceXML->setAttribute('shared', $definition->isShared() ? 'true' : 'false');
373373
$serviceXML->setAttribute('abstract', $definition->isAbstract() ? 'true' : 'false');
374-
$serviceXML->setAttribute('autowired', $definition->isAutowired() ? (Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id') : 'false');
374+
$serviceXML->setAttribute('autowired', $definition->isAutowired() ? 'true' : 'false');
375375
$serviceXML->setAttribute('file', $definition->getFile());
376376

377377
$calls = $definition->getMethodCalls();

‎src/Symfony/Component/DependencyInjection/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/CHANGELOG.md
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ CHANGELOG
44
3.3.0
55
-----
66

7+
* [BC BREAK] autowiring now happens only when a type-hint matches its corresponding FQCN id or alias.
8+
Please follow the suggestions provided by the exceptions thrown at compilation to upgrade your service configuration.
79
* added "ServiceSubscriberInterface" - to allow for per-class explicit service-locator definitions
810
* added "container.service_locator" tag for defining service-locator services
911
* added anonymous services support in YAML configuration files using the `!service` tag.

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+38-83Lines changed: 38 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ class AutowirePass extends AbstractRecursivePass
3131
private $types;
3232
private $ambiguousServiceTypes = array();
3333
private $autowired = array();
34-
private $currentDefinition;
3534

3635
/**
3736
* {@inheritdoc}
@@ -77,7 +76,7 @@ public static function createResourceForClass(\ReflectionClass $reflectionClass)
7776
*/
7877
protected function processValue($value, $isRoot = false)
7978
{
80-
if ($value instanceof TypedReference && $this->currentDefinition->isAutowired() && !$this->container->has((string) $value)) {
79+
if ($value instanceof TypedReference && !$this->container->has((string) $value)) {
8180
if ($ref = $this->getAutowiredReference($value->getType(), $value->canBeAutoregistered())) {
8281
$value = new TypedReference((string) $ref, $value->getType(), $value->getInvalidBehavior(), $value->canBeAutoregistered());
8382
} else {
@@ -87,45 +86,37 @@ protected function processValue($value, $isRoot = false)
8786
if (!$value instanceof Definition) {
8887
return parent::processValue($value, $isRoot);
8988
}
89+
if (!$value->isAutowired() || $value->isAbstract() || !$value->getClass()) {
90+
return parent::processValue($value, $isRoot);
91+
}
92+
if (!$reflectionClass = $this->container->getReflectionClass($value->getClass())) {
93+
$this->container->log($this, sprintf('Skipping service "%s": Class or interface "%s" does not exist.', $this->currentId, $value->getClass()));
9094

91-
$parentDefinition = $this->currentDefinition;
92-
$this->currentDefinition = $value;
93-
94-
try {
95-
if (!$value->isAutowired() || $value->isAbstract() || !$value->getClass()) {
96-
return parent::processValue($value, $isRoot);
97-
}
98-
if (!$reflectionClass = $this->container->getReflectionClass($value->getClass())) {
99-
$this->container->log($this, sprintf('Skipping service "%s": Class or interface "%s" does not exist.', $this->currentId, $value->getClass()));
100-
101-
return parent::processValue($value, $isRoot);
102-
}
103-
104-
$autowiredMethods = $this->getMethodsToAutowire($reflectionClass);
105-
$methodCalls = $value->getMethodCalls();
95+
return parent::processValue($value, $isRoot);
96+
}
10697

107-
if ($constructor = $this->getConstructor($value, false)) {
108-
array_unshift($methodCalls, array($constructor, $value->getArguments()));
109-
}
98+
$autowiredMethods = $this->getMethodsToAutowire($reflectionClass);
99+
$methodCalls = $value->getMethodCalls();
110100

111-
$methodCalls = $this->autowireCalls($reflectionClass, $methodCalls, $autowiredMethods);
101+
if ($constructor = $this->getConstructor($value, false)) {
102+
array_unshift($methodCalls, array($constructor, $value->getArguments()));
103+
}
112104

113-
if ($constructor) {
114-
list(, $arguments) = array_shift($methodCalls);
105+
$methodCalls = $this->autowireCalls($reflectionClass, $methodCalls, $autowiredMethods);
115106

116-
if ($arguments !== $value->getArguments()) {
117-
$value->setArguments($arguments);
118-
}
119-
}
107+
if ($constructor) {
108+
list(, $arguments) = array_shift($methodCalls);
120109

121-
if ($methodCalls !== $value->getMethodCalls()) {
122-
$value->setMethodCalls($methodCalls);
110+
if ($arguments !== $value->getArguments()) {
111+
$value->setArguments($arguments);
123112
}
113+
}
124114

125-
return parent::processValue($value, $isRoot);
126-
} finally {
127-
$this->currentDefinition = $parentDefinition;
115+
if ($methodCalls !== $value->getMethodCalls()) {
116+
$value->setMethodCalls($methodCalls);
128117
}
118+
119+
return parent::processValue($value, $isRoot);
129120
}
130121

131122
/**
@@ -186,7 +177,7 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
186177
$reflectionMethod = $autowiredMethods[$lcMethod];
187178
unset($autowiredMethods[$lcMethod]);
188179
} else {
189-
$reflectionMethod = $this->getReflectionMethod($this->currentDefinition, $method);
180+
$reflectionMethod = $this->getReflectionMethod(new Definition($reflectionClass->name), $method);
190181
}
191182

192183
$arguments = $this->autowireMethod($reflectionMethod, $arguments);
@@ -242,7 +233,7 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
242233

243234
// no default value? Then fail
244235
if (!$parameter->isDefaultValueAvailable()) {
245-
throw new RuntimeException(sprintf('Cannot autowire service "%s": argument $%s of method %s() must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
236+
throw new RuntimeException(sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
246237
}
247238

248239
// specifically pass the default value
@@ -251,8 +242,8 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
251242
continue;
252243
}
253244

254-
if (!$value = $this->getAutowiredReference($type)) {
255-
$failureMessage = $this->createTypeNotFoundMessage($type, sprintf('argument $%s of method %s()', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
245+
if (!$value = $this->getAutowiredReference($type, !$parameter->isOptional())) {
246+
$failureMessage = $this->createTypeNotFoundMessage($type, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
256247

257248
if ($parameter->isDefaultValueAvailable()) {
258249
$value = $parameter->getDefaultValue();
@@ -286,19 +277,13 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
286277

287278
/**
288279
* @return Reference|null A reference to the service matching the given type, if any
289-
*
290-
* @throws RuntimeException
291280
*/
292-
private function getAutowiredReference($type, $autoRegister = true)
281+
private function getAutowiredReference($type, $autoRegister)
293282
{
294283
if ($this->container->has($type) && !$this->container->findDefinition($type)->isAbstract()) {
295284
return new Reference($type);
296285
}
297286

298-
if (Definition::AUTOWIRE_BY_ID === $this->currentDefinition->getAutowired()) {
299-
return;
300-
}
301-
302287
if (isset($this->autowired[$type])) {
303288
return $this->autowired[$type] ? new Reference($this->autowired[$type]) : null;
304289
}
@@ -307,19 +292,11 @@ private function getAutowiredReference($type, $autoRegister = true)
307292
$this->populateAvailableTypes();
308293
}
309294

310-
if (isset($this->types[$type])) {
311-
$this->container->log($this, sprintf('Service "%s" matches type "%s" and has been autowired into service "%s".', $this->types[$type], $type, $this->currentId));
312-
295+
if (isset($this->definedTypes[$type])) {
313296
return new Reference($this->types[$type]);
314297
}
315298

316-
if (isset($this->ambiguousServiceTypes[$type])) {
317-
$classOrInterface = class_exists($type, false) ? 'class' : 'interface';
318-
319-
throw new RuntimeException(sprintf('Cannot autowire service "%s": multiple candidate services exist for %s "%s".%s', $this->currentId, $classOrInterface, $type, $this->createTypeAlternatives($type)));
320-
}
321-
322-
if ($autoRegister) {
299+
if ($autoRegister && !isset($this->types[$type]) && !isset($this->ambiguousServiceTypes[$type])) {
323300
return $this->createAutowiredDefinition($type);
324301
}
325302
}
@@ -355,22 +332,8 @@ private function populateAvailableType($id, Definition $definition)
355332
unset($this->ambiguousServiceTypes[$type]);
356333
}
357334

358-
if ($deprecated = $definition->isDeprecated()) {
359-
$prevErrorHandler = set_error_handler(function ($level, $message, $file, $line) use (&$prevErrorHandler) {
360-
return (E_USER_DEPRECATED === $level || !$prevErrorHandler) ? false : $prevErrorHandler($level, $message, $file, $line);
361-
});
362-
}
363-
364-
$e = null;
365-
366-
try {
367-
if (!$reflectionClass = $this->container->getReflectionClass($definition->getClass(), true)) {
368-
return;
369-
}
370-
} finally {
371-
if ($deprecated) {
372-
restore_error_handler();
373-
}
335+
if ($definition->isDeprecated() || !$reflectionClass = $this->container->getReflectionClass($definition->getClass(), true)) {
336+
return;
374337
}
375338

376339
foreach ($reflectionClass->getInterfaces() as $reflectionInterface) {
@@ -429,10 +392,9 @@ private function createAutowiredDefinition($type)
429392
return;
430393
}
431394

432-
$currentDefinition = $this->currentDefinition;
433395
$currentId = $this->currentId;
434396
$this->currentId = $this->autowired[$type] = $argumentId = sprintf('autowired.%s', $type);
435-
$this->currentDefinition = $argumentDefinition = new Definition($type);
397+
$argumentDefinition = new Definition($type);
436398
$argumentDefinition->setPublic(false);
437399
$argumentDefinition->setAutowired(true);
438400

@@ -446,7 +408,6 @@ private function createAutowiredDefinition($type)
446408
return;
447409
} finally {
448410
$this->currentId = $currentId;
449-
$this->currentDefinition = $currentDefinition;
450411
}
451412

452413
$this->container->log($this, sprintf('Type "%s" has been auto-registered for service "%s".', $type, $this->currentId));
@@ -456,20 +417,14 @@ private function createAutowiredDefinition($type)
456417

457418
private function createTypeNotFoundMessage($type, $label)
458419
{
459-
$autowireById = Definition::AUTOWIRE_BY_ID === $this->currentDefinition->getAutowired();
460-
if (!$classOrInterface = class_exists($type, $autowireById) ? 'class' : (interface_exists($type, false) ? 'interface' : null)) {
461-
return sprintf('Cannot autowire service "%s": %s has type "%s" but this class does not exist.', $this->currentId, $label, $type);
462-
}
463-
if (null === $this->types) {
464-
$this->populateAvailableTypes();
465-
}
466-
if ($autowireById) {
467-
$message = sprintf('%s references %s "%s" but no such service exists.%s', $label, $classOrInterface, $type, $this->createTypeAlternatives($type));
420+
if (!$r = $this->container->getReflectionClass($type, true)) {
421+
$message = sprintf('has type "%s" but this class does not exist.', $type);
468422
} else {
469-
$message = sprintf('no services were found matching the "%s" %s and it cannot be auto-registered for %s.', $type, $classOrInterface, $label);
423+
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
424+
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $this->createTypeAlternatives($type));
470425
}
471426

472-
return sprintf('Cannot autowire service "%s": %s', $this->currentId, $message);
427+
return sprintf('Cannot autowire service "%s": %s %s', $this->currentId, $label, $message);
473428
}
474429

475430
private function createTypeAlternatives($type)

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@ protected function processValue($value, $isRoot = false)
3838
}
3939

4040
$serviceMap = array();
41+
$autowire = $value->isAutowired();
4142

4243
foreach ($value->getTag('container.service_subscriber') as $attributes) {
4344
if (!$attributes) {
45+
$autowire = true;
4446
continue;
4547
}
4648
ksort($attributes);
@@ -82,6 +84,9 @@ protected function processValue($value, $isRoot = false)
8284
$key = $type;
8385
}
8486
if (!isset($serviceMap[$key])) {
87+
if (!$autowire) {
88+
throw new InvalidArgumentException(sprintf('Service "%s" misses a "container.service_subscriber" tag with "key"/"id" attributes corresponding to entry "%s" as returned by %s::getSubscribedServices().', $this->currentId, $key, $class));
89+
}
8590
$serviceMap[$key] = new Reference($type);
8691
}
8792

@@ -95,7 +100,7 @@ protected function processValue($value, $isRoot = false)
95100
}
96101

97102
$serviceLocator = $this->serviceLocator;
98-
$this->serviceLocator = (string) ServiceLocatorTagPass::register($this->container, $subscriberMap, $value->getAutowired());
103+
$this->serviceLocator = (string) ServiceLocatorTagPass::register($this->container, $subscriberMap);
99104

100105
try {
101106
return parent::processValue($value);

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ private function doResolveDefinition(ChildDefinition $definition)
100100
$def->setFile($parentDef->getFile());
101101
$def->setPublic($parentDef->isPublic());
102102
$def->setLazy($parentDef->isLazy());
103-
$def->setAutowired($parentDef->getAutowired());
103+
$def->setAutowired($parentDef->isAutowired());
104104

105105
self::mergeDefinition($def, $definition);
106106

@@ -146,7 +146,7 @@ public static function mergeDefinition(Definition $def, ChildDefinition $definit
146146
$def->setDeprecated($definition->isDeprecated(), $definition->getDeprecationMessage('%service_id%'));
147147
}
148148
if (isset($changes['autowired'])) {
149-
$def->setAutowired($definition->getAutowired());
149+
$def->setAutowired($definition->isAutowired());
150150
}
151151
if (isset($changes['decorated_service'])) {
152152
$decoratedService = $definition->getDecoratedService();

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/ServiceLocatorTagPass.php
+1-3Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,10 @@ protected function processValue($value, $isRoot = false)
7575
/**
7676
* @param ContainerBuilder $container
7777
* @param Reference[] $refMap
78-
* @param int|bool $autowired
7978
*
8079
* @return Reference
8180
*/
82-
public static function register(ContainerBuilder $container, array $refMap, $autowired = false)
81+
public static function register(ContainerBuilder $container, array $refMap)
8382
{
8483
foreach ($refMap as $id => $ref) {
8584
$refMap[$id] = new ServiceClosureArgument($ref);
@@ -89,7 +88,6 @@ public static function register(ContainerBuilder $container, array $refMap, $aut
8988
$locator = (new Definition(ServiceLocator::class))
9089
->addArgument($refMap)
9190
->setPublic(false)
92-
->setAutowired($autowired)
9391
->addTag('container.service_locator');
9492

9593
if (!$container->has($id = 'service_locator.'.md5(serialize($locator)))) {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.