Template rules update after peach validation part#2 (#8820)

SonarSource · Feb 28, 2024 · f96478a · f96478a
1 parent f80bdbe
commit f96478a
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 15 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/MessageTemplateAnalyzer.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/MessageTemplateAnalyzer.cs
@@ -27,6 +27,12 @@ namespace SonarAnalyzer.Rules.CSharp;
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 public sealed class MessageTemplateAnalyzer : SonarDiagnosticAnalyzer
 {
+    private static readonly ImmutableHashSet<SyntaxKind> ValidTemplateKinds = ImmutableHashSet.Create(
+        SyntaxKind.StringLiteralExpression,
+        SyntaxKind.AddExpression,
+        SyntaxKind.InterpolatedStringExpression,
+        SyntaxKind.InterpolatedStringText);
+
     private static readonly ImmutableHashSet<IMessageTemplateCheck> Checks = ImmutableHashSet.Create<IMessageTemplateCheck>(
                new NamedPlaceholdersShouldBeUnique(),
                new UsePascalCaseForNamedPlaceHolders());
@@ -41,7 +47,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
             var enabledChecks = Checks.Where(x => x.Rule.IsEnabled(c)).ToArray();
             if (enabledChecks.Length > 0
                 && TemplateArgument(invocation, c.SemanticModel) is { } argument
-                && argument.Expression.IsKind(SyntaxKind.StringLiteralExpression)
+                && HasValidExpression(argument)
                 && Helpers.MessageTemplates.Parse(argument.Expression.ToString()) is { Success: true } result)
             {
                 foreach (var check in enabledChecks)
@@ -74,4 +80,7 @@ private static ArgumentSyntax TemplateArgument(InvocationExpressionSyntax invoca
         && argumentsFound.Length == 1
             ? (ArgumentSyntax)argumentsFound[0].Parent
             : null;
+
+    private static bool HasValidExpression(ArgumentSyntax argument) =>
+        argument.Expression.DescendantNodes().All(x => x.IsAnyKind(ValidTemplateKinds));
 }
diff --git a/analyzers/tests/SonarAnalyzer.Test/Helpers/MessageTemplatesTest.cs b/analyzers/tests/SonarAnalyzer.Test/Helpers/MessageTemplatesTest.cs
@@ -58,11 +58,12 @@ public void Parse_NoPlaceholder(string template)
     [DataRow("{$199}", "199", 2, 3)]
     [DataRow("{@0}", "0", 2, 1)]
     [DataRow("{$world_42}", "world_42", 2, 8)]
-    public void Parse_Placeholder(string template, string placeholder, int start, int end)
+    [DataRow(""" "hello" + "{world}" + "!" """, "world", 13, 5)]
+    public void Parse_Placeholder(string template, string placeholder, int start, int length)
     {
         var result = MessageTemplates.Parse(template);
         ShouldBeSuccess(result, 1);
-        ShouldBe(result.Placeholders[0], placeholder, start, end);
+        ShouldBe(result.Placeholders[0], placeholder, start, length);
     }
 
     [DataTestMethod]
@@ -115,16 +116,17 @@ public void Parse_Placeholder_Multiple()
     }
 
     [DataTestMethod]
-    [DataRow("{")]                              // Left bracket is not allowed
-    [DataRow("{{{")]                            // Third left bracket is not allowed (first two are valid)
-    [DataRow("{}")]                             // Empty placeholder is not allowed
-    [DataRow("{{{}}}")]                         // Empty placeholder is not allowed
-    [DataRow("Login failed for {User")]         // Missing closing bracket
-    [DataRow("Login failed for {&User}")]       // Only '@' and '$' are allowed as prefix
-    [DataRow("Login failed for {User_%Name}")]  // Only alphanumerics and '_' are allowed for placeholders
-    [DataRow("Retry attempt {Cnt,r}")]          // The alignment specifier must be numeric
-    [DataRow("Retry attempt {Cnt,}")]           // Empty alignment specifier is not allowed
-    [DataRow("Retry attempt {Cnt:}")]           // Empty format specifier is not allowed
+    [DataRow("{")]                                  // Left bracket is not allowed
+    [DataRow("{{{")]                                // Third left bracket is not allowed (first two are valid)
+    [DataRow("{}")]                                 // Empty placeholder is not allowed
+    [DataRow("{{{}}}")]                             // Empty placeholder is not allowed
+    [DataRow("Login failed for {User")]             // Missing closing bracket
+    [DataRow("Login failed for {&User}")]           // Only '@' and '$' are allowed as prefix
+    [DataRow("Login failed for {User_%Name}")]      // Only alphanumerics and '_' are allowed for placeholders
+    [DataRow("Retry attempt {Cnt,r}")]              // The alignment specifier must be numeric
+    [DataRow("Retry attempt {Cnt,}")]               // Empty alignment specifier is not allowed
+    [DataRow("Retry attempt {Cnt:}")]               // Empty format specifier is not allowed
+    [DataRow(""" "hello {" + "world" + "}" """)]    // '+' and '"' is not allowed in placeholders
     public void Parse_Placeholder_Failure(string template)
     {
         var result = MessageTemplates.Parse(template);

diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/NamedPlaceholdersShouldBeUnique.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/NamedPlaceholdersShouldBeUnique.cs
@@ -45,10 +45,17 @@ void Compliant(ILogger logger, string foo, string bar)
             logger.LogInformation("Hey {{foo}} and {{foo}}", foo, foo);      // Compliant
             logger.LogInformation("Hey {foo} and {{foo}}", foo, foo);        // Compliant
 
-            // Not string literal
+            // Contains Interpolation
             logger.LogWarning($"Hey {foo} and {foo}", foo, foo);             // Compliant
+            logger.LogWarning($"Hey {foo}" + "and {foo}", foo, foo);         // Compliant
+            logger.LogInformation("Hey {foo}" + $"and {foo}" + "{foo}", foo, foo); // Compliant FN
 
-            logger.LogInformation($"Hey {{foo}} and {{foo}}", foo, foo); // FN, we do not parse interpolated strings
+            // Contains Identifier name
+            logger.LogWarning("Hey {" + foo + "} and {" + foo + "}");        // Compliant
+
+            // False Negatives
+            logger.LogInformation($"Hey {{foo}} and {{foo}}", foo, foo);     // Compliant, we get the template value at syntax level, so we don't escape {{
+            logger.LogInformation("Hey" + "{" + "foo} and {foo" +"}");       // Compliant, same as above (I hope this does not exist out there)
         }
 
         void Noncompliant(ILogger logger, string foo, string bar, string baz, Exception ex, EventId eventId)
@@ -139,6 +146,26 @@ and again {foo}
             //   ^^^ @-3 {{Message template placeholder 'foo' is not unique.}}
             //             ^^^ Secondary @-2
                 foo, bar, foo);
+
+
+            // SyntaxKinds
+            logger.LogInformation("Hey {foo}" + "and {foo}", foo, foo);
+            //                          ^^^ {{Message template placeholder 'foo' is not unique.}}
+            //                                        ^^^ Secondary @-1
+            logger.LogInformation("Hey {foo}" + $"and of course" + "{foo}", foo, foo);
+            //                          ^^^ {{Message template placeholder 'foo' is not unique.}}
+            //                                                       ^^^ Secondary @-1
+            logger.LogInformation("Hey {foo}" + $"{{foo}}" + "{foo}", foo, foo);                    // We miss the second placeholder
+            //                          ^^^ {{Message template placeholder 'foo' is not unique.}}
+            //                                                 ^^^ Secondary @-1
+
+            logger.LogInformation(
+                "Hey {foo}"
+            //        ^^^ {{Message template placeholder 'foo' is not unique.}}
+                +
+                "and {foo}", foo, foo);
+            //        ^^^ Secondary
+
         }
 
         // Fake