Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

diagnostic to detect malformed format strings in lsg template message #81503

Merged
merged 5 commits into from
Feb 5, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/project/list-of-diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ The diagnostic id values reserved for .NET Libraries analyzer warnings are `SYSL
| __`SYSLIB1019`__ | Couldn't find a field of type Microsoft.Extensions.Logging.ILogger |
| __`SYSLIB1020`__ | Found multiple fields of type Microsoft.Extensions.Logging.ILogger |
| __`SYSLIB1021`__ | Can't have the same template with different casing |
| __`SYSLIB1022`__ | Can't have malformed format strings (like dangling {, etc) |
| __`SYSLIB1022`__ | Logging method contains malformed format strings |
| __`SYSLIB1023`__ | Generating more than 6 arguments is not supported |
| __`SYSLIB1024`__ | Argument is using the unsupported out parameter modifier |
| __`SYSLIB1025`__ | Multiple logging methods cannot use the same event name within a class |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public static class DiagnosticDescriptors

public static DiagnosticDescriptor MalformedFormatStrings { get; } = new DiagnosticDescriptor(
id: "SYSLIB1022",
title: new LocalizableResourceString(nameof(SR.MalformedFormatStringsMessage), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
title: new LocalizableResourceString(nameof(SR.MalformedFormatStringsTitle), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
messageFormat: new LocalizableResourceString(nameof(SR.MalformedFormatStringsMessage), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
category: "LoggingGenerator",
DiagnosticSeverity.Error,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,15 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
SkipEnabledCheck = skipEnabledCheck
};

ExtractTemplates(message, lm.TemplateMap, lm.TemplateList);

bool keepMethod = true; // whether or not we want to keep the method definition or if it's got errors making it so we should discard it instead

bool success = ExtractTemplates(message, lm.TemplateMap, lm.TemplateList);
if (!success)
{
Diag(DiagnosticDescriptors.MalformedFormatStrings, method.Identifier.GetLocation(), method.Identifier.ToString());
keepMethod = false;
}

if (lm.Name[0] == '_')
{
// can't have logging method names that start with _ since that can lead to conflicting symbol names
Expand Down Expand Up @@ -595,42 +601,56 @@ private bool IsBaseOrIdentity(ITypeSymbol source, ITypeSymbol dest)
/// <summary>
/// Finds the template arguments contained in the message string.
/// </summary>
private static void ExtractTemplates(string? message, IDictionary<string, string> templateMap, List<string> templateList)
/// <returns>A value indicating whether the extraction was successful.</returns>
private static bool ExtractTemplates(string? message, IDictionary<string, string> templateMap, List<string> templateList)
{
if (string.IsNullOrEmpty(message))
{
return;
return true;
}

int scanIndex = 0;
int endIndex = message!.Length;
int endIndex = message.Length;

bool success = true;
while (scanIndex < endIndex)
{
int openBraceIndex = FindBraceIndex(message, '{', scanIndex, endIndex);
int closeBraceIndex = FindBraceIndex(message, '}', openBraceIndex, endIndex);

if (closeBraceIndex == endIndex)
if (openBraceIndex == -2) // found '}' instead of '{'
{
scanIndex = endIndex;
success = false;
break;
}
else
else if (openBraceIndex == -1) // scanned the string and didn't find any remaining '{' or '}'
{
// Format item syntax : { index[,alignment][ :formatString] }.
int formatDelimiterIndex = FindIndexOfAny(message, _formatDelimiters, openBraceIndex, closeBraceIndex);
break;
}

int closeBraceIndex = FindBraceIndex(message, '}', openBraceIndex + 1, endIndex);

string templateName = message.Substring(openBraceIndex + 1, formatDelimiterIndex - openBraceIndex - 1);
templateMap[templateName] = templateName;
templateList.Add(templateName);
scanIndex = closeBraceIndex + 1;
if (closeBraceIndex <= -1) // unclosed '{'
{
success = false;
break;
}

// Format item syntax : { index[,alignment][ :formatString] }.
int formatDelimiterIndex = FindIndexOfAny(message, _formatDelimiters, openBraceIndex, closeBraceIndex);

string templateName = message.Substring(openBraceIndex + 1, formatDelimiterIndex - openBraceIndex - 1);
templateMap[templateName] = templateName;
templateList.Add(templateName);
scanIndex = closeBraceIndex + 1;
}

return success;
}

private static int FindBraceIndex(string message, char brace, int startIndex, int endIndex)
allantargino marked this conversation as resolved.
Show resolved Hide resolved
{
// Example: {{prefix{{{Argument}}}suffix}}.
allantargino marked this conversation as resolved.
Show resolved Hide resolved
int braceIndex = endIndex;
int braceIndex = -1;
int scanIndex = startIndex;
int braceOccurrenceCount = 0;

Expand All @@ -650,22 +670,29 @@ private static int FindBraceIndex(string message, char brace, int startIndex, in
break;
}
}
else if (message[scanIndex] == brace)
else if (message[scanIndex] == '{')
{
if (brace == '}')
if (message[scanIndex] != brace)
allantargino marked this conversation as resolved.
Show resolved Hide resolved
{
if (braceOccurrenceCount == 0)
{
// For '}' pick the first occurrence.
braceIndex = scanIndex;
}
return -2; // not expected
}
else

// For '{' pick the last occurrence.
braceIndex = scanIndex;
braceOccurrenceCount++;
}
else if (message[scanIndex] == '}')
{
if (message[scanIndex] != brace)
allantargino marked this conversation as resolved.
Show resolved Hide resolved
{
// For '{' pick the last occurrence.
braceIndex = scanIndex;
return -2; // not expected
}

if (braceOccurrenceCount == 0)
{
// For '}' pick the first occurrence.
braceIndex = scanIndex;
}
braceOccurrenceCount++;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@
<value>Can't have the same template with different casing</value>
</data>
<data name="MalformedFormatStringsMessage" xml:space="preserve">
<value>Can't have malformed format strings (like dangling {, etc)</value>
<value>Logging method '{0}' contains malformed format strings</value>
</data>
<data name="GeneratingForMax6ArgumentsMessage" xml:space="preserve">
<value>Generating more than 6 arguments is not supported</value>
Expand All @@ -228,4 +228,7 @@
<data name="ShouldntReuseEventNamesTitle" xml:space="preserve">
<value>Multiple logging methods should not use the same event name within a class</value>
</data>
</root>
<data name="MalformedFormatStringsTitle" xml:space="preserve">
<value>Logging method contains malformed format strings</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsMessage">
<source>Can't have malformed format strings (like dangling {, etc)</source>
<target state="translated">Nemůže mít chybně formátované řetězce (třeba neuzavřené závorky { atd.).</target>
<source>Logging method '{0}' contains malformed format strings</source>
<target state="new">Logging method '{0}' contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsTitle">
<source>Logging method contains malformed format strings</source>
<target state="new">Logging method contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MissingLogLevelMessage">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsMessage">
<source>Can't have malformed format strings (like dangling {, etc)</source>
<target state="translated">Nicht wohlgeformte Formatzeichenfolgen (beispielsweise mit überzähligen geschweiften Klammern) sind unzulässig.</target>
<source>Logging method '{0}' contains malformed format strings</source>
<target state="new">Logging method '{0}' contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsTitle">
<source>Logging method contains malformed format strings</source>
<target state="new">Logging method contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MissingLogLevelMessage">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsMessage">
<source>Can't have malformed format strings (like dangling {, etc)</source>
<target state="translated">No puede tener cadenas con formato incorrecto (como { final, etc.)</target>
<source>Logging method '{0}' contains malformed format strings</source>
<target state="new">Logging method '{0}' contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsTitle">
<source>Logging method contains malformed format strings</source>
<target state="new">Logging method contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MissingLogLevelMessage">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsMessage">
<source>Can't have malformed format strings (like dangling {, etc)</source>
<target state="translated">Chaînes de format incorrect (par exemple { non fermée, etc.)</target>
<source>Logging method '{0}' contains malformed format strings</source>
<target state="new">Logging method '{0}' contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsTitle">
<source>Logging method contains malformed format strings</source>
<target state="new">Logging method contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MissingLogLevelMessage">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsMessage">
<source>Can't have malformed format strings (like dangling {, etc)</source>
<target state="translated">Impossibile avere stringhe di formato non valido (ad esempio, { tralasciato e così via)</target>
<source>Logging method '{0}' contains malformed format strings</source>
<target state="new">Logging method '{0}' contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsTitle">
<source>Logging method contains malformed format strings</source>
<target state="new">Logging method contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MissingLogLevelMessage">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsMessage">
<source>Can't have malformed format strings (like dangling {, etc)</source>
<target state="translated">(dangling {など) 誤った形式の文字列を持つことはできません</target>
<source>Logging method '{0}' contains malformed format strings</source>
<target state="new">Logging method '{0}' contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsTitle">
<source>Logging method contains malformed format strings</source>
<target state="new">Logging method contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MissingLogLevelMessage">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsMessage">
<source>Can't have malformed format strings (like dangling {, etc)</source>
<target state="translated">잘못된 형식의 문자열(예: 짝이 맞지 않는 중괄호({))은 사용할 수 없음</target>
<source>Logging method '{0}' contains malformed format strings</source>
<target state="new">Logging method '{0}' contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsTitle">
<source>Logging method contains malformed format strings</source>
<target state="new">Logging method contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MissingLogLevelMessage">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsMessage">
<source>Can't have malformed format strings (like dangling {, etc)</source>
<target state="translated">Nie może zawierać źle sformułowanego formatu ciągów (takich jak zawieszonego znaku „{”, itp.)</target>
<source>Logging method '{0}' contains malformed format strings</source>
<target state="new">Logging method '{0}' contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MalformedFormatStringsTitle">
<source>Logging method contains malformed format strings</source>
<target state="new">Logging method contains malformed format strings</target>
<note />
</trans-unit>
<trans-unit id="MissingLogLevelMessage">
Expand Down
Loading