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

f286fcc25f [DI] Replace wildcard-based methods autowiring by `@required` annotation
9081699980 Revert "minor #21315 [DI][FrameworkBundle] Show autowired methods in descriptors (ogizanagi)"

Author: fabpot

Console/Descriptor/JsonDescriptor.php

@@ -220,13 +220,9 @@ private function getContainerDefinitionData(Definition $definition, $omitTags =
             'lazy' => $definition->isLazy(),
             'shared' => $definition->isShared(),
             'abstract' => $definition->isAbstract(),
+            'autowire' => $definition->isAutowired(),
         );
 
-        $autowiredCalls = array_values(array_filter($definition->getAutowiredCalls(), function ($method) {
-            return $method !== '__construct';
-        }));
-        $data['autowire'] = $definition->isAutowired() ? ($autowiredCalls ?: true) : false;
-
         foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
             $data['autowiring_types'][] = $autowiringType;
         }

Console/Descriptor/MarkdownDescriptor.php

@@ -182,16 +182,9 @@ protected function describeContainerDefinition(Definition $definition, array $op
             ."\n".'- Lazy: '.($definition->isLazy() ? 'yes' : 'no')
             ."\n".'- Shared: '.($definition->isShared() ? 'yes' : 'no')
             ."\n".'- Abstract: '.($definition->isAbstract() ? 'yes' : 'no')
+            ."\n".'- Autowired: '.($definition->isAutowired() ? 'yes' : 'no')
         ;
 
-        $autowiredCalls = array_filter($definition->getAutowiredCalls(), function ($method) {
-            return $method !== '__construct';
-        });
-        $output .= "\n".'- Autowire: ';
-        $output .= $definition->isAutowired() ? ($autowiredCalls ? implode(', ', array_map(function ($method) {
-            return "`$method`";
-        }, $autowiredCalls)) : 'yes') : 'no';
-
         foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
             $output .= "\n".'- Autowiring Type: `'.$autowiringType.'`';
         }

Console/Descriptor/TextDescriptor.php

@@ -295,11 +295,7 @@ protected function describeContainerDefinition(Definition $definition, array $op
         $tableRows[] = array('Lazy', $definition->isLazy() ? 'yes' : 'no');
         $tableRows[] = array('Shared', $definition->isShared() ? 'yes' : 'no');
         $tableRows[] = array('Abstract', $definition->isAbstract() ? 'yes' : 'no');
-
-        $autowiredCalls = array_filter($definition->getAutowiredCalls(), function ($method) {
-            return $method !== '__construct';
-        });
-        $tableRows[] = array('Autowire', $definition->isAutowired() ? ($autowiredCalls ? implode("\n", $autowiredCalls) : 'yes') : 'no');
+        $tableRows[] = array('Autowired', $definition->isAutowired() ? 'yes' : 'no');
 
         if ($autowiringTypes = $definition->getAutowiringTypes(false)) {
             $tableRows[] = array('Autowiring Types', implode(', ', $autowiringTypes));

Console/Descriptor/XmlDescriptor.php

@@ -374,18 +374,6 @@ private function getContainerDefinitionDocument(Definition $definition, $id = nu
         $serviceXML->setAttribute('autowired', $definition->isAutowired() ? 'true' : 'false');
         $serviceXML->setAttribute('file', $definition->getFile());
 
-        $autowiredCalls = array_filter($definition->getAutowiredCalls(), function ($method) {
-            return $method !== '__construct';
-        });
-        if ($autowiredCalls) {
-            $serviceXML->appendChild($autowiredMethodsXML = $dom->createElement('autowired-calls'));
-            foreach ($autowiredCalls as $autowiredMethod) {
-                $autowiredMethodXML = $dom->createElement('autowired-call');
-                $autowiredMethodXML->appendChild(new \DOMText($autowiredMethod));
-                $autowiredMethodsXML->appendChild($autowiredMethodXML);
-            }
-        }
-
         $calls = $definition->getMethodCalls();
         if (count($calls) > 0) {
             $serviceXML->appendChild($callsXML = $dom->createElement('calls'));

Tests/Console/Descriptor/ObjectsProvider.php

@@ -137,9 +137,6 @@ public static function getContainerDefinitions()
                 ->addTag('tag2')
                 ->addMethodCall('setMailer', array(new Reference('mailer')))
                 ->setFactory(array(new Reference('factory.service'), 'get')),
-            'definition_autowired' => (new Definition('AutowiredService'))->setAutowired(true),
-            'definition_autowired_with_methods' => (new Definition('AutowiredService'))
-                ->setAutowiredCalls(array('__construct', 'set*', 'addFoo')),
         );
     }
 

Tests/Fixtures/Descriptor/alias_with_definition_1.md

@@ -11,6 +11,6 @@
 - Lazy: yes
 - Shared: yes
 - Abstract: yes
-- Autowire: no
+- Autowired: no
 - Factory Class: `Full\Qualified\FactoryClass`
 - Factory Method: `get`

Tests/Fixtures/Descriptor/alias_with_definition_1.txt

@@ -14,7 +14,7 @@
   Lazy             yes                          
   Shared           yes                          
   Abstract         yes                          
-  Autowire         no                           
+  Autowired        no                           
   Factory Class    Full\Qualified\FactoryClass  
   Factory Method   get                          
  ---------------- -----------------------------

Tests/Fixtures/Descriptor/alias_with_definition_2.md

@@ -11,7 +11,7 @@
 - Lazy: no
 - Shared: yes
 - Abstract: no
-- Autowire: no
+- Autowired: no
 - File: `/path/to/file`
 - Factory Service: `factory.service`
 - Factory Method: `get`

Tests/Fixtures/Descriptor/alias_with_definition_2.txt

@@ -17,7 +17,7 @@
   Lazy              no                               
   Shared            yes                              
   Abstract          no                               
-  Autowire          no                               
+  Autowired         no                               
   Required File     /path/to/file                    
   Factory Service   factory.service                  
   Factory Method    get                              

Tests/Fixtures/Descriptor/builder_1_arguments.json

@@ -80,33 +80,6 @@
             "factory_class": "Full\\Qualified\\FactoryClass",
             "factory_method": "get",
             "tags": []
-        },
-        "definition_autowired": {
-            "class": "AutowiredService",
-            "public": true,
-            "synthetic": false,
-            "lazy": false,
-            "shared": true,
-            "abstract": false,
-            "autowire": true,
-            "arguments": [],
-            "file": null,
-            "tags": []
-        },
-        "definition_autowired_with_methods": {
-            "class": "AutowiredService",
-            "public": true,
-            "synthetic": false,
-            "lazy": false,
-            "shared": true,
-            "abstract": false,
-            "autowire": [
-                "set*",
-                "addFoo"
-            ],
-            "arguments": [],
-            "file": null,
-            "tags": []
         }
     },
     "aliases": {