Fix logging formatting (#106283)

* Fix Logging formatting with multiple collections * Feedback
dotnet · Aug 23, 2024 · 2101571 · 2101571
1 parent fd6aab5
commit 2101571
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 16 deletions.
diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs
@@ -186,32 +186,26 @@ internal string Format()
 #if NET8_0_OR_GREATER
         internal string Format<TArg0>(TArg0 arg0)
         {
-            object? arg0String = null;
             return
-                !TryFormatArgumentIfNullOrEnumerable(arg0, ref arg0String) ?
+                !TryFormatArgumentIfNullOrEnumerable(arg0, out object? arg0String) ?
                 string.Format(CultureInfo.InvariantCulture, _format, arg0) :
                 string.Format(CultureInfo.InvariantCulture, _format, arg0String);
         }
 
         internal string Format<TArg0, TArg1>(TArg0 arg0, TArg1 arg1)
         {
-            object? arg0String = null, arg1String = null;
             return
-                !TryFormatArgumentIfNullOrEnumerable(arg0, ref arg0String) &&
-                !TryFormatArgumentIfNullOrEnumerable(arg1, ref arg1String) ?
-                string.Format(CultureInfo.InvariantCulture, _format, arg0, arg1) :
-                string.Format(CultureInfo.InvariantCulture, _format, arg0String ?? arg0, arg1String ?? arg1);
+                TryFormatArgumentIfNullOrEnumerable(arg0, out object? arg0String) | TryFormatArgumentIfNullOrEnumerable(arg1, out object? arg1String) ?
+                string.Format(CultureInfo.InvariantCulture, _format, arg0String ?? arg0, arg1String ?? arg1) :
+                string.Format(CultureInfo.InvariantCulture, _format, arg0, arg1);
         }
 
         internal string Format<TArg0, TArg1, TArg2>(TArg0 arg0, TArg1 arg1, TArg2 arg2)
         {
-            object? arg0String = null, arg1String = null, arg2String = null;
             return
-                !TryFormatArgumentIfNullOrEnumerable(arg0, ref arg0String) &&
-                !TryFormatArgumentIfNullOrEnumerable(arg1, ref arg1String) &&
-                !TryFormatArgumentIfNullOrEnumerable(arg2, ref arg2String) ?
-                string.Format(CultureInfo.InvariantCulture, _format, arg0, arg1, arg2) :
-                string.Format(CultureInfo.InvariantCulture, _format, arg0String ?? arg0, arg1String ?? arg1, arg2String ?? arg2);
+                TryFormatArgumentIfNullOrEnumerable(arg0, out object? arg0String) | TryFormatArgumentIfNullOrEnumerable(arg1, out object? arg1String) | TryFormatArgumentIfNullOrEnumerable(arg2, out object? arg2String) ?
+                string.Format(CultureInfo.InvariantCulture, _format, arg0String ?? arg0, arg1String ?? arg1, arg2String ?? arg2):
+                string.Format(CultureInfo.InvariantCulture, _format, arg0, arg1, arg2);
         }
 #else
         internal string Format(object? arg0) =>
@@ -253,11 +247,10 @@ internal string Format(object? arg0, object? arg1, object? arg2) =>
 
         private static object FormatArgument(object? value)
         {
-            object? stringValue = null;
-            return TryFormatArgumentIfNullOrEnumerable(value, ref stringValue) ? stringValue : value!;
+            return TryFormatArgumentIfNullOrEnumerable(value, out object? stringValue) ? stringValue : value!;
         }
 
-        private static bool TryFormatArgumentIfNullOrEnumerable<T>(T? value, [NotNullWhen(true)] ref object? stringValue)
+        private static bool TryFormatArgumentIfNullOrEnumerable<T>(T? value, [NotNullWhen(true)] out object? stringValue)
         {
             if (value == null)
             {
@@ -284,6 +277,7 @@ private static bool TryFormatArgumentIfNullOrEnumerable<T>(T? value, [NotNullWhe
                 return true;
             }
 
+            stringValue = null;
             return false;
         }
     }

diff --git a/...ons.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs b/...ons.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs
@@ -1409,6 +1409,31 @@ public void ConsoleLoggerOptions_IncludeScopes_IsReadFromLoggingConfiguration()
             Assert.True(formatter.FormatterOptions.IncludeScopes);
         }
 
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
+        public void LogMultipleArrays()
+        {
+            // Arrange
+            using var t = SetUp();
+            var logger = t.Logger;
+            var sink = t.Sink;
+
+            var define1 = LoggerMessage.Define<string[], string[]>(LogLevel.Information, new EventId(), "Log: {Array1} and {Array2}");
+            var define2 = LoggerMessage.Define<int, string[], string[]>(LogLevel.Information, new EventId(), "Log {Number}: {Array1} and {Array2}");
+
+            // Act
+            define1(logger, ["a", "b", "c"], ["d", "e", "f"], null);
+            define2(logger, 30, ["a", "b", "c"], ["d", "e", "f"], null);
+
+            var expectedMessage1 = $"{CreateHeader(ConsoleLoggerFormat.Default)}{Environment.NewLine}{_paddingString}Log: a, b, c and d, e, f{Environment.NewLine}";
+            var expectedMessage2 = $"{CreateHeader(ConsoleLoggerFormat.Default)}{Environment.NewLine}{_paddingString}Log 30: a, b, c and d, e, f{Environment.NewLine}";
+
+            Assert.Equal(4, sink.Writes.Count);
+            Assert.Equal("info", sink.Writes[0].Message);
+            Assert.Equal(expectedMessage1, sink.Writes[1].Message);
+            Assert.Equal("info", sink.Writes[2].Message);
+            Assert.Equal(expectedMessage2, sink.Writes[3].Message);
+        }
+
         public static TheoryData<ConsoleLoggerFormat, LogLevel> FormatsAndLevels
         {
             get