Replace non-test usages of \Drupal::logger() with IoC injection

Please find below my patch.Passing $logger_factory service to construct function in #2 patch.
Please review it and test it.

Added a interdiff file.

Looks ok apart from:

+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -59,14 +75,20 @@ class ModuleInstaller implements ModuleInstallerInterface {
+    $this->routeBuilder = $route_builder;

Why is the route builder being added here?

> I figured that as long as the route builder was being injected it might as well be used. I wonder if we could / should just skip injecting it completely. However, that seemed like too much of a change for this issue.

It does seem like too much of a change. But adding DI boilerplate is increasing the work for when the parameter in services.yml is cleaned up. I would be inclined to file a blocker issue to clean that up first.

I would be inclined to file a blocker issue to clean that up first.

I appreciate issues with a single minded focus...

Reviewers can come in grasp the concept and see that it is repeated ad nauseam.

For this issue, I find myself with the other perspective. There is goodness here that I don't want to delay.

Sure the routeBuilder refinement is out of scope ... but it is easy to see that it too is good/"going in the right direction" .. Yes we should file a follow up

But I would rather not see this issue further delayed.

I hear the drum beat of Drupal9 .. Symfony4 is removing $container->get() so injection is soon to be a thorn in the lion's paw.

Do we need re-rolling of patch here?

As needs reroll, adjust the title/scope a little bit.

+++ b/core/core.services.yml
@@ -445,6 +445,9 @@ services:
+  logger.channel.theme:

@@ -522,7 +525,7 @@ services:
+    arguments: ['%app.root%', '@module_handler', '@kernel', '@logger.channel.default']

@@ -1490,7 +1493,7 @@ services:
+    arguments: ['%app.root%', '@theme.negotiator', '@theme.initialization', '@module_handler', '@logger.channel.theme']

+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -59,14 +68,17 @@ class ModuleInstaller implements ModuleInstallerInterface {
+  public function __construct($root, ModuleHandlerInterface $module_handler, DrupalKernelInterface $kernel, LoggerInterface $logger) {

+++ b/core/lib/Drupal/Core/Theme/ThemeManager.php
@@ -67,12 +75,15 @@ class ThemeManager implements ThemeManagerInterface {
+  public function __construct($root, ThemeNegotiatorInterface $theme_negotiator, ThemeInitializationInterface $theme_initialization, ModuleHandlerInterface $module_handler, LoggerInterface $logger) {

Unlike #4, injected logger channels instead of the logger factory.

+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -59,14 +75,20 @@ class ModuleInstaller implements ModuleInstallerInterface {
+   * @param \Drupal\Core\Routing\RouteBuilderInterface $route_builder
+   *   The route building service.
...
-  public function __construct($root, ModuleHandlerInterface $module_handler, DrupalKernelInterface $kernel) {
+  public function __construct($root, ModuleHandlerInterface $module_handler, DrupalKernelInterface $kernel, RouteBuilderInterface $route_builder, LoggerChannelFactoryInterface $logger_factory) {

Injected/introduced route building service in #2 was removed, out of scope here.

Tasks left:

1. Check if there are more to do in non-test code.
2. add change record(s) and add BC layers for the __construct changes.

Applied issue summary template to IS

CR added. Adding BC layers.

+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -68,16 +68,20 @@ class ModuleInstaller implements ModuleInstallerInterface {
+      @trigger_error('The logger.channel.default service must be passed to ' . __CLASS__ . '::'. __METHOD__ . '(). It was added in drupal:9.1.0 and will be required before drupal:10.0.0. See https://www.drupal.org/node/3160464', E_USER_DEPRECATED);

No space betwtten '::'and ., addressing.

Last patch failed to apply , re-rolling the patch and rearranging use statement alphabetically order. Kindly review a new patch.

Can't fix the tests failed, so instead of inject logger channels, back to inject logger factory.

Forgot uploading the patch.

This needs a reroll because it no longer applies currently, in the last patch, are we sure this fixes all the \Drupal::logger occurances?

I'm also not sure if this is a bug or a task?

Added reroll of patch #28 on Drupal 9.5.x. and removed reroll tag.

This patch does not apply to 10.x and since it introduces a ton of new deprecations I'm not sure if it should go in 9.5 with deprecations to be removed in 10.x or if it should go in both, with the intention to remove the deprecations in 11.x.

Let's start by fixing all the implementations left, I found those with rg \Drupal::logger

core/modules/locale/src/PoDatabaseWriter.php
240:        \Drupal::logger('locale')->error('Import of string "%string" was skipped because of disallowed or malformed HTML.', ['%string' => $translation]);

core/modules/system/src/Access/CronAccessCheck.php
24:      \Drupal::logger('cron')->notice('Cron could not run because an invalid key was used.');
28:      \Drupal::logger('cron')->notice('Cron could not run because the site is in maintenance mode.');

core/modules/migrate/src/MigrateMessage.php
27:    \Drupal::logger('migrate')->log($type, $message);

core/modules/migrate_drupal_ui/src/Batch/MigrateUpgradeImportBatch.php
143:        \Drupal::logger('migrate_drupal_ui')->error($e->getMessage());
155:          \Drupal::logger('migrate_drupal_ui')->notice($message);
193:          \Drupal::logger('migrate_drupal_ui')->error('Operation on @migration failed', ['@migration' => $migration_name]);
198:          \Drupal::logger('migrate_drupal_ui')->error('Operation on @migration skipped due to unfulfilled dependencies', ['@migration' => $migration_name]);
215:        \Drupal::logger('migrate_drupal_ui')->error($message);

core/modules/filter/src/Element/ProcessedText.php
149:    return \Drupal::logger($channel);

core/modules/filter/src/Plugin/Filter/FilterNull.php
38:      \Drupal::logger('filter')->alert('Missing filter plugin: %filter.', ['%filter' => $plugin_id]);

core/modules/image/src/Entity/ImageStyle.php
324:      \Drupal::logger('image')->error('Failed to create style directory: %directory', ['%directory' => $directory]);
334:        \Drupal::logger('image')->error('Cached image file %destination already exists. There may be an issue with your rewrite configuration.', ['%destination' => $derivative_uri]);

core/lib/Drupal/Core/Theme/ThemeManager.php
88:      $this->logger = \Drupal::logger('theme');

core/lib/Drupal/Core/Extension/ModuleInstaller.php
114:      $this->logger = \Drupal::logger('system');

core/lib/Drupal/Core/Entity/EntityDisplayBase.php
571:    return \Drupal::logger('system');

Pretty sure it's too late for 9.5.x/10.0.x to introduce any more deprecations except for module/theme deprecations.
So moving this to 10.1.x.

Also pretty sure this is a task, since nothing is "broken".

For many of these we should probably autoconfigure the loggers instead Loggers can be autoconfigured for service classes implementing \Psr\Log\LoggerAwareInterface

EDIT: I take it back. Most of these aren't in services, and if they are they use a different logger channel than what is autoconfigured. Perhaps we could make use of LoggerAwareInterface and LoggerAwareTrait though.