Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commitab93fea

Browse files
committed
feature#22295 [BC BREAK][DI] Always autowire "by id" instead of using reflection against all existing services (nicolas-grekas)
This PR was merged into the 3.3-dev branch.Discussion----------[BC BREAK][DI] Always autowire "by id" instead of using reflection against all existing services| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? | no| New feature? | yes| BC breaks? | yes - compile time only| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -(patch best reviewed [ignoring whitespaces](https://github.com/symfony/symfony/pull/22295/files?w=1).)"By-id" autowiring, as introduced in#22060 is free from all the issues that "by-type" autowiring has:- it has no magic and requires explicit type<>id matching (*vs* using reflection on all services to cherry-pick *the* one that matches some type-hint *at that time*, which is fragile)- it is free from any ambiguities (*vs* the Damocles' sword of breaking config just by enabling some unrelated bundle)- it is easily introspected: just look at DI config files (*vs* inspecting the type-hierarchy of all services + their type-hints)- ~~it is side-effect free, thus plain predictable (*vs* auto-registration of discovered types as services)~~- it plays nice with deprecated services or classes (see#22282)- *etc.*Another consideration is that any "by-type" autowired configuration is either broken (because of future ambiguities) - or equivalent to a "by-id" configuration (because resolving ambiguities *means* adding explicit type<>id mappings.) For theoreticians, we could say that "by-id" autowiring is the asymptotic limit of "by-type" autowiring :-)For all these reasons - and also because it reduces the complexity of the code base - I propose to change the behavior and only support "by-id" autowiring in 3.3. This will break some configurations relying on "by-type" autowiring. Yet the break will only happen at compile-time, which means this won't *silently* break any apps. For all core Symfony services, they will work out of the box thanks to#22098 *et al.* For the remaining services, fixing ones config should be pretty straightforward: just follow the suggestions provided by the exception messages. If they are fine to you, you'll end up with the exact same config in the end. And maybe you'll spot issues that were hidden previously.Commits-------cc5e582 [BC BREAK][DI] Always autowire "by id" instead of using reflection against all existing services
2 parentsd33c0ee +cc5e582 commitab93fea

File tree

29 files changed

+212
-345
lines changed

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');
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
protectedfunctionprocessValue($value,$isRoot =false)
7978
{
80-
if ($valueinstanceof TypedReference &&$this->currentDefinition->isAutowired() &&!$this->container->has((string)$value)) {
79+
if ($valueinstanceof TypedReference && !$this->container->has((string)$value)) {
8180
if ($ref =$this->getAutowiredReference($value->getType(),$value->canBeAutoregistered())) {
8281
$value =newTypedReference((string)$ref,$value->getType(),$value->getInvalidBehavior(),$value->canBeAutoregistered());
8382
}else {
@@ -87,45 +86,37 @@ protected function processValue($value, $isRoot = false)
8786
if (!$valueinstanceof Definition) {
8887
returnparent::processValue($value,$isRoot);
8988
}
89+
if (!$value->isAutowired() ||$value->isAbstract() || !$value->getClass()) {
90+
returnparent::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-
returnparent::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-
returnparent::processValue($value,$isRoot);
102-
}
103-
104-
$autowiredMethods =$this->getMethodsToAutowire($reflectionClass);
105-
$methodCalls =$value->getMethodCalls();
95+
returnparent::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-
returnparent::processValue($value,$isRoot);
126-
}finally {
127-
$this->currentDefinition =$parentDefinition;
115+
if ($methodCalls !==$value->getMethodCalls()) {
116+
$value->setMethodCalls($methodCalls);
128117
}
118+
119+
returnparent::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(newDefinition($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-
thrownewRuntimeException(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+
thrownewRuntimeException(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-
privatefunctiongetAutowiredReference($type,$autoRegister =true)
281+
privatefunctiongetAutowiredReference($type,$autoRegister)
293282
{
294283
if ($this->container->has($type) && !$this->container->findDefinition($type)->isAbstract()) {
295284
returnnewReference($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] ?newReference($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
returnnewReference($this->types[$type]);
314297
}
315298

316-
if (isset($this->ambiguousServiceTypes[$type])) {
317-
$classOrInterface =class_exists($type,false) ?'class' :'interface';
318-
319-
thrownewRuntimeException(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 =newDefinition($type);
397+
$argumentDefinition =newDefinition($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
privatefunctioncreateTypeNotFoundMessage($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-
returnsprintf('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-
returnsprintf('Cannot autowire service "%s": %s',$this->currentId,$message);
427+
returnsprintf('Cannot autowire service "%s": %s %s',$this->currentId,$label,$message);
473428
}
474429

475430
privatefunctioncreateTypeAlternatives($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+
thrownewInvalidArgumentException(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] =newReference($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
returnparent::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();

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp