Drastically improved debug:autowiring + new autowiring info system #26142

weaverryan · 2018-02-11T20:47:56Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	none
License	MIT
Doc PR	symfony/symfony-docs#9247

Hey guys!

This PR makes debug:autowiring WAY more useful :). See screenshot at the bottom.

It's now human-focused: you look for the problem you need to solve (Cache, Logger, etc).

Note: if there are multiple types for the same thing - e.g. Twig\Environment &
Twig_Environment - then by convention, we'll choose one to "recommend" and only
add info about it. The other will still appear at the bottom of the list.

Behind the scenes, this introduces a new mini-system where bundles can tell the
core the purpose of the classes/interfaces that they expose. I didn't include
this at the container level (e.g. in Definition), because I think that's overkill:
this is just a nice decoration around the system. The system is also NOT included
when kernel.debug = false, so there's zero prod overhead. The debug:autowiring
command still works in this situation, but falls back to its original "one big list".

Cheers!

This introduces a new interface + tag that can be used by bundles to inform the core about the *purpose* of the autowirable classes/interfaces. This is makes debug:autowiring human-focused: showing you a list of the autowireable services based on the *purpose* of them.

bmax · 2018-02-11T21:10:02Z

src/Symfony/Bundle/FrameworkBundle/Command/DebugAutowiringCommand.php

@@ -15,6 +15,8 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\DependencyInjection\Debug\AutowiringInfoManager;
+use Symfony\Component\DependencyInjection\Debug\AutowiringTypeInfo;


looks like this isn't used.

They are both used - one I think just for phpdoc :). We have a PR bot (fabbot) that validates cs, including unused use statements

Oh how come include namespaces for just docblocks? Interesting!

It lets you use the short class names in phpdoc - like @param Foo - editors will use the use statement to know the full class name that you’re referencing :)

ro0NL

Cool :)

ro0NL · 2018-02-11T21:53:36Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AutowireDebugInfoPass.php

+ */
+class AutowireDebugInfoPass implements CompilerPassInterface
+{
+    const AUTOWIRING_INFO_PROVIDER_TAG = 'debug.autowiring_info_provider';


public const But needed? This is the first tag const right...

ro0NL · 2018-02-11T21:55:04Z

src/Symfony/Component/DependencyInjection/Debug/AutowiringInfoManager.php

+     *
+     * @return AutowiringTypeInfo|null
+     */
+    public function getInfo(string $type)


: ?AutowiringTypeInfo (docblock is redundant)

+1 I keep thinking Symfony requires 7.0, so no nullable types :). I'll change this happily

ro0NL · 2018-02-11T21:55:18Z

src/Symfony/Component/DependencyInjection/Debug/AutowiringInfoManager.php

+        return isset($this->typeInfos[$type]) ? $this->typeInfos[$type] : null;
+    }
+
+    private function populateTypesInfo()


we dont do :void right?

we could for new APIs existing only in 4.1+

ro0NL · 2018-02-11T21:56:03Z

src/Symfony/Component/DependencyInjection/Debug/AutowiringTypeInfo.php

+final class AutowiringTypeInfo
+{
+    private $type;
+


new lines to be removed

ro0NL · 2018-02-11T21:56:18Z

src/Symfony/Component/DependencyInjection/Debug/AutowiringTypeInfo.php

+     */
+    public static function create(string $type, string $name, int $priority = 0)
+    {
+        return new static($type, $name, $priority);


new self will do as it's final

ro0NL · 2018-02-11T22:00:39Z

src/Symfony/Component/DependencyInjection/Debug/AutowiringInfoManager.php

+
+    private $typeInfos = null;
+
+    public function __construct(array $autowiringInfoProviders)


iterable to leverage type tagged in DI

ro0NL · 2018-02-11T22:00:57Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/debug.xml

@@ -23,5 +23,13 @@
            <argument type="service" id="debug.argument_resolver.inner" />
            <argument type="service" id="debug.stopwatch" />
        </service>
+
+        <service id="debug.autowiring_info_manager" class="Symfony\Component\DependencyInjection\Debug\AutowiringInfoManager">
+            <argument /> <!-- argument info providers -->


type="tagged"?

ro0NL · 2018-02-11T22:03:57Z

src/Symfony/Bundle/FrameworkBundle/Command/DebugAutowiringCommand.php

+            return;
+        }
+
+        $io->title('Key Services');


i'd say Main Services / Common Services / General Services or so =/

ro0NL · 2018-02-11T22:05:38Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkAutowiringInfoProvider.php

+            AutowiringTypeInfo::create(Registry::class, 'Workflow')
+                ->setDescription('use to fetch workflows'),
+
+            AutowiringTypeInfo::create(Registry::class, 'Workflow')


duplicated :)

nicolas-grekas · 2018-02-11T22:12:35Z

That's great!
I'm wondering two things:

is this really related to autowiring? Any services could be described like this, and the other commands could also leverage the info, isn't it?
reciprocally, this reminds me a lot The debug:container command is confusing #24562, which needs just additional but very similar extra info. Would be great to have only one such system. (Could be just extending Definition in fact, not sure it would be overkill :))

ro0NL · 2018-02-11T22:26:54Z

For me the new priority will do (compared to suggested autowireable semantic in #24562 (comment)), related to that i was thinking to group services by info provider (visually in CLI with a label), that way it's already prioritized by provider (tag priority) and the new priority value is not really needed.

is this really related to autowiring?

ServiceInfoProviderInterface seems better indeed

Could be just extending Definition in fact, not sure it would be overkill :)

Tend to like having all info at one place, compared to potentially scattered around all over. But doing it per service definition with definition config is reasonable (overkill IMHO).

dunglas · 2018-02-12T20:09:43Z

This is great. So nice that it shouldn't be limited to autowiring. It can be useful for other services, in other contexts (I think to the Graphviz dumper, and to the debug:container command). I agree with @nicolas-grekas, it would be nice to be able to set the description directly in the XML or YAML file. Then the command can just put the ones with a description first.

weaverryan · 2018-02-14T14:29:11Z

I’m going to rethink this in the context of all services, and possibly a new debug:services command that dumps things in a new way.

Thanks for the suggestions :)

artursvonda · 2018-08-02T13:08:48Z

Could we leverage PHPDocs of classes for this? As a fallback. Strip out tags, for example. But class (or interface) purpose probably matches service purpose.
Or do we want to keep those things separate?

nicolas-grekas · 2018-10-24T14:57:55Z

Closing in favor of #28970, thanks for pushing!

carsonbot added Status: Needs Review Feature labels Feb 11, 2018

weaverryan mentioned this pull request Feb 11, 2018

[WCM] New output for debug:autowiring command symfony/symfony-docs#9247

Closed

weaverryan force-pushed the autowiring-debug-improved branch from 6e10bf2 to dffc922 Compare February 11, 2018 20:49

bmax reviewed Feb 11, 2018

View reviewed changes

ro0NL reviewed Feb 11, 2018

View reviewed changes

javiereguiluz added the DX label Feb 12, 2018

nicolas-grekas added this to the 4.1 milestone Feb 12, 2018

chalasr added Status: Needs Work and removed Status: Needs Review labels Mar 2, 2018

javiereguiluz added the ❄️ Feature Freeze label Mar 12, 2018

weaverryan removed the ❄️ Feature Freeze label Mar 29, 2018

nicolas-grekas modified the milestones: 4.1, next Apr 20, 2018

nicolas-grekas mentioned this pull request May 9, 2018

[RFC] Introduce "semantics": a way to make service ids meaningful by binding them to types+descriptions #27207

Closed

nicolas-grekas mentioned this pull request May 17, 2018

[DI] Public, private and hidden services #27293

Closed

nicolas-grekas closed this Oct 24, 2018

nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Drastically improved debug:autowiring + new autowiring info system #26142

Drastically improved debug:autowiring + new autowiring info system #26142

weaverryan commented Feb 11, 2018 •

edited

Loading

bmax Feb 11, 2018

weaverryan Feb 11, 2018

bmax Feb 11, 2018

weaverryan Feb 11, 2018

ro0NL left a comment

ro0NL Feb 11, 2018

ro0NL Feb 11, 2018

weaverryan Feb 12, 2018

ro0NL Feb 11, 2018

stof Feb 14, 2018

ro0NL Feb 11, 2018

ro0NL Feb 11, 2018

ro0NL Feb 11, 2018

ro0NL Feb 11, 2018

ro0NL Feb 11, 2018 •

edited

Loading

ro0NL Feb 11, 2018

nicolas-grekas commented Feb 11, 2018

ro0NL commented Feb 11, 2018 •

edited

Loading

dunglas commented Feb 12, 2018 •

edited

Loading

weaverryan commented Feb 14, 2018

artursvonda commented Aug 2, 2018

nicolas-grekas commented Oct 24, 2018


		private $typeInfos = null;

		public function __construct(array $autowiringInfoProviders)

Drastically improved debug:autowiring + new autowiring info system #26142

Drastically improved debug:autowiring + new autowiring info system #26142

Conversation

weaverryan commented Feb 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Feb 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Feb 11, 2018

ro0NL commented Feb 11, 2018 • edited Loading

dunglas commented Feb 12, 2018 • edited Loading

weaverryan commented Feb 14, 2018

artursvonda commented Aug 2, 2018

nicolas-grekas commented Oct 24, 2018

weaverryan commented Feb 11, 2018 •

edited

Loading

ro0NL Feb 11, 2018 •

edited

Loading

ro0NL commented Feb 11, 2018 •

edited

Loading

dunglas commented Feb 12, 2018 •

edited

Loading