Merge pull request #1547 from microsoft/dev/andarno/fixregextime

Remove slow regex from threading analyzers

Author: AArnott

.github/copilot-instructions.md

@@ -9,6 +9,7 @@
 
 * Design APIs to be highly testable, and all functionality should be tested.
 * Avoid introducing binary breaking changes in public APIs of projects under `src` unless their project files have `IsPackable` set to `false`.
+* `InternalsVisibleTo` attributes are *not allowed*.
 
 ## Testing
 
@@ -17,6 +18,8 @@
 * There should generally be one test project (under the `test` directory) per shipping project (under the `src` directory). Test projects are named after the project being tested with a `.Tests` suffix.
 * Tests use xunit v3 with Microsoft.Testing.Platform (MTP v2). Traditional VSTest `--filter` syntax does NOT work.
 * Some tests are known to be unstable. When running tests, you should skip the unstable ones by using `-- --filter-not-trait "FailsInCloudTest=true"`.
+* Since `InternalsVisibleTo` is not allowed, testing must be done at the public API level.
+  In rare cases where there are static utility methods that need to be thoroughly tested, which may be impossible or inefficient to do via public APIs, the static methods may be moved to a .cs file that is then linked both into the product and into the test project so that it may be tested directly.
 
 ### Running Tests
 

.vscode/mcp.json

@@ -0,0 +1,8 @@
+{
+  "servers": {
+    "github": {
+      "url": "https://api.githubcopilot.com/mcp/"
+    }
+  },
+  "inputs": []
+}

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

@@ -69,17 +69,6 @@ public static class CommonInterest
 
     private const string GetAwaiterMethodName = "GetAwaiter";
 
