diff --git a/SerilogAnalyzer/SerilogAnalyzer.Test/ConvertMessageTemplateAnalyzerTests.cs b/SerilogAnalyzer/SerilogAnalyzer.Test/ConvertMessageTemplateAnalyzerTests.cs index c80ada9..4c5ff90 100644 --- 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/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.StringConcat.cs b/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.StringConcat.cs index 10b5808..7c41cb2 100644 --- a/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.StringConcat.cs +++ b/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.StringConcat.cs @@ -56,8 +56,9 @@ void FindExpressions(ExpressionSyntax exp) } FindExpressions(stringConcat); - var sb = new StringBuilder("$\""); + var sb = new StringBuilder(); var replacements = new List(); + bool shouldUseVerbatim = false; int argumentPosition = 0; foreach (var child in concatExpressions) { @@ -65,6 +66,7 @@ void FindExpressions(ExpressionSyntax exp) { 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; + } } } \ No newline at end of file diff --git a/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.cs b/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.cs index c8b2b1d..ecfbff1 100644 --- a/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.cs +++ b/SerilogAnalyzer/SerilogAnalyzer/ConvertToMessageTemplateCodeRefactoringProvider.cs @@ -77,7 +77,12 @@ private static async Task 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 InlineFormatAndArgumentsIntoLoggerStatementA } } - sb.Append("\""); + sb.Append('"'); var messageTemplate = SyntaxFactory.Argument(SyntaxFactory.ParseExpression(sb.ToString())); var seperatedSyntax = loggerArguments.Replace(loggerArguments[argumentIndex], messageTemplate); diff --git a/SerilogAnalyzer/SerilogAnalyzer/ObjectDisplay.cs b/SerilogAnalyzer/SerilogAnalyzer/ObjectDisplay.cs new file mode 100644 index 0000000..f74aad7 --- /dev/null +++ b/SerilogAnalyzer/SerilogAnalyzer/ObjectDisplay.cs @@ -0,0 +1,189 @@ +// lifted from https://github.com/dotnet/roslyn/blob/e7869bdaa642214a142d27e6a77263e0e3ba1a66/src/Compilers/CSharp/Portable/SymbolDisplay/ObjectDisplay.cs +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis.CSharp; +using System; +using System.Diagnostics; +using System.Globalization; +using System.Text; + +namespace SerilogAnalyzer +{ +#pragma warning disable RS0010 + /// + /// Displays a value in the C# style. + /// + /// + /// Separate from because we want to link this functionality into + /// the Formatter project and we don't want it to be public there. + /// + /// +#pragma warning restore RS0010 + class ObjectDisplay + { + /// + /// Returns true if the character should be replaced and sets + /// to the replacement text. + /// + private static bool TryReplaceChar(char c, out string replaceWith) + { + replaceWith = null; + switch (c) + { + case '\\': + replaceWith = "\\\\"; + break; + case '\0': + replaceWith = "\\0"; + break; + case '\a': + replaceWith = "\\a"; + break; + case '\b': + replaceWith = "\\b"; + break; + case '\f': + replaceWith = "\\f"; + break; + case '\n': + replaceWith = "\\n"; + break; + case '\r': + replaceWith = "\\r"; + break; + case '\t': + replaceWith = "\\t"; + break; + case '\v': + replaceWith = "\\v"; + break; + } + + if (replaceWith != null) + { + return true; + } + + if (NeedsEscaping(CharUnicodeInfo.GetUnicodeCategory(c))) + { + replaceWith = "\\u" + ((int)c).ToString("x4"); + return true; + } + + return false; + } + + private static bool NeedsEscaping(UnicodeCategory category) + { + switch (category) + { + case UnicodeCategory.Control: + case UnicodeCategory.OtherNotAssigned: + case UnicodeCategory.ParagraphSeparator: + case UnicodeCategory.LineSeparator: + case UnicodeCategory.Surrogate: + return true; + default: + return false; + } + } + + /// + /// Returns a C# string literal with the given value. + /// + /// The value that the resulting string literal should have. + /// A string literal with the given value. + /// + /// Optionally escapes non-printable characters. + /// + public static string FormatLiteral(string value, bool useQuotes, bool escapeNonPrintable) + { + if (value == null) + { + throw new ArgumentNullException(nameof(value)); + } + + const char quote = '"'; + + var builder = new StringBuilder(); + var isVerbatim = useQuotes && !escapeNonPrintable && ContainsNewLine(value); + + if (useQuotes) + { + if (isVerbatim) + { + builder.Append('@'); + } + builder.Append(quote); + } + + for (int i = 0; i < value.Length; i++) + { + char c = value[i]; + if (escapeNonPrintable && CharUnicodeInfo.GetUnicodeCategory(c) == UnicodeCategory.Surrogate) + { + var category = CharUnicodeInfo.GetUnicodeCategory(value, i); + if (category == UnicodeCategory.Surrogate) + { + // an unpaired surrogate + builder.Append("\\u" + ((int)c).ToString("x4")); + } + else if (NeedsEscaping(category)) + { + // a surrogate pair that needs to be escaped + var unicode = char.ConvertToUtf32(value, i); + builder.Append("\\U" + unicode.ToString("x8")); + i++; // skip the already-encoded second surrogate of the pair + } + else + { + // copy a printable surrogate pair directly + builder.Append(c); + builder.Append(value[++i]); + } + } + else if (escapeNonPrintable && TryReplaceChar(c, out var replaceWith)) + { + builder.Append(replaceWith); + } + else if (useQuotes && c == quote) + { + if (isVerbatim) + { + builder.Append(quote); + builder.Append(quote); + } + else + { + builder.Append('\\'); + builder.Append(quote); + } + } + else + { + builder.Append(c); + } + } + + if (useQuotes) + { + builder.Append(quote); + } + + return builder.ToString(); + } + + private static bool ContainsNewLine(string s) + { + foreach (char c in s) + { + if (SyntaxFacts.IsNewLine(c)) + { + return true; + } + } + + return false; + } + } +} \ No newline at end of file diff --git a/SerilogAnalyzer/SerilogAnalyzer/SerilogAnalyzer.csproj b/SerilogAnalyzer/SerilogAnalyzer/SerilogAnalyzer.csproj index 6bb8aee..485701b 100644 --- a/SerilogAnalyzer/SerilogAnalyzer/SerilogAnalyzer.csproj +++ b/SerilogAnalyzer/SerilogAnalyzer/SerilogAnalyzer.csproj @@ -39,6 +39,7 @@ +