Review 2

SonarSource · Mar 6, 2024 · 851cbae · 851cbae
1 parent 7111c68
commit 851cbae
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 14 deletions.
diff --git a/...zers/its/expected/ManuallyAddedNoncompliantIssues.CS/S103-IntentionalFindings-net8.0.json b/...zers/its/expected/ManuallyAddedNoncompliantIssues.CS/S103-IntentionalFindings-net8.0.json
@@ -0,0 +1,10 @@
+{
+  "Issues": [
+    {
+      "Id": "S103",
+      "Message": "Split this 219 characters long line (which is greater than 200 authorized).",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6673.cs#L9",
+      "Location": "Line 9 Position 1-220"
+    }
+  ]
+}
diff --git a/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S1128-IntentionalFindings-net8.0.json b/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S1128-IntentionalFindings-net8.0.json
@@ -11,6 +11,12 @@
       "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"
+    },
+    {
+      "Id": "S1128",
+      "Message": "Remove this unnecessary \u0027using\u0027.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6673.cs#L1",
+      "Location": "Line 1 Position 1-14"
     }
   ]
 }
diff --git a/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S1144-IntentionalFindings-net8.0.json b/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S1144-IntentionalFindings-net8.0.json
@@ -17,6 +17,12 @@
       "Message": "Remove the unused private method \u0027Log\u0027.",
       "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": "S1144",
+      "Message": "Remove the unused private method \u0027Log\u0027.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6673.cs#L8",
+      "Location": "Line 8 Position 22-25"
     }
   ]
 }
diff --git a/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S1451-IntentionalFindings-net8.0.json b/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S1451-IntentionalFindings-net8.0.json
@@ -23,6 +23,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/S6670.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/S6673.cs#L1",
+      "Location": "Line 1 Position 1-1"
     }
   ]
 }
diff --git a/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S2325-IntentionalFindings-net8.0.json b/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S2325-IntentionalFindings-net8.0.json
@@ -17,6 +17,12 @@
       "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"
+    },
+    {
+      "Id": "S2325",
+      "Message": "Make \u0027Log\u0027 a static method.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6673.cs#L8",
+      "Location": "Line 8 Position 22-25"
     }
   ]
 }
diff --git a/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S4055-IntentionalFindings-net8.0.json b/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S4055-IntentionalFindings-net8.0.json
@@ -11,6 +11,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/S6670.cs#L10",
       "Location": "Line 10 Position 25-34"
+    },
+    {
+      "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/S6673.cs#L9",
+      "Location": "Line 9 Position 35-60"
     }
   ]
 }
