New Rule S2629: Logging templates should be constant

analyzers/its/expected/Ember-MM/S2629-Ember.Plugins.json

@@ -0,0 +1,16 @@
+{
+  "Issues": [
+    {
+      "Id": "S2629",
+      "Message": "Don\u0027t use String.Format in logging message templates.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Ember-MM/Ember.Plugins/PluginBase.cs#L71-L73",
+      "Location": "Lines 71-73 Position 27-49"
+    },
+    {
+      "Id": "S2629",
+      "Message": "Don\u0027t use String.Format in logging message templates.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Ember-MM/Ember.Plugins/PluginSectionHandler.cs#L70",
+      "Location": "Line 70 Position 35-94"
+    }
+  ]
+}

analyzers/rspec/cs/S2629.html

@@ -0,0 +1,38 @@
+<h2>Why is this an issue?</h2>
+<p>Logging arguments should not require evaluation in order to avoid unnecessary performance overhead. When passing concatenated strings or string
+interpolations directly into a logging method, the evaluation of these expressions occurs every time the logging method is called, regardless of the
+log level. This can lead to inefficient code execution and increased resource consumption.</p>
+<p>Instead, it is recommended to use the overload of the logger that accepts a log format and its arguments as separate parameters. By separating the
+log format from the arguments, the evaluation of expressions can be deferred until it is necessary, based on the log level. This approach improves
+performance by reducing unnecessary evaluations and ensures that logging statements are only evaluated when needed.</p>
+<p>Furthermore, using a constant log format enhances observability and facilitates searchability in log aggregation and monitoring software.</p>
+<p>The rule covers the following logging frameworks:</p>
+<ul>
+  <li> <a href="https://www.nuget.org/packages/Microsoft.Extensions.Logging">Microsoft.Extensions.Logging</a> </li>
+  <li> <a href="https://www.nuget.org/packages/Castle.Core">Castle.Core</a> </li>
+  <li> <a href="https://www.nuget.org/packages/log4net">log4net</a> </li>
+  <li> <a href="https://www.nuget.org/packages/Serilog">Serilog</a> </li>
+  <li> <a href="https://www.nuget.org/packages/NLog">Nlog</a> </li>
+</ul>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+public void Method(ILogger logger, bool parameter)
+{
+    logger.DebugFormat($"The value of the parameter is: {parameter}.");
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+public void Method(ILogger logger, bool parameter)
+{
+    logger.DebugFormat("The value of the parameter is: {Parameter}.", parameter);
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Microsoft Learn - <a
+  href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.interpolatedstringhandlerattribute">InterpolatedStringHandlerArgumentAttribute</a> </li>
+</ul>
+

analyzers/rspec/cs/S2629.json

@@ -0,0 +1,18 @@
+{
+  "title": "Logging templates should be constant",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "performance",
+    "logging"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-2629",
+  "sqKey": "S2629",
+  "scope": "Main",
+  "quickfix": "infeasible"
+}

analyzers/rspec/cs/Sonar_way_profile.json

@@ -106,6 +106,7 @@
     "S2583",
     "S2589",
     "S2612",
+    "S2629",
     "S2681",
     "S2688",
     "S2692",

analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs

@@ -162,6 +162,7 @@ public static SyntaxNode WalkUpParentheses(this SyntaxNode node)
                 MemberAccessExpressionSyntax { Name.Identifier: var identifier } => identifier,
                 MemberBindingExpressionSyntax { Name.Identifier: var identifier } => identifier,
                 MethodDeclarationSyntax { Identifier: var identifier } => identifier,
+                NameColonSyntax nameColon => nameColon.Name.Identifier,
                 NamespaceDeclarationSyntax { Name: { } name } => GetIdentifier(name),
                 NullableTypeSyntax { ElementType: { } elementType } => GetIdentifier(elementType),
                 ObjectCreationExpressionSyntax { Type: var type } => GetIdentifier(type),

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

@@ -0,0 +1,125 @@
+/*
+ * 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 UseConstantLoggingTemplate : SonarDiagnosticAnalyzer
+{
+    private const string DiagnosticId = "S2629";
+    private const string MessageFormat = "{0}";
+    private const string OnUsingStringInterpolation = "Don't use string interpolation in logging message templates.";
+    private const string OnUsingStringFormat = "Don't use String.Format in logging message templates.";
+    private const string OnUsingStringConcatenation = "Don't use string concatenation in logging message templates.";
+
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
+
+    private static readonly ImmutableDictionary<SyntaxKind, string> Messages = new Dictionary<SyntaxKind, string>
+    {
+        {SyntaxKind.AddExpression, OnUsingStringConcatenation},
+        {SyntaxKind.InterpolatedStringExpression, OnUsingStringInterpolation},
+        {SyntaxKind.InvocationExpression, OnUsingStringFormat},
+    }.ToImmutableDictionary();
+
+    private static readonly ImmutableArray<KnownType> LoggerTypes = ImmutableArray.Create(
+        KnownType.Castle_Core_Logging_ILogger,
+        KnownType.log4net_ILog,
+        KnownType.log4net_Util_ILogExtensions,
+        KnownType.Microsoft_Extensions_Logging_LoggerExtensions,
+        KnownType.NLog_ILogger,
+        KnownType.NLog_ILoggerBase,
+        KnownType.NLog_ILoggerExtensions,
+        KnownType.Serilog_ILogger,
+        KnownType.Serilog_Log);
+
+    private static readonly ImmutableHashSet<string> LoggerMethodNames = ImmutableHashSet.Create(
+        "ConditionalDebug",
+        "ConditionalTrace",
+        "Debug",
+        "DebugFormat",
+        "Error",
+        "ErrorFormat",
+        "Fatal",
+        "FatalFormat",
+        "Info",
+        "InfoFormat",
+        "Information",
+        "Log",
+        "LogCritical",
+        "LogDebug",
+        "LogError",
+        "LogFormat",
+        "LogInformation",
+        "LogTrace",
+        "LogWarning",
+        "Trace",
+        "TraceFormat",
+        "Verbose",
+        "Warn",
+        "WarnFormat",
+        "Warning");
+
+    private static readonly ImmutableHashSet<string> LogMessageParameterNames = ImmutableHashSet.Create(
+        "format",
+        "message",
+        "messageTemplate");
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    protected override void Initialize(SonarAnalysisContext context) =>
+        context.RegisterNodeAction(c =>
+        {
+            var invocation = (InvocationExpressionSyntax)c.Node;
+            if (LoggerMethodNames.Contains(invocation.GetName())
+                && c.SemanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol method
+                && LoggerTypes.Any(x => x.Matches(method.ContainingType))
+                && method.Parameters.FirstOrDefault(x => LogMessageParameterNames.Contains(x.Name)) is { } messageParameter
+                && ArgumentValue(invocation, method, messageParameter) is { } argumentValue
+                && InvalidSyntaxNode(argumentValue, c.SemanticModel) is { } invalidNode)
+            {
+                c.ReportIssue(Diagnostic.Create(Rule, invalidNode.GetLocation(), Messages[invalidNode.Kind()]));
+            }
+        },
+        SyntaxKind.InvocationExpression);
+
+    private static CSharpSyntaxNode ArgumentValue(InvocationExpressionSyntax invocation, IMethodSymbol method, IParameterSymbol parameter)
+    {
+        if (invocation.ArgumentList.Arguments.FirstOrDefault(x => x.NameColon?.GetName() == parameter.Name) is { } argument)
+        {
+            return argument.Expression;
+        }
+        else
+        {
+            var paramIndex = method.Parameters.IndexOf(parameter);
+            return invocation.ArgumentList.Arguments[paramIndex].Expression;
+        }
+    }
+
+    private static SyntaxNode InvalidSyntaxNode(SyntaxNode messageArgument, SemanticModel model) =>
+        messageArgument.DescendantNodesAndSelf().FirstOrDefault(x =>
+            x.Kind() is SyntaxKind.InterpolatedStringExpression or SyntaxKind.AddExpression
+            || IsStringFormatInvocation(x, model));
+
+    private static bool IsStringFormatInvocation(SyntaxNode node, SemanticModel model) =>
+        node is InvocationExpressionSyntax invocation
+        && node.GetName() == "Format"
+        && model.GetSymbolInfo(invocation).Symbol is IMethodSymbol method
+        && KnownType.System_String.Matches(method.ContainingType);
+}

analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs

@@ -151,6 +151,8 @@ public sealed partial class KnownType
         public static readonly KnownType NHibernate_Impl_AbstractSessionImpl = new("NHibernate.Impl.AbstractSessionImpl");
         public static readonly KnownType NLog_ILogger = new("NLog.ILogger");
         public static readonly KnownType NLog_ILoggerBase = new("NLog.ILoggerBase");
+        public static readonly KnownType NLog_ILoggerExtensions = new("NLog.ILoggerExtensions");
+        public static readonly KnownType NLog_Logger = new("NLog.Logger");
         public static readonly KnownType NLog_LogManager = new("NLog.LogManager");
         public static readonly KnownType NUnit_Framework_Assert = new("NUnit.Framework.Assert");
         public static readonly KnownType NUnit_Framework_AssertionException = new("NUnit.Framework.AssertionException");
@@ -174,6 +176,8 @@ public sealed partial class KnownType
         public static readonly KnownType Serilog_LoggerConfiguration = new("Serilog.LoggerConfiguration");
         public static readonly KnownType ServiceStack_OrmLite_OrmLiteReadApi = new("ServiceStack.OrmLite.OrmLiteReadApi");
         public static readonly KnownType ServiceStack_OrmLite_OrmLiteReadApiAsync = new("ServiceStack.OrmLite.OrmLiteReadApiAsync");
+        public static readonly KnownType Serilog_ILogger = new("Serilog.ILogger");
+        public static readonly KnownType Serilog_Log = new("Serilog.Log");
         public static readonly KnownType System_Action = new("System.Action");
         public static readonly KnownType System_Action_T = new("System.Action", "T");
         public static readonly KnownType System_Action_T1_T2 = new("System.Action", "T1", "T2");
@@ -377,6 +381,7 @@ public sealed partial class KnownType
         public static readonly KnownType System_Numerics_IEqualityOperators_TSelf_TOther_TResult = new("System.Numerics.IEqualityOperators", "TSelf", "TOther", "TResult");
         public static readonly KnownType System_Numerics_IFloatingPointIeee754_TSelf = new("System.Numerics.IFloatingPointIeee754", "TSelf");
         public static readonly KnownType System_Object = new("System.Object");
+        public static readonly KnownType System_Object_Array = new("System.Object") { IsArray = true };
         public static readonly KnownType System_ObsoleteAttribute = new("System.ObsoleteAttribute");
         public static readonly KnownType System_OutOfMemoryException = new("System.OutOfMemoryException");
         public static readonly KnownType System_Random = new("System.Random");

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

@@ -2553,7 +2553,7 @@ internal static class RuleTypeMappingCS
         // ["S2626"],
         // ["S2627"],
         // ["S2628"],
-        // ["S2629"],
+        ["S2629"] = "CODE_SMELL",
         // ["S2630"],
         // ["S2631"],
         // ["S2632"],