Skip to content

Commit c4644e8

Browse files
AArnottAArnott
committed
Remove slow regex from threading analyzers
1 parent ed9bfee commit c4644e8

5 files changed

Lines changed: 479 additions & 40 deletions

File tree

‎src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs‎

Lines changed: 37 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,6 @@ public static class CommonInterest
6969

7070
private const string GetAwaiterMethodName = "GetAwaiter";
7171

72-
private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(5); // Prevent expensive CPU hang in Regex.Match if backtracking occurs due to pathological input (see #485).
73-
74-
private static readonly Regex NegatableTypeOrMemberReferenceRegex = new Regex(@"^(?<negated>!)?\[(?<typeName>[^\[\]\:]+)+\](?:\:\:(?<memberName>\S+))?\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout);
75-
76-
private static readonly Regex MemberReferenceRegex = new Regex(@"^\[(?<typeName>[^\[\]\:]+)+\]::(?<memberName>\S+)\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout);
77-
78-
/// <summary>
79-
/// An array with '.' as its only element.
80-
/// </summary>
81-
private static readonly char[] QualifiedIdentifierSeparators = ['.'];
82-
8372
public static IEnumerable<QualifiedMember> ReadMethods(AnalyzerOptions analyzerOptions, Regex fileNamePattern, CancellationToken cancellationToken)
8473
{
8574
foreach (string line in ReadAdditionalFiles(analyzerOptions, fileNamePattern, cancellationToken))
@@ -92,28 +81,15 @@ public static IEnumerable<TypeMatchSpec> ReadTypesAndMembers(AnalyzerOptions ana
9281
{
9382
foreach (string line in ReadAdditionalFiles(analyzerOptions, fileNamePattern, cancellationToken))
9483
{
95-
Match? match = null;
96-
try
97-
{
98-
match = NegatableTypeOrMemberReferenceRegex.Match(line);
99-
}
100-
catch (RegexMatchTimeoutException)
101-
{
102-
throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}");
103-
}
104-
105-
if (!match.Success)
84+
if (!CommonInterestParsing.TryParseNegatableTypeOrMemberReference(line, out bool negated, out ReadOnlyMemory<char> typeNameMemory, out string? memberNameValue))
10685
{
10786
throw new InvalidOperationException($"Parsing error on line: {line}");
10887
}
10988

110-
bool inverted = match.Groups["negated"].Success;
111-
string[] typeNameElements = match.Groups["typeName"].Value.Split(QualifiedIdentifierSeparators);
112-
string typeName = typeNameElements[typeNameElements.Length - 1];
113-
var containingNamespace = typeNameElements.Take(typeNameElements.Length - 1).ToImmutableArray();
89+
(ImmutableArray<string> containingNamespace, string? typeName) = SplitQualifiedIdentifier(typeNameMemory);
11490
var type = new QualifiedType(containingNamespace, typeName);
115-
QualifiedMember member = match.Groups["memberName"].Success ? new QualifiedMember(type, match.Groups["memberName"].Value) : default(QualifiedMember);
116-
yield return new TypeMatchSpec(type, member, inverted);
91+
QualifiedMember member = memberNameValue is not null ? new QualifiedMember(type, memberNameValue) : default(QualifiedMember);
92+
yield return new TypeMatchSpec(type, member, negated);
11793
}
11894
}
11995

@@ -345,26 +321,47 @@ public static IEnumerable<string> ReadLinesFromAdditionalFile(SourceText text)
345321

346322
public static QualifiedMember ParseAdditionalFileMethodLine(string line)
347323
{
348-
Match? match = null;
349-
try
324+
if (!CommonInterestParsing.TryParseMemberReference(line, out ReadOnlyMemory<char> typeNameMemory, out string? memberName))
350325
{
351-
match = MemberReferenceRegex.Match(line);
326+
throw new InvalidOperationException($"Parsing error on line: {line}");
352327
}
353-
catch (RegexMatchTimeoutException)
328+
329+
(ImmutableArray<string> containingNamespace, string? typeName) = SplitQualifiedIdentifier(typeNameMemory);
330+
var containingType = new QualifiedType(containingNamespace, typeName);
331+
return new QualifiedMember(containingType, memberName!);
332+
}
333+
334+
/// <summary>
335+
/// Splits a qualified type name (e.g. <c>My.Namespace.MyType</c>) into its containing namespace
336+
/// segments and the simple type name, without allocating an intermediate joined string.
337+
/// </summary>
338+
/// <param name="qualifiedName">The qualified type name as a memory slice.</param>
339+
/// <returns>The namespace segments and the simple type name.</returns>
340+
private static (ImmutableArray<string> ContainingNamespace, string TypeName) SplitQualifiedIdentifier(ReadOnlyMemory<char> qualifiedName)
341+
{
342+
int lastDot = qualifiedName.Span.LastIndexOf('.');
343+
if (lastDot < 0)
354344
{
355-
throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}");
345+
return (ImmutableArray<string>.Empty, qualifiedName.ToString());
356346
}
357347

358-
if (!match.Success)
348+
string typeName = qualifiedName.Slice(lastDot + 1).ToString();
349+
ReadOnlyMemory<char> nsPart = qualifiedName.Slice(0, lastDot);
350+
ImmutableArray<string>.Builder nsBuilder = ImmutableArray.CreateBuilder<string>();
351+
while (!nsPart.IsEmpty)
359352
{
360-
throw new InvalidOperationException($"Parsing error on line: {line}");
353+
int dot = nsPart.Span.IndexOf('.');
354+
if (dot < 0)
355+
{
356+
nsBuilder.Add(nsPart.ToString());
357+
break;
358+
}
359+
360+
nsBuilder.Add(nsPart.Slice(0, dot).ToString());
361+
nsPart = nsPart.Slice(dot + 1);
361362
}
362363

363-
string methodName = match.Groups["memberName"].Value;
364-
string[] typeNameElements = match.Groups["typeName"].Value.Split(QualifiedIdentifierSeparators);
365-
string typeName = typeNameElements[typeNameElements.Length - 1];
366-
var containingType = new QualifiedType(typeNameElements.Take(typeNameElements.Length - 1).ToImmutableArray(), typeName);
367-
return new QualifiedMember(containingType, methodName);
364+
return (nsBuilder.ToImmutable(), typeName);
368365
}
369366

370367
private static bool TestGetAwaiterMethod(IMethodSymbol getAwaiterMethod)

‎src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterestParsing.cs‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System;
5+
6+
namespace Microsoft.VisualStudio.Threading.Analyzers;
7+
8+
/// <summary>
9+
/// Parsing helpers for additional-file lines used by <see cref="CommonInterest" />.
10+
/// This class is intentionally kept free of Roslyn dependencies so it can be
11+
/// linked directly into test projects for unit testing.
12+
/// </summary>
13+
internal static class CommonInterestParsing
14+
{
15+
/// <summary>
16+
/// Parses a line that may begin with an optional <c>!</c>, followed by <c>[TypeName]</c>,
17+
/// and optionally <c>::MemberName</c>, without using regular expressions.
18+
/// </summary>
19+
/// <param name="line">The line to parse.</param>
20+
/// <param name="negated"><see langword="true" /> if the line begins with '!'.</param>
21+
/// <param name="typeName">The type name parsed from the brackets.</param>
22+
/// <param name="memberName">The member name after '::', or <see langword="null" /> if not present.</param>
23+
/// <returns><see langword="true" /> if parsing succeeded.</returns>
24+
internal static bool TryParseNegatableTypeOrMemberReference(string line, out bool negated, out ReadOnlyMemory<char> typeName, out string? memberName)
25+
{
26+
negated = false;
27+
typeName = default;
28+
memberName = null;
29+
30+
ReadOnlySpan<char> span = line.AsSpan();
31+
int pos = 0;
32+
33+
// Optional negation prefix.
34+
if (pos < span.Length && span[pos] == '!')
35+
{
36+
negated = true;
37+
pos++;
38+
}
39+
40+
// Required '[TypeName]'.
41+
int bracketStart = pos;
42+
if (!TryParseBracketedTypeName(span, ref pos, out _))
43+
{
44+
return false;
45+
}
46+
47+
// Compute memory slice for type name (between the brackets), using recorded bracket position.
48+
ReadOnlyMemory<char> typeNameMemory = line.AsMemory(bracketStart + 1, pos - bracketStart - 2);
49+
50+
// Optional '::memberName'.
51+
ReadOnlySpan<char> memberNameSpan = default;
52+
if (pos + 1 < span.Length && span[pos] == ':' && span[pos + 1] == ':')
53+
{
54+
pos += 2;
55+
int memberNameStart = pos;
56+
while (pos < span.Length && !char.IsWhiteSpace(span[pos]))
57+
{
58+
pos++;
59+
}
60+
61+
if (pos == memberNameStart)
62+
{
63+
// '::' present but no member name follows.
64+
return false;
65+
}
66+
67+
memberNameSpan = span.Slice(memberNameStart, pos - memberNameStart);
68+
}
69+
70+
// Allow only trailing whitespace.
71+
while (pos < span.Length && char.IsWhiteSpace(span[pos]))
72+
{
73+
pos++;
74+
}
75+
76+
if (pos != span.Length)
77+
{
78+
return false;
79+
}
80+
81+
// Only allocate strings after full validation.
82+
typeName = typeNameMemory;
83+
memberName = memberNameSpan.IsEmpty ? null : memberNameSpan.ToString();
84+
return true;
85+
}
86+
87+
/// <summary>
88+
/// Parses a line of the form <c>[TypeName]::MemberName</c>, without using regular expressions.
89+
/// </summary>
90+
/// <param name="line">The line to parse.</param>
91+
/// <param name="typeName">The type name parsed from the brackets.</param>
92+
/// <param name="memberName">The member name after '::'.</param>
93+
/// <returns><see langword="true" /> if parsing succeeded.</returns>
94+
internal static bool TryParseMemberReference(string line, out ReadOnlyMemory<char> typeName, out string? memberName)
95+
{
96+
typeName = default;
97+
memberName = null;
98+
99+
ReadOnlySpan<char> span = line.AsSpan();
100+
int pos = 0;
101+
102+
// Required '[TypeName]'.
103+
int bracketStart = pos;
104+
if (!TryParseBracketedTypeName(span, ref pos, out _))
105+
{
106+
return false;
107+
}
108+
109+
// Compute memory slice for type name (between the brackets), using recorded bracket position.
110+
ReadOnlyMemory<char> typeNameMemory = line.AsMemory(bracketStart + 1, pos - bracketStart - 2);
111+
112+
// Required '::'.
113+
if (pos + 1 >= span.Length || span[pos] != ':' || span[pos + 1] != ':')
114+
{
115+
return false;
116+
}
117+
118+
pos += 2;
119+
120+
// Member name: one or more non-whitespace chars.
121+
int memberNameStart = pos;
122+
while (pos < span.Length && !char.IsWhiteSpace(span[pos]))
123+
{
124+
pos++;
125+
}
126+
127+
if (pos == memberNameStart)
128+
{
129+
return false;
130+
}
131+
132+
ReadOnlySpan<char> memberNameSpan = span.Slice(memberNameStart, pos - memberNameStart);
133+
134+
// Allow only trailing whitespace.
135+
while (pos < span.Length && char.IsWhiteSpace(span[pos]))
136+
{
137+
pos++;
138+
}
139+
140+
if (pos != span.Length)
141+
{
142+
return false;
143+
}
144+
145+
// Only allocate strings after full validation.
146+
typeName = typeNameMemory;
147+
memberName = memberNameSpan.ToString();
148+
return true;
149+
}
150+
151+
/// <summary>
152+
/// Advances <paramref name="pos" /> past a <c>[TypeName]</c> token and outputs the type-name span.
153+
/// </summary>
154+
/// <param name="span">The full input span.</param>
155+
/// <param name="pos">The current parse position; advanced past the closing <c>]</c> on success.</param>
156+
/// <param name="typeName">A slice of <paramref name="span" /> containing the type name, without the brackets.</param>
157+
/// <returns><see langword="true" /> if a non-empty bracketed type name was consumed.</returns>
158+
internal static bool TryParseBracketedTypeName(ReadOnlySpan<char> span, ref int pos, out ReadOnlySpan<char> typeName)
159+
{
160+
typeName = default;
161+
162+
// Required opening bracket.
163+
if (pos >= span.Length || span[pos] != '[')
164+
{
165+
return false;
166+
}
167+
168+
pos++;
169+
170+
// Type name: one or more chars that are not '[', ']', or ':'.
171+
int typeNameStart = pos;
172+
while (pos < span.Length && span[pos] != '[' && span[pos] != ']' && span[pos] != ':')
173+
{
174+
pos++;
175+
}
176+
177+
if (pos == typeNameStart)
178+
{
179+
return false;
180+
}
181+
182+
typeName = span.Slice(typeNameStart, pos - typeNameStart);
183+
184+
// Required closing bracket.
185+
if (pos >= span.Length || span[pos] != ']')
186+
{
187+
return false;
188+
}
189+
190+
pos++;
191+
return true;
192+
}
193+
}

0 commit comments

Comments
 (0)