Skip to content

Commit 3fa8a05

Browse files
fabpotfabpot
committed
feature #21763 [DI] Replace wildcard-based methods autowiring by @required annotation (nicolas-grekas)
This PR was squashed before being merged into the 3.3-dev branch (closes #21763). Discussion ---------- [DI] Replace wildcard-based methods autowiring by `@required` annotation | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no (affects things that are only on master) | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - While playing a bit with new features in master around DI configuration, several people around me got bitten by wildcard-based autowiring. The typical example is adding `autowire: [set*]` in `_defaults`: use that on `resource: ../src/Command/` PSR4-based loading and boom, `setApplication` and `setHelperSet` will now be wrongly called. You could tell me "of course, don't to that" - but being bitten so early on a master-only feature makes me really unconfident that this will be easy enough for people after the release. If wildcard-based autowiring is removed, then I don't see anymore the need for allowing arrays as in `autowire: [setFoo,getBar]`. Moreover, this array syntax has a core DX issue: it's a dead end as far as the learning curve is concerned. You learn it, then when becoming a more advanced dev, someone teaches you that you'd better use another syntax: explicit wiring. And in fact, we don't need it at all, because something else already exists: just declare a method call, but don't define its arguments. If `autowire: true` is set, then the AutowiringPass already fills in the holes. There is only one tweak required to make this work: don't autowire optional arguments for method calls - or that'd be a BC break. To my PoV that's even better: this makes autowiring fit a "do the minimum to make it work" strategy. A really good one to me. But there is still an issue: wildcard-based autowiring fits a need. Namely, it allows one to define a convention (eg. `'set*'`), and have all such methods that follow the convention be autowired. To me, this looks like doing it reverse (the DI config should adapt to the code, not reverse). So, to fill this need, let the declaration be in the source: just use an annotation! This PR adds support for the `@required` annotation, borrowed from the Spring framework: https://www.tutorialspoint.com/spring/spring_required_annotation.htm Using the annotation is totally optional of course. If you do, *and if autowiring is on*, then it'll be autowired. If you don't, nothing changes: do manual wiring. Even when not using autowiring, the annotation is still a nice hint for the consumer of your classes: it tells the reader that this method needs to be called for correct instantiation - thus lowering one drawback of setter injection (discoverability). The implementation of the annotation parsing is done using a few regexp (no dep on any complex parser) - and works with inheritance, by leveraging the `@inheritdoc` tag (the default behavior being to *not* inherit anything from parent methods). All in all, looking at the diff stats, it makes everything simpler. Good sign, isn't it? Commits ------- f286fcc [DI] Replace wildcard-based methods autowiring by `@required` annotation 9081699 Revert "minor #21315 [DI][FrameworkBundle] Show autowired methods in descriptors (ogizanagi)"
2 parents 8d99d57 + f286fcc commit 3fa8a05

File tree

69 files changed

+164
-665
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+164
-665
lines changed

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,9 @@ private function getContainerDefinitionData(Definition $definition, $omitTags =
220220
'lazy' => $definition->isLazy(),
221221
'shared' => $definition->isShared(),
222222
'abstract' => $definition->isAbstract(),
223+
'autowire' => $definition->isAutowired(),
223224
);
224225

225-
$autowiredCalls = array_values(array_filter($definition->getAutowiredCalls(), function ($method) {
226-
return $method !== '__construct';
227-
}));
228-
$data['autowire'] = $definition->isAutowired() ? ($autowiredCalls ?: true) : false;
229-
230226
foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
231227
$data['autowiring_types'][] = $autowiringType;
232228
}

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -182,16 +182,9 @@ 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() ? 'yes' : 'no')
185186
;
186187

187-
$autowiredCalls = array_filter($definition->getAutowiredCalls(), function ($method) {
188-
return $method !== '__construct';
189-
});
190-
$output .= "\n".'- Autowire: ';
191-
$output .= $definition->isAutowired() ? ($autowiredCalls ? implode(', ', array_map(function ($method) {
192-
return "`$method`";
193-
}, $autowiredCalls)) : 'yes') : 'no';
194-
195188
foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
196189
$output .= "\n".'- Autowiring Type: `'.$autowiringType.'`';
197190
}

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -295,11 +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-
299-
$autowiredCalls = array_filter($definition->getAutowiredCalls(), function ($method) {
300-
return $method !== '__construct';
301-
});
302-
$tableRows[] = array('Autowire', $definition->isAutowired() ? ($autowiredCalls ? implode("\n", $autowiredCalls) : 'yes') : 'no');
298+
$tableRows[] = array('Autowired', $definition->isAutowired() ? 'yes' : 'no');
303299

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

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -374,18 +374,6 @@ private function getContainerDefinitionDocument(Definition $definition, $id = nu
374374
$serviceXML->setAttribute('autowired', $definition->isAutowired() ? 'true' : 'false');
375375
$serviceXML->setAttribute('file', $definition->getFile());
376376

377-
$autowiredCalls = array_filter($definition->getAutowiredCalls(), function ($method) {
378-
return $method !== '__construct';
379-
});
380-
if ($autowiredCalls) {
381-
$serviceXML->appendChild($autowiredMethodsXML = $dom->createElement('autowired-calls'));
382-
foreach ($autowiredCalls as $autowiredMethod) {
383-
$autowiredMethodXML = $dom->createElement('autowired-call');
384-
$autowiredMethodXML->appendChild(new \DOMText($autowiredMethod));
385-
$autowiredMethodsXML->appendChild($autowiredMethodXML);
386-
}
387-
}
388-
389377
$calls = $definition->getMethodCalls();
390378
if (count($calls) > 0) {
391379
$serviceXML->appendChild($callsXML = $dom->createElement('calls'));

‎src/Symfony/Bundle/FrameworkBundle/Tests/Console/Descriptor/ObjectsProvider.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,6 @@ public static function getContainerDefinitions()
137137
->addTag('tag2')
138138
->addMethodCall('setMailer', array(new Reference('mailer')))
139139
->setFactory(array(new Reference('factory.service'), 'get')),
140-
'definition_autowired' => (new Definition('AutowiredService'))->setAutowired(true),
141-
'definition_autowired_with_methods' => (new Definition('AutowiredService'))
142-
->setAutowiredCalls(array('__construct', 'set*', 'addFoo')),
143140
);
144141
}
145142

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/alias_with_definition_1.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111
- Lazy: yes
1212
- Shared: yes
1313
- Abstract: yes
14-
- Autowire: no
14+
- Autowired: no
1515
- Factory Class: `Full\Qualified\FactoryClass`
1616
- Factory Method: `get`

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/alias_with_definition_1.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
Lazy yes
1515
Shared yes
1616
Abstract yes
17-
Autowire no
17+
Autowired no
1818
Factory Class Full\Qualified\FactoryClass
1919
Factory Method get
2020
---------------- -----------------------------

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/alias_with_definition_2.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
- Lazy: no
1212
- Shared: yes
1313
- Abstract: no
14-
- Autowire: no
14+
- Autowired: no
1515
- File: `/path/to/file`
1616
- Factory Service: `factory.service`
1717
- Factory Method: `get`

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/alias_with_definition_2.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
Lazy no
1818
Shared yes
1919
Abstract no
20-
Autowire no
20+
Autowired no
2121
Required File /path/to/file
2222
Factory Service factory.service
2323
Factory Method get

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_arguments.json

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -80,33 +80,6 @@
8080
"factory_class": "Full\\Qualified\\FactoryClass",
8181
"factory_method": "get",
8282
"tags": []
83-
},
84-
"definition_autowired": {
85-
"class": "AutowiredService",
86-
"public": true,
87-
"synthetic": false,
88-
"lazy": false,
89-
"shared": true,
90-
"abstract": false,
91-
"autowire": true,
92-
"arguments": [],
93-
"file": null,
94-
"tags": []
95-
},
96-
"definition_autowired_with_methods": {
97-
"class": "AutowiredService",
98-
"public": true,
99-
"synthetic": false,
100-
"lazy": false,
101-
"shared": true,
102-
"abstract": false,
103-
"autowire": [
104-
"set*",
105-
"addFoo"
106-
],
107-
"arguments": [],
108-
"file": null,
109-
"tags": []
11083
}
11184
},
11285
"aliases": {

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_arguments.md

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,33 +12,11 @@ Definitions
1212
- Lazy: yes
1313
- Shared: yes
1414
- Abstract: yes
15-
- Autowire: no
15+
- Autowired: no
1616
- Arguments: yes
1717
- Factory Class: `Full\Qualified\FactoryClass`
1818
- Factory Method: `get`
1919

20-
### definition_autowired
21-
22-
- Class: `AutowiredService`
23-
- Public: yes
24-
- Synthetic: no
25-
- Lazy: no
26-
- Shared: yes
27-
- Abstract: no
28-
- Autowire: yes
29-
- Arguments: no
30-
31-
### definition_autowired_with_methods
32-
33-
- Class: `AutowiredService`
34-
- Public: yes
35-
- Synthetic: no
36-
- Lazy: no
37-
- Shared: yes
38-
- Abstract: no
39-
- Autowire: `set*`, `addFoo`
40-
- Arguments: no
41-
4220

4321
Aliases
4422
-------

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_arguments.txt

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,12 @@
22
Symfony Container Public Services
33
=================================
44

5-
----------------------------------- --------------------------------------------------------
6-
 Service ID   Class name 
7-
----------------------------------- --------------------------------------------------------
8-
alias_1 alias for "service_1"
9-
alias_2 alias for "service_2"
10-
definition_1 Full\Qualified\Class1
11-
definition_autowired AutowiredService
12-
definition_autowired_with_methods AutowiredService
13-
service_container Symfony\Component\DependencyInjection\ContainerBuilder
14-
----------------------------------- --------------------------------------------------------
5+
------------------- --------------------------------------------------------
6+
 Service ID   Class name 
7+
------------------- --------------------------------------------------------
8+
alias_1 alias for "service_1"
9+
alias_2 alias for "service_2"
10+
definition_1 Full\Qualified\Class1
11+
service_container Symfony\Component\DependencyInjection\ContainerBuilder
12+
------------------- --------------------------------------------------------
1513

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_arguments.xml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,5 @@
2929
<argument key="def2" type="service" id="definition_2"/>
3030
</argument>
3131
</definition>
32-
<definition id="definition_autowired" class="AutowiredService" public="true" synthetic="false" lazy="false" shared="true" abstract="false" autowired="true" file=""/>
33-
<definition id="definition_autowired_with_methods" class="AutowiredService" public="true" synthetic="false" lazy="false" shared="true" abstract="false" autowired="true" file="">
34-
<autowired-calls>
35-
<autowired-call>set*</autowired-call>
36-
<autowired-call>addFoo</autowired-call>
37-
</autowired-calls>
38-
</definition>
3932
<service id="service_container" class="Symfony\Component\DependencyInjection\ContainerBuilder"/>
4033
</container>

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_public.json

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,31 +12,6 @@
1212
"factory_class": "Full\\Qualified\\FactoryClass",
1313
"factory_method": "get",
1414
"tags": []
15-
},
16-
"definition_autowired": {
17-
"class": "AutowiredService",
18-
"public": true,
19-
"synthetic": false,
20-
"lazy": false,
21-
"shared": true,
22-
"abstract": false,
23-
"autowire": true,
24-
"file": null,
25-
"tags": []
26-
},
27-
"definition_autowired_with_methods": {
28-
"class": "AutowiredService",
29-
"public": true,
30-
"synthetic": false,
31-
"lazy": false,
32-
"shared": true,
33-
"abstract": false,
34-
"autowire": [
35-
"set*",
36-
"addFoo"
37-
],
38-
"file": null,
39-
"tags": []
4015
}
4116
},
4217
"aliases": {

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_public.md

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,10 @@ Definitions
1212
- Lazy: yes
1313
- Shared: yes
1414
- Abstract: yes
15-
- Autowire: no
15+
- Autowired: no
1616
- Factory Class: `Full\Qualified\FactoryClass`
1717
- Factory Method: `get`
1818

19-
### definition_autowired
20-
21-
- Class: `AutowiredService`
22-
- Public: yes
23-
- Synthetic: no
24-
- Lazy: no
25-
- Shared: yes
26-
- Abstract: no
27-
- Autowire: yes
28-
29-
### definition_autowired_with_methods
30-
31-
- Class: `AutowiredService`
32-
- Public: yes
33-
- Synthetic: no
34-
- Lazy: no
35-
- Shared: yes
36-
- Abstract: no
37-
- Autowire: `set*`, `addFoo`
38-
3919

4020
Aliases
4121
-------

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_public.txt

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,12 @@
22
Symfony Container Public Services
33
=================================
44

5-
----------------------------------- --------------------------------------------------------
6-
 Service ID   Class name 
7-
----------------------------------- --------------------------------------------------------
8-
alias_1 alias for "service_1"
9-
alias_2 alias for "service_2"
10-
definition_1 Full\Qualified\Class1
11-
definition_autowired AutowiredService
12-
definition_autowired_with_methods AutowiredService
13-
service_container Symfony\Component\DependencyInjection\ContainerBuilder
14-
----------------------------------- --------------------------------------------------------
5+
------------------- --------------------------------------------------------
6+
 Service ID   Class name 
7+
------------------- --------------------------------------------------------
8+
alias_1 alias for "service_1"
9+
alias_2 alias for "service_2"
10+
definition_1 Full\Qualified\Class1
11+
service_container Symfony\Component\DependencyInjection\ContainerBuilder
12+
------------------- --------------------------------------------------------
1513

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_public.xml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,5 @@
55
<definition id="definition_1" class="Full\Qualified\Class1" public="true" synthetic="false" lazy="true" shared="true" abstract="true" autowired="false" file="">
66
<factory class="Full\Qualified\FactoryClass" method="get"/>
77
</definition>
8-
<definition id="definition_autowired" class="AutowiredService" public="true" synthetic="false" lazy="false" shared="true" abstract="false" autowired="true" file=""/>
9-
<definition id="definition_autowired_with_methods" class="AutowiredService" public="true" synthetic="false" lazy="false" shared="true" abstract="false" autowired="true" file="">
10-
<autowired-calls>
11-
<autowired-call>set*</autowired-call>
12-
<autowired-call>addFoo</autowired-call>
13-
</autowired-calls>
14-
</definition>
158
<service id="service_container" class="Symfony\Component\DependencyInjection\ContainerBuilder"/>
169
</container>

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_services.json

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -46,31 +46,6 @@
4646
"parameters": []
4747
}
4848
]
49-
},
50-
"definition_autowired": {
51-
"class": "AutowiredService",
52-
"public": true,
53-
"synthetic": false,
54-
"lazy": false,
55-
"shared": true,
56-
"abstract": false,
57-
"autowire": true,
58-
"file": null,
59-
"tags": []
60-
},
61-
"definition_autowired_with_methods": {
62-
"class": "AutowiredService",
63-
"public": true,
64-
"synthetic": false,
65-
"lazy": false,
66-
"shared": true,
67-
"abstract": false,
68-
"autowire": [
69-
"set*",
70-
"addFoo"
71-
],
72-
"file": null,
73-
"tags": []
7449
}
7550
},
7651
"aliases": {

‎src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_1_services.md

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Definitions
1212
- Lazy: yes
1313
- Shared: yes
1414
- Abstract: yes
15-
- Autowire: no
15+
- Autowired: no
1616
- Factory Class: `Full\Qualified\FactoryClass`
1717
- Factory Method: `get`
1818

@@ -24,7 +24,7 @@ Definitions
2424
- Lazy: no
2525
- Shared: yes
2626
- Abstract: no
27-
- Autowire: no
27+
- Autowired: no
2828
- File: `/path/to/file`
2929
- Factory Service: `factory.service`
3030
- Factory Method: `get`
@@ -36,26 +36,6 @@ Definitions
3636
- Attr3: val3
3737
- Tag: `tag2`
3838

39-
### definition_autowired
40-
41-
- Class: `AutowiredService`
42-
- Public: yes
43-
- Synthetic: no
44-
- Lazy: no
45-
- Shared: yes
46-
- Abstract: no
47-
- Autowire: yes
48-
49-
### definition_autowired_with_methods
50-
51-
- Class: `AutowiredService`
52-
- Public: yes
53-
- Synthetic: no
54-
- Lazy: no
55-
- Shared: yes
56-
- Abstract: no
57-
- Autowire: `set*`, `addFoo`
58-
5939

6040
Aliases
6141
-------
@@ -74,4 +54,4 @@ Aliases
7454
Services
7555
--------
7656

77-
- `service_container`: `Symfony\Component\DependencyInjection\ContainerBuilder`
57+
- `service_container`: `Symfony\Component\DependencyInjection\ContainerBuilder`

0 commit comments

Comments
 (0)