New rule S6670: Trace.Write and Trace.WriteLine should not be used (#…

…8864)

analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S1128-IntentionalFindings-net8.0.json

@@ -5,6 +5,12 @@
       "Message": "Remove this unnecessary \u0027using\u0027.",
       "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S2857.cs#L9",
       "Location": "Line 9 Position 1-29"
+    },
+    {
+      "Id": "S1128",
+      "Message": "Remove this unnecessary \u0027using\u0027.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6670.cs#L1",
+      "Location": "Line 1 Position 1-14"
     }
   ]
 }

analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S1451-IntentionalFindings-net8.0.json

@@ -17,6 +17,12 @@
       "Message": "Add or update the header of this file.",
       "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6668.cs#L1",
       "Location": "Line 1 Position 1-1"
+    },
+    {
+      "Id": "S1451",
+      "Message": "Add or update the header of this file.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6670.cs#L1",
+      "Location": "Line 1 Position 1-1"
     }
   ]
 }

analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S2325-IntentionalFindings-net8.0.json

@@ -11,6 +11,12 @@
       "Message": "Make \u0027Log\u0027 a static method.",
       "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6668.cs#L8",
       "Location": "Line 8 Position 22-25"
+    },
+    {
+      "Id": "S2325",
+      "Message": "Make \u0027Method\u0027 a static method.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6670.cs#L8",
+      "Location": "Line 8 Position 21-27"
     }
   ]
 }

analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S4055-IntentionalFindings-net8.0.json

@@ -5,6 +5,12 @@
       "Message": "Replace this string literal with a string retrieved through an instance of the \u0027ResourceManager\u0027 class.",
       "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6668.cs#L9",
       "Location": "Line 9 Position 32-55"
+    },
+    {
+      "Id": "S4055",
+      "Message": "Replace this string literal with a string retrieved through an instance of the \u0027ResourceManager\u0027 class.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6670.cs#L10",
+      "Location": "Line 10 Position 25-34"
     }
   ]
 }

analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6670-IntentionalFindings-net8.0.json

@@ -0,0 +1,10 @@
+{
+  "Issues": [
+    {
+      "Id": "S6670",
+      "Message": "Avoid using Trace.Write, use instead methods that specify the trace event type.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6670.cs#L10",
+      "Location": "Line 10 Position 19-24"
+    }
+  ]
+}

analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6670.cs

@@ -0,0 +1,13 @@
+using System;
+using System.Diagnostics;
+
+namespace IntentionalFindings
+{
+    public class S6670
+    {
+        public void Method()
+        {
+            Trace.Write("Message"); // Noncompliant (S6670) {{Avoid using Trace.Write, use instead methods that specify the trace event type.}}
+        }
+    }
+}

analyzers/rspec/cs/S6670.html

@@ -0,0 +1,51 @@
+<h2>Why is this an issue?</h2>
+<p><a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.write">Trace.Write</a> and <a
+href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.writeline">Trace.WriteLine</a> methods are writing to the underlying
+output stream directly, bypassing the trace formatting and filtering performed by <a
+href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.tracelistener.traceevent">TraceListener.TraceEvent</a> implementations. It is
+preferred to use <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.traceerror">Trace.TraceError</a>, <a
+href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.tracewarning">Trace.TraceWarning</a> and <a
+href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.traceinformation">Trace.TraceInformation</a> methods instead because they
+call the <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.tracelistener.traceevent">TraceEvent method</a> which filters the
+trace output according to the <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.traceeventtype">TraceEventType</a> (Error,
+Warning or Information) and enhance the output with additional information.</p>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+try
+{
+    var message = RetrieveMessage();
+    Trace.Write($"Message received: {message}"); // Noncompliant
+}
+catch (Exception ex)
+{
+    Trace.WriteLine(ex); // Noncompliant
+}
+</pre>
+<pre data-diff-id="1" data-diff-type="compliant">
+try
+{
+    var message = RetrieveMessage();
+    Trace.TraceInformation($"Message received: {message}");
+}
+catch (Exception ex)
+{
+    Trace.TraceError(ex);
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.traceerror">Trace.TraceError Method</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.traceinformation">Trace.TraceInformation
+  Method</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.tracewarning">Trace.TraceWarning Method</a>
+  </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.write">Trace.Write Method</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace.writeline">Trace.WriteLine Method</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.tracelistener.traceevent">TraceListener.TraceEvent
+  Method</a> </li>
+</ul>
+<h3>Articles &amp; blog posts</h3>
+<ul>
+  <li> Stackoverflow - <a href="https://stackoverflow.com/q/26350620">Difference between Trace.Write() and Trace.TraceInformation()</a> </li>
+</ul>
+

analyzers/rspec/cs/S6670.json

@@ -0,0 +1,17 @@
+{
+  "title": "\"Trace.Write\" and \"Trace.WriteLine\" should not be used",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "logging"
+  ],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-6670",
+  "sqKey": "S6670",
+  "scope": "Main",
+  "quickfix": "targeted"
+}

analyzers/rspec/cs/Sonar_way_profile.json

