Fixed a bug with null message level contexts, moved the null checking…

… responsibility from LoggerBase to LoggerContextManager, added StringBuilderLoggingExtensionsTests

src/Core.UnitTests/Logging/LoggerBaseTests.cs

@@ -56,17 +56,6 @@ public void ForContext_CreatesNewLoggerWithUpdatedContextManager()
         _ = settingsProvider.Received().IsVerboseEnabled;
     }
 
-    [TestMethod]
-    public void ForContext_FiltersOutNullAndEmptyContext()
-    {
-        var newContextManager = Substitute.For<ILoggerContextManager>();
-        contextManager.CreateAugmentedContext(Arg.Any<IEnumerable<string>>()).Returns(newContextManager);
-
-        testSubject.ForContext("ctx", string.Empty, null);
-
-        contextManager.Received(1).CreateAugmentedContext(Arg.Is<IEnumerable<string>>(x => x.SequenceEqual(new[] { "ctx" })));
-    }
-
     [TestMethod]
     public void ForVerboseContext_CreatesNewLoggerWithUpdatedContextManager()
     {
@@ -84,17 +73,6 @@ public void ForVerboseContext_CreatesNewLoggerWithUpdatedContextManager()
         _ = settingsProvider.Received().IsVerboseEnabled;
     }
 
