String.Concat refactoring: preserve linebreaks, fixes #23

- Additionally determine whether to use a normal or verbatim string based on whether a verbatim string is used in the source that has either linebreaks or quotes inside
Suchiman · Aug 23, 2017 · 7926e1a · 7926e1a
1 parent 42036e3
commit 7926e1a
Show file tree

Hide file tree

Showing 5 changed files with 370 additions and 5 deletions.
diff --git a/SerilogAnalyzer/SerilogAnalyzer.Test/ConvertMessageTemplateAnalyzerTests.cs b/SerilogAnalyzer/SerilogAnalyzer.Test/ConvertMessageTemplateAnalyzerTests.cs
@@ -781,6 +781,133 @@ public static void Test()
             VerifyCSharpFix(src, fixtest);
         }
 
+        [TestMethod]
+        public void TestStringConcatWithLineBreaks()
+        {
+            string src = @"
+using Serilog;
+
+class TypeName
+{
+    public static void Test()
+    {
+        string name = null;
+        Log.Verbose(""Hello World\nName: '"" + name + ""'\n"");
+    }
+}";
+
+            var expected = new DiagnosticResult
+            {
+                Id = "Serilog004",
+                Message = String.Format("MessageTemplate argument {0} is not constant", @"""Hello World\nName: '"" + name + ""'\n"""),
+                Severity = DiagnosticSeverity.Warning,
+                Locations = new[]
+                {
+                    new DiagnosticResultLocation("Test0.cs", 9, 21, 37)
+                }
+            };
+            VerifyCSharpDiagnostic(src, expected);
+
+            var fixtest = @"
+using Serilog;
+
+class TypeName
+{
+    public static void Test()
+    {
+        string name = null;
+        Log.Verbose(""Hello World\nName: '{Name}'\n"", name);
+    }
+}";
+            VerifyCSharpFix(src, fixtest);
+        }
+
+        [TestMethod]
+        public void TestStringConcatWithVerbatim()
+        {
+            string src = @"
+using Serilog;
+
+class TypeName
+{
+    public static void Test()
+    {
+        string name = null;
+        Log.Verbose(@""Hello World
+Name: '"" + name + ""'\r\n"");
+    }
+}";
+
+            var expected = new DiagnosticResult
+            {
+                Id = "Serilog004",
+                Message = String.Format("MessageTemplate argument {0} is not constant", @"@""Hello World
+Name: '"" + name + ""'\r\n"""),
+                Severity = DiagnosticSeverity.Warning,
+                Locations = new[]
+                {
+                    new DiagnosticResultLocation("Test0.cs", 9, 21, 40)
+                }
+            };
+            VerifyCSharpDiagnostic(src, expected);
+
+            var fixtest = @"
+using Serilog;
+
+class TypeName
+{
+    public static void Test()
+    {
+        string name = null;
+        Log.Verbose(@""Hello World
+Name: '{Name}'
+"", name);
+    }
+}";
+            VerifyCSharpFix(src, fixtest);
+        }
+
+        [TestMethod]
+        public void TestStringConcatWithVerbatimIgnored()
+        {
+            string src = @"
+using Serilog;
+
+class TypeName
+{
+    public static void Test()
+    {
+        string name = null;
+        Log.Verbose(""Hello World\nName: '"" + name + @""'"");
+    }
+}";
+
+            var expected = new DiagnosticResult
+            {
+                Id = "Serilog004",
+                Message = String.Format("MessageTemplate argument {0} is not constant", @"""Hello World\nName: '"" + name + @""'"""),
+                Severity = DiagnosticSeverity.Warning,
+                Locations = new[]
+                {
+                    new DiagnosticResultLocation("Test0.cs", 9, 21, 36)
+                }
+            };
+            VerifyCSharpDiagnostic(src, expected);
+
+            var fixtest = @"
+using Serilog;
+
+class TypeName
+{
+    public static void Test()
+    {
+        string name = null;
+        Log.Verbose(""Hello World\nName: '{Name}'"", name);
+    }
+}";
+            VerifyCSharpFix(src, fixtest);
+        }
+
         [TestMethod]
         public void TestStringConcatComplex()
         {

diff --git a/...gAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.StringConcat.cs b/...gAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.StringConcat.cs
@@ -56,15 +56,17 @@ void FindExpressions(ExpressionSyntax exp)
             }
             FindExpressions(stringConcat);
 
-            var sb = new StringBuilder("$\"");
+            var sb = new StringBuilder();
             var replacements = new List<string>();
+            bool shouldUseVerbatim = false;
             int argumentPosition = 0;
             foreach (var child in concatExpressions)
             {
                 switch (child)
                 {
                     case LiteralExpressionSyntax literal:
                         sb.Append(literal.Token.ValueText);
+                        shouldUseVerbatim |= literal.Token.Text.StartsWith("@", System.StringComparison.Ordinal) && ContainsQuotesOrLineBreaks(literal.Token.ValueText);
                         break;
                     case ExpressionSyntax exp:
 
@@ -76,10 +78,51 @@ void FindExpressions(ExpressionSyntax exp)
                         break;
                 }
             }
-            sb.Append("\"");
 
-            format = (InterpolatedStringExpressionSyntax)SyntaxFactory.ParseExpression(sb.ToString());
+            if (shouldUseVerbatim)
+            {
+                for (int i = 0; i < sb.Length; i++)
+                {
+                    if (IsForbiddenInVerbatimString(sb[i]))
+                    {
+                        shouldUseVerbatim = false;
+                        break;
+                    }
+                }
+            }
+
+            var text = ObjectDisplay.FormatLiteral(sb.ToString(), useQuotes: true, escapeNonPrintable: !shouldUseVerbatim);
+
+            format = (InterpolatedStringExpressionSyntax)SyntaxFactory.ParseExpression("$" + text);
             expressions = concatExpressions.Where(x => !(x is LiteralExpressionSyntax)).ToList();
         }
+
+        private static bool IsForbiddenInVerbatimString(char c)
+        {
+            switch (c)
+            {
+                case '\a':
+                case '\b':
+                case '\f':
+                case '\v':
+                case '\0':
+                    return true;
+            }
+
+            return false;
+        }
+
+        private static bool ContainsQuotesOrLineBreaks(string s)
+        {
+            foreach (char c in s)
+            {
+                if (c == '\r' || c == '\n' || c == '"')
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
     }
 }
diff --git a/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.cs b/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.cs
@@ -77,7 +77,12 @@ private static async Task<Document> InlineFormatAndArgumentsIntoLoggerStatementA
             var loggerArguments = logger.ArgumentList.Arguments;
             var argumentIndex = loggerArguments.IndexOf(x => x.Expression == originalTemplateExpression);
 
-            var sb = new StringBuilder("\"");
+            var sb = new StringBuilder();
+            if (format.StringStartToken.ValueText.Contains("@"))
+            {
+                sb.Append('@');
+            }
+            sb.Append('"');
 
             var semanticModel = await document.GetSemanticModelAsync(cancellationToken);
             var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken);
@@ -153,7 +158,7 @@ private static async Task<Document> InlineFormatAndArgumentsIntoLoggerStatementA
                 }
             }
 
-            sb.Append("\"");
+            sb.Append('"');
             var messageTemplate = SyntaxFactory.Argument(SyntaxFactory.ParseExpression(sb.ToString()));
 
             var seperatedSyntax = loggerArguments.Replace(loggerArguments[argumentIndex], messageTemplate);