diff --git a/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6673-IntentionalFindings-net8.0.json b/...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6673-IntentionalFindings-net8.0.json
@@ -0,0 +1,10 @@
+{
+  "Issues": [
+    {
+      "Id": "S6673",
+      "Message": "Template placeholders should be in the right order: placeholder \u0027Arg\u0027 does not match with argument \u0027anotherArg\u0027.",
+      "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6673.cs#L9",
+      "Location": "Line 9 Position 42-45"
+    }
+  ]
+}
diff --git a/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6673.cs b/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6673.cs
@@ -0,0 +1,11 @@
+using System;
+using Microsoft.Extensions.Logging;
+
+namespace IntentionalFindings
+{
+    public class S6673
+    {
+        private void Log(ILogger logger, int arg, int anotherArg) =>
+            logger.LogInformation("Arg: {Arg} {AnotherArg}", anotherArg, arg); // Noncompliant (S6673) {{Template placeholders should be in the right order: placeholder 'Arg' does not match with argument 'anotherArg'.}}
+    }
+}
diff --git a/...SonarAnalyzer.CSharp/Rules/MessageTemplates/LoggingTemplatePlaceHoldersShouldBeInOrder.cs b/...SonarAnalyzer.CSharp/Rules/MessageTemplates/LoggingTemplatePlaceHoldersShouldBeInOrder.cs
@@ -52,7 +52,7 @@ public void Execute(SonarSyntaxNodeReportingContext context, InvocationExpressio
 
     private static IEnumerable<SyntaxNode> PlaceholderValues(InvocationExpressionSyntax invocation, IMethodSymbol methodSymbol)
     {
-        var parameters = methodSymbol.Parameters.Where(x => x.Name is "args"
+        var parameters = methodSymbol.Parameters.Where(x => x.Name == "args"
             || x.Name.StartsWith("argument")
             || x.Name.StartsWith("propertyValue"))
             .ToArray();

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/MessageTemplateAnalyzer.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/MessageTemplateAnalyzer.cs
@@ -40,22 +40,28 @@ public sealed class MessageTemplateAnalyzer : SonarDiagnosticAnalyzer
         Checks.Select(x => x.Rule).ToImmutableArray();
 
     protected override void Initialize(SonarAnalysisContext context) =>
-        context.RegisterNodeAction(c =>
+        context.RegisterCompilationStartAction(cc =>
         {
-            var invocation = (InvocationExpressionSyntax)c.Node;
-            var enabledChecks = Checks.Where(x => x.Rule.IsEnabled(c)).ToArray();
-            if (enabledChecks.Length > 0
-                && MessageTemplateExtractor.TemplateArgument(invocation, c.SemanticModel) is { } argument
-                && HasValidExpression(argument)
-                && MessageTemplatesParser.Parse(argument.Expression.ToString()) is { Success: true } result)
+            if (cc.Compilation.ReferencesAny(KnownAssembly.MicrosoftExtensionsLoggingAbstractions, KnownAssembly.Serilog, KnownAssembly.NLog))
             {
-                foreach (var check in enabledChecks)
+                cc.RegisterNodeAction(c =>
                 {
-                    check.Execute(c, invocation, argument, result.Placeholders);
-                }
+                    var invocation = (InvocationExpressionSyntax)c.Node;
+                    var enabledChecks = Checks.Where(x => x.Rule.IsEnabled(c)).ToArray();
+                    if (enabledChecks.Length > 0
+                        && MessageTemplateExtractor.TemplateArgument(invocation, c.SemanticModel) is { } argument
+                        && HasValidExpression(argument)
+                        && MessageTemplatesParser.Parse(argument.Expression.ToString()) is { Success: true } result)
+                    {
+                        foreach (var check in enabledChecks)
+                        {
+                            check.Execute(c, invocation, argument, result.Placeholders);
+                        }
+                    }
+                },
+                SyntaxKind.InvocationExpression);
             }
-        },
-        SyntaxKind.InvocationExpression);
+        });
 
     private static bool HasValidExpression(ArgumentSyntax argument) =>
         argument.Expression.DescendantNodes().All(x => x.IsAnyKind(ValidTemplateKinds));

diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/LoggingTemplatePlaceHoldersShouldBeInOrderTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/LoggingTemplatePlaceHoldersShouldBeInOrderTest.cs
@@ -70,7 +70,7 @@ public void Method(ILogger logger, int arg1, int arg2)
     [DataRow("Warning")]
     [DataRow("Verbose")]
     public void LoggingTemplatePlaceHoldersShouldBeInOrder_Serilog_CS(string methodName) =>
-    Builder.AddSnippet($$"""
+        Builder.AddSnippet($$"""
             using Serilog;
             using Serilog.Events;
 
@@ -107,6 +107,27 @@ public void Method(ILogger iLogger, Logger logger, MyLogger myLogger, int arg1,
                                                                             // Secondary @-1
                 }
             }
+
             public class MyLogger : Logger { }
             """).Verify();
+
+    [TestMethod]
+    public void LoggingTemplatePlaceHoldersShouldBeInOrder_FakeLoggerWithSameName() =>
+        new VerifierBuilder<MessageTemplateAnalyzer>()
+            .WithOnlyDiagnostics(LoggingTemplatePlaceHoldersShouldBeInOrder.S6673)
+            .AddSnippet("""
+                public class Program
+                {
+                    public void Method(ILogger logger, int arg1, int arg2)
+                    {
+                        logger.Info("{Arg1} {Arg2}", arg1, arg2);     // Compliant
+                        logger.Info("{Arg1} {Arg2}", arg2, arg1);     // Compliant - the method is not from any of the known logging frameworks
+                    }
+                }
+
+                public interface ILogger
+                {
+                    void Info(string message, params object[] args);
+                }
+                """).Verify();
 }