-    [TestMethod]
-    public void ForVerboseContext_FiltersOutNullAndEmptyContext()
-    {
-        var newContextManager = Substitute.For<ILoggerContextManager>();
-        contextManager.CreateAugmentedVerboseContext(Arg.Any<IEnumerable<string>>()).Returns(newContextManager);
-
-        testSubject.ForVerboseContext("ctx", string.Empty, null);
-
-        contextManager.Received(1).CreateAugmentedVerboseContext(Arg.Is<IEnumerable<string>>(x => x.SequenceEqual(new[] { "ctx" })));
-    }
-
     [TestMethod]
     public void LogVerbose_VerboseDisabled_DoesNothing()
     {

src/Core.UnitTests/Logging/LoggerContextManagerTests.cs

@@ -116,6 +116,14 @@ public void GetFormattedContextOrNull_NoContext_ReturnsNull() =>
     public void GetFormattedContextOrNull_NoBaseContext_ReturnsMessageLevelContextOnly() =>
         testSubject.GetFormattedContextOrNull(new MessageLevelContext{Context = ["a", "b"], VerboseContext = ["c", "d"]}).Should().Be("a > b");
 
+    [TestMethod]
+    public void GetFormattedContextOrNull_NullAndEmptyItems_ReturnsNonNullMessageLevelContextOnly() =>
+        testSubject.GetFormattedContextOrNull(new MessageLevelContext{Context = ["a", null, "", "b"], VerboseContext = ["c", "d"]}).Should().Be("a > b");
+
+    [TestMethod]
+    public void GetFormattedContextOrNull_NullAndEmptyItemsOnly_ReturnsNull() =>
+        testSubject.GetFormattedContextOrNull(new MessageLevelContext{Context = [null, ""], VerboseContext = ["c", "d"]}).Should().BeNull();
+
     [TestMethod]
     public void GetFormattedContextOrNull_MessageLevelContextNotCached()
     {
@@ -139,6 +147,14 @@ public void GetFormattedVerboseContextOrNull_NoContext_ReturnsNull() =>
     public void GetFormattedVerboseContextOrNull_NoBaseContext_ReturnsMessageLevelContextOnly() =>
         testSubject.GetFormattedVerboseContextOrNull(new MessageLevelContext{Context = ["a", "b"], VerboseContext = ["c", "d"]}).Should().Be("c > d");
 
+    [TestMethod]
+    public void GetFormattedVerboseContextOrNull_NullAndEmptyItems_ReturnsNonNullMessageLevelContextOnly() =>
+        testSubject.GetFormattedVerboseContextOrNull(new MessageLevelContext{Context = ["a", "b"], VerboseContext = ["c", null, "", "d"]}).Should().Be("c > d");
+
+    [TestMethod]
+    public void GetFormattedVerboseContextOrNull_NullAndEmptyItemsOnly_ReturnsNull() =>
+        testSubject.GetFormattedVerboseContextOrNull(new MessageLevelContext{Context = ["a", "b"], VerboseContext = [null, ""]}).Should().BeNull();
+
     [TestMethod]
     public void GetFormattedVerboseContextOrNull_MessageLevelContextNotCached()
     {

src/Core.UnitTests/Logging/StringBuilderLoggingExtensionsTests.cs

@@ -0,0 +1,60 @@
+/*
+ * SonarLint for Visual Studio
+ * Copyright (C) 2016-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+using System.Text;
+using SonarLint.VisualStudio.Core.Logging;
+
+namespace SonarLint.VisualStudio.Core.UnitTests.Logging;
+
+[TestClass]
+public class StringBuilderLoggingExtensionsTests
+{
+    [DataTestMethod]
+    [DataRow(null, "", "[] ")]
+    [DataRow(null, null, "[] ")]
+    [DataRow(null, "a", "[a] ")]
+    [DataRow("", "a", "[a] ")]
+    [DataRow("", "a {0}", "[a {0}] ")]
+    [DataRow("abc", "def", "abc[def] ")]
+    [DataRow("abc ", "def", "abc [def] ")]
+    public void AppendProperty_AddsPlainPropertyValueToTheEnd(string original, string property, string expected) =>
+        new StringBuilder(original).AppendProperty(property).ToString().Should().Be(expected);
+
+    [DataTestMethod]
+    [DataRow(null, "", "[] ")]
+    [DataRow(null, "a", "[a] ")]
+    [DataRow("", "a", "[a] ")]
+    [DataRow("abc", "def", "abc[def] ")]
+    [DataRow("abc ", "def", "abc [def] ")]
+    public void AppendPropertyFormat_NonFormattedProperty_AddsPlainValueToTheEnd(string original, string property, string expected) =>
+        new StringBuilder(original).AppendPropertyFormat(property).ToString().Should().Be(expected);
+
+    [TestMethod]
+    public void AppendPropertyFormat_FormattedString_CorrectlyAppliesStringFormat() =>
+        new StringBuilder().AppendPropertyFormat("for{0}ted", "mat").ToString().Should().Be("[formatted] ");
+
+    [TestMethod]
+    public void AppendPropertyFormat_IncorrectNumberOfParameters_Throws()
+    {
+        var act =()=> new StringBuilder().AppendPropertyFormat("for{0}t{1}", "mat");
+
+        act.Should().Throw<FormatException>();
+    }
+}

src/Core/Logging/LoggerBase.cs

@@ -31,13 +31,13 @@ internal class LoggerBase(
 
     public ILogger ForContext(params string[] context) =>
         new LoggerBase(
-            contextManager.CreateAugmentedContext(context.Where(x => !string.IsNullOrEmpty(x))),
+            contextManager.CreateAugmentedContext(context),
             writer,
             settingsProvider);
 
     public ILogger ForVerboseContext(params string[] context) =>
         new LoggerBase(
-            contextManager.CreateAugmentedVerboseContext(context.Where(x => !string.IsNullOrEmpty(x))),
+            contextManager.CreateAugmentedVerboseContext(context),
             writer,
             settingsProvider);
 

src/Core/Logging/LoggerContextManager.cs

@@ -45,14 +45,16 @@ private LoggerContextManager(ImmutableList<string> contexts, ImmutableList<strin
         formatedVerboseContext = new Lazy<string>(() => MergeContextsIntoSingleProperty(verboseContexts), LazyThreadSafetyMode.PublicationOnly);
     }
 
+    public ILoggerContextManager CreateAugmentedContext(IEnumerable<string> additionalContexts) => new LoggerContextManager(contexts.AddRange(FilterContexts(additionalContexts)), verboseContexts);
+
+    public ILoggerContextManager CreateAugmentedVerboseContext(IEnumerable<string> additionalVerboseContexts) => new LoggerContextManager(contexts, verboseContexts.AddRange(FilterContexts(additionalVerboseContexts)));
+
     public string GetFormattedContextOrNull(MessageLevelContext messageLevelContext) =>
         GetContextInternal(formatedContext.Value, messageLevelContext.Context);
     public string GetFormattedVerboseContextOrNull(MessageLevelContext messageLevelContext) =>
         GetContextInternal(formatedVerboseContext.Value, messageLevelContext.VerboseContext);
 
-    public ILoggerContextManager CreateAugmentedContext(IEnumerable<string> additionalContexts) => new LoggerContextManager(contexts.AddRange(additionalContexts), verboseContexts);
-
-    public ILoggerContextManager CreateAugmentedVerboseContext(IEnumerable<string> additionalVerboseContexts) => new LoggerContextManager(contexts, verboseContexts.AddRange(additionalVerboseContexts));
+    private static IEnumerable<string> FilterContexts(IEnumerable<string> contexts) => contexts.Where(context => !string.IsNullOrEmpty(context));
 
     private static string GetContextInternal(string baseContext, IReadOnlyCollection<string> messageLevelContext)
     {
@@ -61,7 +63,7 @@ private static string GetContextInternal(string baseContext, IReadOnlyCollection
             return baseContext;
         }
 
-        IEnumerable<string> resultingContext = messageLevelContext;
+        IEnumerable<string> resultingContext = FilterContexts(messageLevelContext);
         if (baseContext != null)
         {
             resultingContext = resultingContext.Prepend(baseContext);