8000 [BC BREAK][DI] Always autowire "by id" instead of using reflection ag… · symfony/symfony@cc5e582 · GitHub
[go: up one dir, main page]
Skip to content

Commit cc5e582

Browse 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

29 files changed

+212
-345
lines changed

‎UPGRADE-3.3.md

Lines 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

Lines 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

Lines 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

Lines 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

Lines 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');
< 1C6A code>299299

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

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

Lines 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

Lines 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

Lines 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-> 10000 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

Lines 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

Lines 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

Lines 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