@@ -300,6 +300,7 @@
     "S6640",
     "S6667",
     "S6668",
+    "S6670",
     "S6674",
     "S6677",
     "S6678",

analyzers/src/SonarAnalyzer.CSharp/Rules/DontUseTraceWrite.cs

@@ -0,0 +1,38 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2024 SonarSource SA
+ * mailto: contact 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.
+ */
+
+namespace SonarAnalyzer.Rules.CSharp;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class DontUseTraceWrite : DoNotCallMethodsBase<SyntaxKind, InvocationExpressionSyntax>
+{
+    private const string DiagnosticId = "S6670";
+
+    protected override string MessageFormat => "Avoid using {0}, use instead methods that specify the trace event type.";
+
+    protected override IEnumerable<MemberDescriptor> CheckedMethods => [
+        new(KnownType.System_Diagnostics_Trace, "Write"),
+        new(KnownType.System_Diagnostics_Trace, "WriteLine")
+    ];
+
+    protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
+
+    public DontUseTraceWrite() : base(DiagnosticId) { }
+}

analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs

@@ -6594,7 +6594,7 @@ internal static class RuleTypeMappingCS
         ["S6667"] = "CODE_SMELL",
         ["S6668"] = "CODE_SMELL",
         // ["S6669"],
-        // ["S6670"],
+        ["S6670"] = "CODE_SMELL",
         // ["S6671"],
         // ["S6672"],
         // ["S6673"],

analyzers/tests/SonarAnalyzer.Test/Rules/DontUseTraceWriteTest.cs

@@ -0,0 +1,33 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2024 SonarSource SA
+ * mailto: contact 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 SonarAnalyzer.Rules.CSharp;
+
+namespace SonarAnalyzer.Test.Rules;
+
+[TestClass]
+public class DontUseTraceWriteTest
+{
+    private readonly VerifierBuilder builder = new VerifierBuilder<DontUseTraceWrite>();
+
+    [TestMethod]
+    public void DontUseTraceWrite_CS() =>
+        builder.AddPaths("DontUseTraceWrite.cs").Verify();
+}

analyzers/tests/SonarAnalyzer.Test/TestCases/DontUseTraceWrite.cs

@@ -0,0 +1,82 @@
+using System;
+using System.Diagnostics;
+using AliasedTrace = System.Diagnostics.Trace;
+
+public class Program
+{
+    public void Noncompliant_TraceMethods()
+    {
+        Trace.Write("Message");             // Noncompliant {{Avoid using Trace.Write, use instead methods that specify the trace event type.}}
+//            ^^^^^
+        Trace.WriteLine("Message");         // Noncompliant {{Avoid using Trace.WriteLine, use instead methods that specify the trace event type.}}
+    }
+
+    public void Compliant_TraceMethods(string arg)
+    {
+        Trace.WriteIf(true, "Message");
+        Trace.WriteLineIf(true, "Message");
+
+        Trace.TraceError("Message");
+        Trace.TraceError("Message: {0}", arg);
+
+        Trace.TraceInformation("Message");
+        Trace.TraceInformation("Message: {0}", arg);
+
+        Trace.TraceWarning("Message");
+        Trace.TraceWarning("Message: {0}", arg);
+    }
+
+    public void Write_Overloads()
+    {
+        Trace.Write("Message");                         // Noncompliant
+        Trace.Write(42);                                // Noncompliant
+        Trace.Write("Message", "Category");             // Noncompliant
+        Trace.Write(42, "Category");                    // Noncompliant
+
+        Trace.WriteLine("Message");                     // Noncompliant
+        Trace.WriteLine(42);                            // Noncompliant
+        Trace.WriteLine("Message", "Category");         // Noncompliant
+        Trace.WriteLine(42, "Category");                // Noncompliant
+    }
+
+    public void Not_TraceClass(string arg)
+    {
+        Console.Write("Message");
+        Console.Write("Message: {0}", arg);
+        Console.WriteLine("Message");
+        Console.WriteLine("Message: {0}", arg);
+
+        Debug.Write("Message");
+        Debug.Write("Message: {0}", arg);
+        Debug.WriteLine("Message");
+        Debug.WriteLine("Message: {0}", arg);
+    }
+
+    public void Aliased_Trace()
+    {
+        AliasedTrace.Write("Message");                  // Noncompliant
+        AliasedTrace.WriteLine("Message");              // Noncompliant
+    }
+}
+
+namespace MyNamespace
+{
+    public class Test
+    {
+        public void Using_CustomTraceClass(string arg)
+        {
+            Trace.Write("Message");                     // Compliant - the method is not from the System.Diagnostics.Trace class
+            Trace.Write("Message: {0}", arg);
+            Trace.WriteLine("Message");
+            Trace.WriteLine("Message: {0}", arg);
+        }
+    }
+
+    public static class Trace
+    {
+        public static void Write(string message) { }
+        public static void Write(string message, params object[] args) { }
+        public static void WriteLine(string message) { }
+        public static void WriteLine(string message, params object[] args) { }
+    }
+}