-    private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(5);  // Prevent expensive CPU hang in Regex.Match if backtracking occurs due to pathological input (see #485).
-
-    private static readonly Regex NegatableTypeOrMemberReferenceRegex = new Regex(@"^(?<negated>!)?\[(?<typeName>[^\[\]\:]+)+\](?:\:\:(?<memberName>\S+))?\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout);
-
-    private static readonly Regex MemberReferenceRegex = new Regex(@"^\[(?<typeName>[^\[\]\:]+)+\]::(?<memberName>\S+)\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout);
-
-    /// <summary>
-    /// An array with '.' as its only element.
-    /// </summary>
-    private static readonly char[] QualifiedIdentifierSeparators = ['.'];
-
     public static IEnumerable<QualifiedMember> ReadMethods(AnalyzerOptions analyzerOptions, Regex fileNamePattern, CancellationToken cancellationToken)
     {
         foreach (string line in ReadAdditionalFiles(analyzerOptions, fileNamePattern, cancellationToken))
@@ -92,28 +81,15 @@ public static IEnumerable<TypeMatchSpec> ReadTypesAndMembers(AnalyzerOptions ana
     {
         foreach (string line in ReadAdditionalFiles(analyzerOptions, fileNamePattern, cancellationToken))
         {
-            Match? match = null;
-            try
-            {
-                match = NegatableTypeOrMemberReferenceRegex.Match(line);
-            }
-            catch (RegexMatchTimeoutException)
-            {
-                throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}");
-            }
-
-            if (!match.Success)
+            if (!CommonInterestParsing.TryParseNegatableTypeOrMemberReference(line, out bool negated, out ReadOnlyMemory<char> typeNameMemory, out string? memberNameValue))
             {
                 throw new InvalidOperationException($"Parsing error on line: {line}");
             }
 
-            bool inverted = match.Groups["negated"].Success;
-            string[] typeNameElements = match.Groups["typeName"].Value.Split(QualifiedIdentifierSeparators);
-            string typeName = typeNameElements[typeNameElements.Length - 1];
-            var containingNamespace = typeNameElements.Take(typeNameElements.Length - 1).ToImmutableArray();
+            (ImmutableArray<string> containingNamespace, string? typeName) = SplitQualifiedIdentifier(typeNameMemory);
             var type = new QualifiedType(containingNamespace, typeName);
-            QualifiedMember member = match.Groups["memberName"].Success ? new QualifiedMember(type, match.Groups["memberName"].Value) : default(QualifiedMember);
-            yield return new TypeMatchSpec(type, member, inverted);
+            QualifiedMember member = memberNameValue is not null ? new QualifiedMember(type, memberNameValue) : default(QualifiedMember);
+            yield return new TypeMatchSpec(type, member, negated);
         }
     }
 
@@ -345,26 +321,47 @@ public static IEnumerable<string> ReadLinesFromAdditionalFile(SourceText text)
 
     public static QualifiedMember ParseAdditionalFileMethodLine(string line)
     {
-        Match? match = null;
-        try
+        if (!CommonInterestParsing.TryParseMemberReference(line, out ReadOnlyMemory<char> typeNameMemory, out string? memberName))
         {
-            match = MemberReferenceRegex.Match(line);
+            throw new InvalidOperationException($"Parsing error on line: {line}");
         }
-        catch (RegexMatchTimeoutException)
+
+        (ImmutableArray<string> containingNamespace, string? typeName) = SplitQualifiedIdentifier(typeNameMemory);
+        var containingType = new QualifiedType(containingNamespace, typeName);
+        return new QualifiedMember(containingType, memberName!);
+    }
+
+    /// <summary>
+    /// Splits a qualified type name (e.g. <c>My.Namespace.MyType</c>) into its containing namespace
+    /// segments and the simple type name, without allocating an intermediate joined string.
+    /// </summary>
+    /// <param name="qualifiedName">The qualified type name as a memory slice.</param>
+    /// <returns>The namespace segments and the simple type name.</returns>
+    private static (ImmutableArray<string> ContainingNamespace, string TypeName) SplitQualifiedIdentifier(ReadOnlyMemory<char> qualifiedName)
+    {
+        int lastDot = qualifiedName.Span.LastIndexOf('.');
+        if (lastDot < 0)
         {
-            throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}");
+            return (ImmutableArray<string>.Empty, qualifiedName.ToString());
         }
 
-        if (!match.Success)
+        string typeName = qualifiedName.Slice(lastDot + 1).ToString();
+        ReadOnlyMemory<char> nsPart = qualifiedName.Slice(0, lastDot);
+        ImmutableArray<string>.Builder nsBuilder = ImmutableArray.CreateBuilder<string>();
+        while (!nsPart.IsEmpty)
         {
-            throw new InvalidOperationException($"Parsing error on line: {line}");
+            int dot = nsPart.Span.IndexOf('.');
+            if (dot < 0)
+            {
+                nsBuilder.Add(nsPart.ToString());
+                break;
+            }
+
+            nsBuilder.Add(nsPart.Slice(0, dot).ToString());
+            nsPart = nsPart.Slice(dot + 1);
         }
 
-        string methodName = match.Groups["memberName"].Value;
-        string[] typeNameElements = match.Groups["typeName"].Value.Split(QualifiedIdentifierSeparators);
-        string typeName = typeNameElements[typeNameElements.Length - 1];
-        var containingType = new QualifiedType(typeNameElements.Take(typeNameElements.Length - 1).ToImmutableArray(), typeName);
-        return new QualifiedMember(containingType, methodName);
+        return (nsBuilder.ToImmutable(), typeName);
     }
 
     private static bool TestGetAwaiterMethod(IMethodSymbol getAwaiterMethod)

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

@@ -0,0 +1,193 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+
+namespace Microsoft.VisualStudio.Threading.Analyzers;
+
+/// <summary>
+/// Parsing helpers for additional-file lines used by <see cref="CommonInterest" />.
+/// This class is intentionally kept free of Roslyn dependencies so it can be
+/// linked directly into test projects for unit testing.
+/// </summary>
+internal static class CommonInterestParsing
+{
+    /// <summary>
+    /// Parses a line that may begin with an optional <c>!</c>, followed by <c>[TypeName]</c>,
+    /// and optionally <c>::MemberName</c>, without using regular expressions.
+    /// </summary>
+    /// <param name="line">The line to parse.</param>
+    /// <param name="negated"><see langword="true" /> if the line begins with '!'.</param>
+    /// <param name="typeName">The type name parsed from the brackets.</param>
+    /// <param name="memberName">The member name after '::', or <see langword="null" /> if not present.</param>
+    /// <returns><see langword="true" /> if parsing succeeded.</returns>
+    internal static bool TryParseNegatableTypeOrMemberReference(string line, out bool negated, out ReadOnlyMemory<char> typeName, out string? memberName)
+    {
+        negated = false;
+        typeName = default;
+        memberName = null;
+
+        ReadOnlySpan<char> span = line.AsSpan();
+        int pos = 0;
+
+        // Optional negation prefix.
+        if (pos < span.Length && span[pos] == '!')
+        {
+            negated = true;
+            pos++;
+        }
+
+        // Required '[TypeName]'.
+        int bracketStart = pos;
+        if (!TryParseBracketedTypeName(span, ref pos, out _))
+        {
+            return false;
+        }
+
+        // Compute memory slice for type name (between the brackets), using recorded bracket position.
+        ReadOnlyMemory<char> typeNameMemory = line.AsMemory(bracketStart + 1, pos - bracketStart - 2);
+
+        // Optional '::memberName'.
+        ReadOnlySpan<char> memberNameSpan = default;
+        if (pos + 1 < span.Length && span[pos] == ':' && span[pos + 1] == ':')
+        {
+            pos += 2;
+            int memberNameStart = pos;
+            while (pos < span.Length && !char.IsWhiteSpace(span[pos]))
+            {
+                pos++;
+            }
+
+            if (pos == memberNameStart)
+            {
+                // '::' present but no member name follows.
+                return false;
+            }
+
+            memberNameSpan = span.Slice(memberNameStart, pos - memberNameStart);
+        }
+
+        // Allow only trailing whitespace.
+        while (pos < span.Length && char.IsWhiteSpace(span[pos]))
+        {
+            pos++;
+        }
+
+        if (pos != span.Length)
+        {
+            return false;
+        }
+
+        // Only allocate strings after full validation.
+        typeName = typeNameMemory;
+        memberName = memberNameSpan.IsEmpty ? null : memberNameSpan.ToString();
+        return true;
+    }
+
+    /// <summary>
+    /// Parses a line of the form <c>[TypeName]::MemberName</c>, without using regular expressions.
+    /// </summary>
+    /// <param name="line">The line to parse.</param>
+    /// <param name="typeName">The type name parsed from the brackets.</param>
+    /// <param name="memberName">The member name after '::'.</param>
+    /// <returns><see langword="true" /> if parsing succeeded.</returns>
+    internal static bool TryParseMemberReference(string line, out ReadOnlyMemory<char> typeName, out string? memberName)
+    {
+        typeName = default;
+        memberName = null;
+
+        ReadOnlySpan<char> span = line.AsSpan();
+        int pos = 0;
+
+        // Required '[TypeName]'.
+        int bracketStart = pos;
+        if (!TryParseBracketedTypeName(span, ref pos, out _))
+        {
+            return false;
+        }
+
+        // Compute memory slice for type name (between the brackets), using recorded bracket position.
+        ReadOnlyMemory<char> typeNameMemory = line.AsMemory(bracketStart + 1, pos - bracketStart - 2);
+
+        // Required '::'.
+        if (pos + 1 >= span.Length || span[pos] != ':' || span[pos + 1] != ':')
+        {
+            return false;
+        }
+
+        pos += 2;
+
+        // Member name: one or more non-whitespace chars.
+        int memberNameStart = pos;
+        while (pos < span.Length && !char.IsWhiteSpace(span[pos]))
+        {
+            pos++;
+        }
+
+        if (pos == memberNameStart)
+        {
+            return false;
+        }
+
+        ReadOnlySpan<char> memberNameSpan = span.Slice(memberNameStart, pos - memberNameStart);
+
+        // Allow only trailing whitespace.
+        while (pos < span.Length && char.IsWhiteSpace(span[pos]))
+        {
+            pos++;
+        }
+
+        if (pos != span.Length)
+        {
+            return false;
+        }
+
+        // Only allocate strings after full validation.
+        typeName = typeNameMemory;
+        memberName = memberNameSpan.ToString();
+        return true;
+    }
+
+    /// <summary>
+    /// Advances <paramref name="pos" /> past a <c>[TypeName]</c> token and outputs the type-name span.
+    /// </summary>
+    /// <param name="span">The full input span.</param>
+    /// <param name="pos">The current parse position; advanced past the closing <c>]</c> on success.</param>
+    /// <param name="typeName">A slice of <paramref name="span" /> containing the type name, without the brackets.</param>
+    /// <returns><see langword="true" /> if a non-empty bracketed type name was consumed.</returns>
+    internal static bool TryParseBracketedTypeName(ReadOnlySpan<char> span, ref int pos, out ReadOnlySpan<char> typeName)
+    {
+        typeName = default;
+
+        // Required opening bracket.
+        if (pos >= span.Length || span[pos] != '[')
+        {
+            return false;
+        }
+
+        pos++;
+
+        // Type name: one or more chars that are not '[', ']', or ':'.
+        int typeNameStart = pos;
+        while (pos < span.Length && span[pos] != '[' && span[pos] != ']' && span[pos] != ':')
+        {
+            pos++;
+        }
+
+        if (pos == typeNameStart)
+        {
+            return false;
+        }
+
+        typeName = span.Slice(typeNameStart, pos - typeNameStart);
+
+        // Required closing bracket.
+        if (pos >= span.Length || span[pos] != ']')
+        {
+            return false;
+        }
+
+        pos++;
+        return true;
+    }
+}