From b275911f16fa6bf8c64b09c8ee6c9ad3117984ef Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Wed, 25 Feb 2026 09:34:14 -0700 Subject: [PATCH 1/3] Add an mcp.json file --- .vscode/mcp.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .vscode/mcp.json diff --git a/.vscode/mcp.json b/.vscode/mcp.json new file mode 100644 index 000000000..cbe47099a --- /dev/null +++ b/.vscode/mcp.json @@ -0,0 +1,8 @@ +{ + "servers": { + "github": { + "url": "https://api.githubcopilot.com/mcp/" + } + }, + "inputs": [] +} From ed9bfee84341ef76535b0d717460f26522a5346a Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Wed, 25 Feb 2026 10:16:00 -0700 Subject: [PATCH 2/3] Document `InternalsVisibleTo` ban so Copilot knows --- .github/copilot-instructions.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index deb29535e..1aefbe271 100644 --- a/.github/copilot-instructions.md +++ b/.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 From c4644e82ab60eb2787fbd8bd1eee181544c0e057 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Wed, 25 Feb 2026 09:13:47 -0700 Subject: [PATCH 3/3] Remove slow regex from threading analyzers --- .../CommonInterest.cs | 77 +++--- .../CommonInterestParsing.cs | 193 ++++++++++++++ .../CommonInterestParsingTests.cs | 244 ++++++++++++++++++ ...ualStudio.Threading.Analyzers.Tests.csproj | 4 + .../Usings.cs | 1 + 5 files changed, 479 insertions(+), 40 deletions(-) create mode 100644 src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterestParsing.cs create mode 100644 test/Microsoft.VisualStudio.Threading.Analyzers.Tests/CommonInterestParsingTests.cs diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs index d0efdc342..b4fdfc07a 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs +++ b/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(@"^(?!)?\[(?[^\[\]\:]+)+\](?:\:\:(?\S+))?\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout); - - private static readonly Regex MemberReferenceRegex = new Regex(@"^\[(?[^\[\]\:]+)+\]::(?\S+)\s*$", RegexOptions.Singleline | RegexOptions.CultureInvariant, RegexMatchTimeout); - - /// - /// An array with '.' as its only element. - /// - private static readonly char[] QualifiedIdentifierSeparators = ['.']; - public static IEnumerable ReadMethods(AnalyzerOptions analyzerOptions, Regex fileNamePattern, CancellationToken cancellationToken) { foreach (string line in ReadAdditionalFiles(analyzerOptions, fileNamePattern, cancellationToken)) @@ -92,28 +81,15 @@ public static IEnumerable 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 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 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 ReadLinesFromAdditionalFile(SourceText text) public static QualifiedMember ParseAdditionalFileMethodLine(string line) { - Match? match = null; - try + if (!CommonInterestParsing.TryParseMemberReference(line, out ReadOnlyMemory typeNameMemory, out string? memberName)) { - match = MemberReferenceRegex.Match(line); + throw new InvalidOperationException($"Parsing error on line: {line}"); } - catch (RegexMatchTimeoutException) + + (ImmutableArray containingNamespace, string? typeName) = SplitQualifiedIdentifier(typeNameMemory); + var containingType = new QualifiedType(containingNamespace, typeName); + return new QualifiedMember(containingType, memberName!); + } + + /// + /// Splits a qualified type name (e.g. My.Namespace.MyType) into its containing namespace + /// segments and the simple type name, without allocating an intermediate joined string. + /// + /// The qualified type name as a memory slice. + /// The namespace segments and the simple type name. + private static (ImmutableArray ContainingNamespace, string TypeName) SplitQualifiedIdentifier(ReadOnlyMemory qualifiedName) + { + int lastDot = qualifiedName.Span.LastIndexOf('.'); + if (lastDot < 0) { - throw new InvalidOperationException($"Regex.Match timeout when parsing line: {line}"); + return (ImmutableArray.Empty, qualifiedName.ToString()); } - if (!match.Success) + string typeName = qualifiedName.Slice(lastDot + 1).ToString(); + ReadOnlyMemory nsPart = qualifiedName.Slice(0, lastDot); + ImmutableArray.Builder nsBuilder = ImmutableArray.CreateBuilder(); + 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) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterestParsing.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterestParsing.cs new file mode 100644 index 000000000..7088ab858 --- /dev/null +++ b/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; + +/// +/// Parsing helpers for additional-file lines used by . +/// This class is intentionally kept free of Roslyn dependencies so it can be +/// linked directly into test projects for unit testing. +/// +internal static class CommonInterestParsing +{ + /// + /// Parses a line that may begin with an optional !, followed by [TypeName], + /// and optionally ::MemberName, without using regular expressions. + /// + /// The line to parse. + /// if the line begins with '!'. + /// The type name parsed from the brackets. + /// The member name after '::', or if not present. + /// if parsing succeeded. + internal static bool TryParseNegatableTypeOrMemberReference(string line, out bool negated, out ReadOnlyMemory typeName, out string? memberName) + { + negated = false; + typeName = default; + memberName = null; + + ReadOnlySpan 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 typeNameMemory = line.AsMemory(bracketStart + 1, pos - bracketStart - 2); + + // Optional '::memberName'. + ReadOnlySpan 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; + } + + /// + /// Parses a line of the form [TypeName]::MemberName, without using regular expressions. + /// + /// The line to parse. + /// The type name parsed from the brackets. + /// The member name after '::'. + /// if parsing succeeded. + internal static bool TryParseMemberReference(string line, out ReadOnlyMemory typeName, out string? memberName) + { + typeName = default; + memberName = null; + + ReadOnlySpan 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 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 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; + } + + /// + /// Advances past a [TypeName] token and outputs the type-name span. + /// + /// The full input span. + /// The current parse position; advanced past the closing ] on success. + /// A slice of containing the type name, without the brackets. + /// if a non-empty bracketed type name was consumed. + internal static bool TryParseBracketedTypeName(ReadOnlySpan span, ref int pos, out ReadOnlySpan 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; + } +} diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/CommonInterestParsingTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/CommonInterestParsingTests.cs new file mode 100644 index 000000000..879c1fe53 --- /dev/null +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/CommonInterestParsingTests.cs @@ -0,0 +1,244 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.VisualStudio.Threading.Analyzers; + +public class CommonInterestParsingTests +{ + public class TryParseNegatableTypeOrMemberReferenceTests + { + [Fact] + public void TypeOnly() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType]", out bool negated, out ReadOnlyMemory typeName, out string? memberName)); + Assert.False(negated); + Assert.Equal("MyType", typeName.ToString()); + Assert.Null(memberName); + } + + [Fact] + public void TypeWithNamespace() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[My.Namespace.MyType]", out bool negated, out ReadOnlyMemory typeName, out string? memberName)); + Assert.False(negated); + Assert.Equal("My.Namespace.MyType", typeName.ToString()); + Assert.Null(memberName); + } + + [Fact] + public void TypeAndMember() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType]::MyMethod", out bool negated, out ReadOnlyMemory typeName, out string? memberName)); + Assert.False(negated); + Assert.Equal("MyType", typeName.ToString()); + Assert.Equal("MyMethod", memberName); + } + + [Fact] + public void TypeWithNamespaceAndMember() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[My.Namespace.MyType]::MyMethod", out bool negated, out ReadOnlyMemory typeName, out string? memberName)); + Assert.False(negated); + Assert.Equal("My.Namespace.MyType", typeName.ToString()); + Assert.Equal("MyMethod", memberName); + } + + [Fact] + public void NegatedTypeOnly() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("![MyType]", out bool negated, out ReadOnlyMemory typeName, out string? memberName)); + Assert.True(negated); + Assert.Equal("MyType", typeName.ToString()); + Assert.Null(memberName); + } + + [Fact] + public void NegatedTypeAndMember() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("![MyType]::MyMethod", out bool negated, out ReadOnlyMemory typeName, out string? memberName)); + Assert.True(negated); + Assert.Equal("MyType", typeName.ToString()); + Assert.Equal("MyMethod", memberName); + } + + [Fact] + public void TrailingWhitespace() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType] ", out _, out ReadOnlyMemory typeName, out _)); + Assert.Equal("MyType", typeName.ToString()); + } + + [Fact] + public void TrailingWhitespaceAfterMember() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType]::MyMethod ", out _, out ReadOnlyMemory typeName, out string? memberName)); + Assert.Equal("MyType", typeName.ToString()); + Assert.Equal("MyMethod", memberName); + } + + /// + /// Regression test: the long type name from the bug report must parse quickly without catastrophic backtracking. + /// + [Fact] + public void LongQualifiedTypeNameFromBugReport() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("![Microsoft.VisualStudio.Shell.Interop.IVsRunningDocumentTablePrivate]", out bool negated, out ReadOnlyMemory typeName, out string? memberName)); + Assert.True(negated); + Assert.Equal("Microsoft.VisualStudio.Shell.Interop.IVsRunningDocumentTablePrivate", typeName.ToString()); + Assert.Null(memberName); + } + + [Fact] + public void EmptyString() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference(string.Empty, out _, out _, out _)); + } + + [Fact] + public void NoBrackets() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("MyType", out _, out _, out _)); + } + + [Fact] + public void MissingOpeningBracket() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("MyType]", out _, out _, out _)); + } + + [Fact] + public void MissingClosingBracket() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType", out _, out _, out _)); + } + + [Fact] + public void EmptyTypeName() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[]", out _, out _, out _)); + } + + [Fact] + public void SingleColonNotDouble() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType]:MyMethod", out _, out _, out _)); + } + + [Fact] + public void DoubleColonWithoutMemberName() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType]::", out _, out _, out _)); + } + + [Fact] + public void ColonInsideBrackets() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType::NotAllowed]", out _, out _, out _)); + } + + [Fact] + public void LeadingWhitespace() + { + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference(" [MyType]", out _, out _, out _)); + } + + [Fact] + public void SpaceInMiddleOfMemberName() + { + // The member name scanner stops at whitespace, so "extra" content after a space is not consumed + // and the trailing-only-whitespace check rejects the line. + Assert.False(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[MyType]::MyMethod extra", out _, out _, out _)); + } + + [Fact] + public void WildcardTypeName() + { + Assert.True(CommonInterestParsing.TryParseNegatableTypeOrMemberReference("[*]", out bool negated, out ReadOnlyMemory typeName, out string? memberName)); + Assert.False(negated); + Assert.Equal("*", typeName.ToString()); + Assert.Null(memberName); + } + } + + public class TryParseMemberReferenceTests + { + [Fact] + public void TypeAndMember() + { + Assert.True(CommonInterestParsing.TryParseMemberReference("[MyType]::MyMethod", out ReadOnlyMemory typeName, out string? memberName)); + Assert.Equal("MyType", typeName.ToString()); + Assert.Equal("MyMethod", memberName); + } + + [Fact] + public void TypeWithNamespaceAndMember() + { + Assert.True(CommonInterestParsing.TryParseMemberReference("[My.Namespace.MyType]::MyMethod", out ReadOnlyMemory typeName, out string? memberName)); + Assert.Equal("My.Namespace.MyType", typeName.ToString()); + Assert.Equal("MyMethod", memberName); + } + + [Fact] + public void TrailingWhitespace() + { + Assert.True(CommonInterestParsing.TryParseMemberReference("[MyType]::MyMethod ", out ReadOnlyMemory typeName, out string? memberName)); + Assert.Equal("MyType", typeName.ToString()); + Assert.Equal("MyMethod", memberName); + } + + [Fact] + public void EmptyString() + { + Assert.False(CommonInterestParsing.TryParseMemberReference(string.Empty, out _, out _)); + } + + [Fact] + public void TypeOnly_NoMember() + { + Assert.False(CommonInterestParsing.TryParseMemberReference("[MyType]", out _, out _)); + } + + [Fact] + public void NoBrackets() + { + Assert.False(CommonInterestParsing.TryParseMemberReference("MyType::MyMethod", out _, out _)); + } + + [Fact] + public void MissingClosingBracket() + { + Assert.False(CommonInterestParsing.TryParseMemberReference("[MyType::MyMethod", out _, out _)); + } + + [Fact] + public void EmptyTypeName() + { + Assert.False(CommonInterestParsing.TryParseMemberReference("[]::MyMethod", out _, out _)); + } + + [Fact] + public void SingleColonNotDouble() + { + Assert.False(CommonInterestParsing.TryParseMemberReference("[MyType]:MyMethod", out _, out _)); + } + + [Fact] + public void DoubleColonWithoutMemberName() + { + Assert.False(CommonInterestParsing.TryParseMemberReference("[MyType]::", out _, out _)); + } + + [Fact] + public void LeadingWhitespace() + { + Assert.False(CommonInterestParsing.TryParseMemberReference(" [MyType]::MyMethod", out _, out _)); + } + + [Fact] + public void Negation_NotSupported() + { + // TryParseMemberReference does not accept a leading '!'. + Assert.False(CommonInterestParsing.TryParseMemberReference("![MyType]::MyMethod", out _, out _)); + } + } +} diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Microsoft.VisualStudio.Threading.Analyzers.Tests.csproj b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Microsoft.VisualStudio.Threading.Analyzers.Tests.csproj index 50bbac324..78d772b56 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Microsoft.VisualStudio.Threading.Analyzers.Tests.csproj +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Microsoft.VisualStudio.Threading.Analyzers.Tests.csproj @@ -44,6 +44,10 @@ + + + + diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Usings.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Usings.cs index d38229f3d..4fb29506c 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Usings.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Usings.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +global using System; global using System.Threading.Tasks; global using Microsoft; global using Microsoft.CodeAnalysis;