diff --git a/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexAnalyzer.cs b/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexAnalyzer.cs index 9e5c4788aa4cce..cf231dd2f9941c 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexAnalyzer.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexAnalyzer.cs @@ -28,6 +28,9 @@ public sealed class UpgradeToGeneratedRegexAnalyzer : DiagnosticAnalyzer internal const string PatternArgumentName = "pattern"; internal const string OptionsArgumentName = "options"; + /// Names of static Regex methods the analyzer considers fixable. + internal static readonly HashSet FixableMethodNames = new() { "Count", "EnumerateMatches", "IsMatch", "Match", "Matches", "Split", "Replace" }; + /// public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.UseRegexSourceGeneration); @@ -51,7 +54,7 @@ public override void Initialize(AnalysisContext context) // Pre-compute a hash with all of the method symbols that we want to analyze for possibly emitting // a diagnostic. HashSet staticMethodsToDetect = GetMethodSymbolHash(regexTypeSymbol, - new HashSet { "Count", "EnumerateMatches", "IsMatch", "Match", "Matches", "Split", "Replace" }); + FixableMethodNames); // Register analysis of calls to the Regex constructors context.RegisterOperationAction(context => AnalyzeObjectCreation(context, regexTypeSymbol), OperationKind.ObjectCreation); @@ -63,10 +66,6 @@ public override void Initialize(AnalysisContext context) // Creates a HashSet of all of the method Symbols containing the static methods to analyze. static HashSet GetMethodSymbolHash(INamedTypeSymbol regexTypeSymbol, HashSet methodNames) { - // This warning is due to a false positive bug https://github.com/dotnet/roslyn-analyzers/issues/5804 - // This issue has now been fixed, but we are not yet consuming the fix and getting this package - // as a transitive dependency from Microsoft.CodeAnalysis.CSharp.Workspaces. Once that dependency - // is updated at the repo-level, we should come and remove the pragma disable. HashSet hash = new HashSet(SymbolEqualityComparer.Default); ImmutableArray allMembers = regexTypeSymbol.GetMembers(); @@ -153,7 +152,7 @@ private static void AnalyzeObjectCreation(OperationAnalysisContext context, INam /// Validates the operation arguments ensuring the pattern and options are constant values. /// Returns false if a timeout argument is used. /// - private static bool ValidateParameters(ImmutableArray arguments) + internal static bool ValidateParameters(ImmutableArray arguments) { const string timeoutArgumentName = "timeout"; const string matchTimeoutArgumentName = "matchTimeout"; diff --git a/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexCodeFixer.cs b/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexCodeFixer.cs index 4f21419199016b..f27c55a12de764 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexCodeFixer.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexCodeFixer.cs @@ -121,6 +121,21 @@ operation is not (IInvocationOperation or IObjectCreationOperation)) { memberName = $"{DefaultRegexPropertyName}{memberCount++}"; } + + // When the BatchFixer applies multiple fixes concurrently, each fix sees the + // original compilation and picks the same first-available name. To avoid + // duplicates, determine this node's position among all Regex call sites in + // the type that would generate new names, and offset accordingly. + int precedingCount = CountPrecedingRegexCallSites( + typeSymbol, compilation, regexSymbol, nodeToFix, cancellationToken); + for (int i = 0; i < precedingCount; i++) + { + memberName = $"{DefaultRegexPropertyName}{memberCount++}"; + while (GetAllMembers(typeSymbol).Any(m => m.Name == memberName)) + { + memberName = $"{DefaultRegexPropertyName}{memberCount++}"; + } + } } // Add partial to all ancestors. @@ -324,6 +339,10 @@ private static Document TryAddNewMember( // Add the member to the type. if (oldMember is null) { + // Prepend a blank line so the generated member is visually separated from preceding members. + newMember = newMember.WithLeadingTrivia( + newMember.GetLeadingTrivia().Insert(0, SyntaxFactory.ElasticCarriageReturnLineFeed)); + newTypeDeclarationOrCompilationUnit = newTypeDeclarationOrCompilationUnit is TypeDeclarationSyntax newTypeDeclaration ? newTypeDeclaration.AddMembers((MemberDeclarationSyntax)newMember) : ((CompilationUnitSyntax)newTypeDeclarationOrCompilationUnit).AddMembers((ClassDeclarationSyntax)generator.ClassDeclaration("Program", modifiers: DeclarationModifiers.Partial, members: new[] { newMember })); @@ -457,5 +476,104 @@ private static IEnumerable GetAllMembers(INamedTypeSymbol typeSymbol) } } } + + /// + /// Counts how many Regex call sites in the same type (across all partial declarations) + /// appear before the given node in a deterministic order. This ensures that when the + /// BatchFixer applies fixes concurrently against the original compilation, each fix + /// picks a unique generated property name. + /// + private static int CountPrecedingRegexCallSites( + INamedTypeSymbol typeSymbol, Compilation compilation, + INamedTypeSymbol regexSymbol, SyntaxNode nodeToFix, + CancellationToken cancellationToken) + { + // Build a map from SyntaxTree to its index in the compilation, used as a + // tiebreaker when FilePath is null/empty (e.g., in-memory documents). + var treeIndexMap = new Dictionary(); + int treeCounter = 0; + foreach (SyntaxTree tree in compilation.SyntaxTrees) + { + treeIndexMap[tree] = treeCounter++; + } + + var callSites = new List<(string FilePath, int TreeIndex, int Position)>(); + var semanticModelCache = new Dictionary(); + + foreach (SyntaxReference syntaxRef in typeSymbol.DeclaringSyntaxReferences) + { + SyntaxNode declSyntax = syntaxRef.GetSyntax(cancellationToken); + + if (!semanticModelCache.TryGetValue(syntaxRef.SyntaxTree, out SemanticModel? declModel)) + { + declModel = compilation.GetSemanticModel(syntaxRef.SyntaxTree); + semanticModelCache[syntaxRef.SyntaxTree] = declModel; + } + + int treeIndex = treeIndexMap.TryGetValue(syntaxRef.SyntaxTree, out int idx) ? idx : -1; + + foreach (SyntaxNode descendant in declSyntax.DescendantNodes()) + { + if (descendant is not (InvocationExpressionSyntax or ObjectCreationExpressionSyntax or ImplicitObjectCreationExpressionSyntax)) + { + continue; + } + + // Skip call sites inside nested type declarations — they belong to + // a different type and won't affect this type's generated names. + // Extension blocks are not nested types, so don't skip those. + // Only check ancestors up to (not including) declSyntax, so that + // types *containing* declSyntax (e.g., an outer class) are not + // mistaken for nested types. + if (descendant.Ancestors().TakeWhile(a => a != declSyntax).Any(a => + a is TypeDeclarationSyntax && a is not ExtensionBlockDeclarationSyntax)) + { + continue; + } + + IOperation? op = declModel.GetOperation(descendant, cancellationToken); + bool isFixableRegexCall = op switch + { + IInvocationOperation inv => inv.TargetMethod.IsStatic && + SymbolEqualityComparer.Default.Equals(inv.TargetMethod.ContainingType, regexSymbol) && + UpgradeToGeneratedRegexAnalyzer.FixableMethodNames.Contains(inv.TargetMethod.Name) && + UpgradeToGeneratedRegexAnalyzer.ValidateParameters(inv.Arguments), + IObjectCreationOperation create => SymbolEqualityComparer.Default.Equals(create.Type, regexSymbol) && + create.Arguments.Length <= 2 && + UpgradeToGeneratedRegexAnalyzer.ValidateParameters(create.Arguments), + _ => false + }; + + if (isFixableRegexCall) + { + callSites.Add((syntaxRef.SyntaxTree.FilePath ?? string.Empty, treeIndex, descendant.SpanStart)); + } + } + } + + if (callSites.Count <= 1) + { + return 0; + } + + callSites.Sort((a, b) => + { + int cmp = StringComparer.Ordinal.Compare(a.FilePath, b.FilePath); + if (cmp != 0) return cmp; + cmp = a.TreeIndex.CompareTo(b.TreeIndex); + return cmp != 0 ? cmp : a.Position.CompareTo(b.Position); + }); + + string currentFilePath = nodeToFix.SyntaxTree.FilePath ?? string.Empty; + int currentTreeIndex = treeIndexMap.TryGetValue(nodeToFix.SyntaxTree, out int currentIdx) ? currentIdx : -1; + int currentPosition = nodeToFix.SpanStart; + + int index = callSites.FindIndex(c => + StringComparer.Ordinal.Equals(c.FilePath, currentFilePath) && + c.TreeIndex == currentTreeIndex && + c.Position == currentPosition); + + return index > 0 ? index : 0; + } } } diff --git a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/UpgradeToGeneratedRegexAnalyzerTests.cs b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/UpgradeToGeneratedRegexAnalyzerTests.cs index 24b7308fe93fd4..f6dd73f7d8918a 100644 --- a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/UpgradeToGeneratedRegexAnalyzerTests.cs +++ b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/UpgradeToGeneratedRegexAnalyzerTests.cs @@ -1073,6 +1073,7 @@ public static void Main() [GeneratedRegex(""a|b"")] private static partial Regex MyRegex { get; } + [GeneratedRegex(""c|d"", RegexOptions.CultureInvariant)] private static partial Regex MyRegex1 { get; } } @@ -1085,6 +1086,700 @@ public static void Main() }.RunAsync(); } + [Fact] + public async Task CodeFixerGeneratesUniqueNamesAcrossPartialClassDeclarations() + { + string test = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + var r = [|new Regex|](""abc""); + return r; + } +} + +internal partial class C +{ + public static Regex B() + { + var r = [|new Regex|](""def""); + return r; + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + var r = MyRegex; + return r; + } + + [GeneratedRegex(""abc"")] + private static partial Regex MyRegex { get; } +} + +internal partial class C +{ + public static Regex B() + { + var r = MyRegex1; + return r; + } + + [GeneratedRegex(""def"")] + private static partial Regex MyRegex1 { get; } +} +"; + + await VerifyCS.VerifyCodeFixAsync(test, fixedSource); + } + + [Fact] + public async Task CodeFixerGeneratesUniqueNamesAcrossPartialClassDeclarationsInSeparateFiles() + { + await new VerifyCS.Test + { + TestState = + { + Sources = + { + @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return [|new Regex|](""abc""); + } +} +", + @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex B() + { + return [|new Regex|](""def""); + } +} +" + } + }, + FixedState = + { + Sources = + { + @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return MyRegex; + } + + [GeneratedRegex(""abc"")] + private static partial Regex MyRegex { get; } +} +", + @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex B() + { + return MyRegex1; + } + + [GeneratedRegex(""def"")] + private static partial Regex MyRegex1 { get; } +} +" + } + }, + }.RunAsync(); + } + + [Fact] + public async Task CodeFixerGeneratesUniqueNamesAcrossThreePartialClassDeclarations() + { + string test = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return [|new Regex|](""aaa""); + } +} + +internal partial class C +{ + public static Regex B() + { + return [|new Regex|](""bbb""); + } +} + +internal partial class C +{ + public static Regex D() + { + return [|new Regex|](""ccc""); + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return MyRegex; + } + + [GeneratedRegex(""aaa"")] + private static partial Regex MyRegex { get; } +} + +internal partial class C +{ + public static Regex B() + { + return MyRegex1; + } + + [GeneratedRegex(""bbb"")] + private static partial Regex MyRegex1 { get; } +} + +internal partial class C +{ + public static Regex D() + { + return MyRegex2; + } + + [GeneratedRegex(""ccc"")] + private static partial Regex MyRegex2 { get; } +} +"; + + await VerifyCS.VerifyCodeFixAsync(test, fixedSource); + } + + [Fact] + public async Task CodeFixerGeneratesUniqueNamesWhenExistingMemberNamedMyRegex() + { + string test = @"using System.Text.RegularExpressions; + +internal partial class C +{ + private static void MyRegex() { } + + public static Regex A() + { + return [|new Regex|](""abc""); + } +} + +internal partial class C +{ + public static Regex B() + { + return [|new Regex|](""def""); + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial class C +{ + private static void MyRegex() { } + + public static Regex A() + { + return MyRegex1; + } + + [GeneratedRegex(""abc"")] + private static partial Regex MyRegex1 { get; } +} + +internal partial class C +{ + public static Regex B() + { + return MyRegex2; + } + + [GeneratedRegex(""def"")] + private static partial Regex MyRegex2 { get; } +} +"; + + await VerifyCS.VerifyCodeFixAsync(test, fixedSource); + } + + [Fact] + public async Task CodeFixerIgnoresNonFixableRegexCallsAcrossPartialDeclarations() + { + string test = @"using System; +using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return [|new Regex|](""abc""); + } + + public static Regex NonFixable() + { + return new Regex(""def"", RegexOptions.None, TimeSpan.FromSeconds(1)); + } +} + +internal partial class C +{ + public static Regex B() + { + return [|new Regex|](""ghi""); + } +} +"; + + string fixedSource = @"using System; +using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return MyRegex; + } + + public static Regex NonFixable() + { + return new Regex(""def"", RegexOptions.None, TimeSpan.FromSeconds(1)); + } + + [GeneratedRegex(""abc"")] + private static partial Regex MyRegex { get; } +} + +internal partial class C +{ + public static Regex B() + { + return MyRegex1; + } + + [GeneratedRegex(""ghi"")] + private static partial Regex MyRegex1 { get; } +} +"; + + await VerifyCS.VerifyCodeFixAsync(test, fixedSource); + } + + [Fact] + public async Task CodeFixerIgnoresNonFixableStaticMethodsAcrossPartialDeclarations() + { + string test = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static string A() + { + return Regex.Escape(""abc""); + } + + public static Regex B() + { + return [|new Regex|](""def""); + } +} + +internal partial class C +{ + public static Regex D() + { + return [|new Regex|](""ghi""); + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static string A() + { + return Regex.Escape(""abc""); + } + + public static Regex B() + { + return MyRegex; + } + + [GeneratedRegex(""def"")] + private static partial Regex MyRegex { get; } +} + +internal partial class C +{ + public static Regex D() + { + return MyRegex1; + } + + [GeneratedRegex(""ghi"")] + private static partial Regex MyRegex1 { get; } +} +"; + + await VerifyCS.VerifyCodeFixAsync(test, fixedSource); + } + + [Fact] + public async Task CodeFixerNestedTypeCallsDoNotAffectOuterTypeNaming() + { + string test = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return [|new Regex|](""abc""); + } + + class Inner + { + public static Regex X() + { + return [|new Regex|](""inner""); + } + } +} + +internal partial class C +{ + public static Regex B() + { + return [|new Regex|](""def""); + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return MyRegex; + } + + partial class Inner + { + public static Regex X() + { + return MyRegex; + } + + [GeneratedRegex(""inner"")] + private static partial Regex MyRegex { get; } + } + + [GeneratedRegex(""abc"")] + private static partial Regex MyRegex { get; } +} + +internal partial class C +{ + public static Regex B() + { + return MyRegex1; + } + + [GeneratedRegex(""def"")] + private static partial Regex MyRegex1 { get; } +} +"; + + await new VerifyCS.Test + { + TestCode = test, + FixedCode = fixedSource, + NumberOfFixAllIterations = 2, + }.RunAsync(); + } + + [Fact] + public async Task CodeFixerNonConstantPatternDoesNotAffectNaming() + { + string test = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A(string pattern) + { + var r = new Regex(pattern); + return r; + } + + public static Regex B() + { + return [|new Regex|](""abc""); + } +} + +internal partial class C +{ + public static Regex D() + { + return [|new Regex|](""def""); + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A(string pattern) + { + var r = new Regex(pattern); + return r; + } + + public static Regex B() + { + return MyRegex; + } + + [GeneratedRegex(""abc"")] + private static partial Regex MyRegex { get; } +} + +internal partial class C +{ + public static Regex D() + { + return MyRegex1; + } + + [GeneratedRegex(""def"")] + private static partial Regex MyRegex1 { get; } +} +"; + + await VerifyCS.VerifyCodeFixAsync(test, fixedSource); + } + + [Fact] + public async Task CodeFixerPartialStructGeneratesUniqueNames() + { + string test = @"using System.Text.RegularExpressions; + +internal partial struct S +{ + public static Regex A() + { + return [|new Regex|](""abc""); + } +} + +internal partial struct S +{ + public static Regex B() + { + return [|new Regex|](""def""); + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial struct S +{ + public static Regex A() + { + return MyRegex; + } + + [GeneratedRegex(""abc"")] + private static partial Regex MyRegex { get; } +} + +internal partial struct S +{ + public static Regex B() + { + return MyRegex1; + } + + [GeneratedRegex(""def"")] + private static partial Regex MyRegex1 { get; } +} +"; + + await VerifyCS.VerifyCodeFixAsync(test, fixedSource); + } + + [Fact] + public async Task CodeFixerMixedConstructorAndStaticMethodCallsAcrossPartials() + { + string test = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return [|new Regex|](""abc""); + } +} + +internal partial class C +{ + public static bool B(string input) + { + return [|Regex.IsMatch|](input, ""def""); + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial class C +{ + public static Regex A() + { + return MyRegex; + } + + [GeneratedRegex(""abc"")] + private static partial Regex MyRegex { get; } +} + +internal partial class C +{ + public static bool B(string input) + { + return MyRegex1.IsMatch(input); + } + + [GeneratedRegex(""def"")] + private static partial Regex MyRegex1 { get; } +} +"; + + await VerifyCS.VerifyCodeFixAsync(test, fixedSource); + } + + [Fact] + public async Task CodeFixerNestedPartialTypeDoesNotInterfereWithOuterPartial() + { + string test = @"using System.Text.RegularExpressions; + +internal partial class Outer +{ + public static Regex A() + { + return [|new Regex|](""outer1""); + } + + internal partial class Inner + { + public static Regex X() + { + return [|new Regex|](""inner1""); + } + } +} + +internal partial class Outer +{ + public static Regex B() + { + return [|new Regex|](""outer2""); + } + + internal partial class Inner + { + public static Regex Y() + { + return [|new Regex|](""inner2""); + } + } +} +"; + + string fixedSource = @"using System.Text.RegularExpressions; + +internal partial class Outer +{ + public static Regex A() + { + return MyRegex; + } + + internal partial class Inner + { + public static Regex X() + { + return MyRegex; + } + + [GeneratedRegex(""inner1"")] + private static partial Regex MyRegex { get; } + } + + [GeneratedRegex(""outer1"")] + private static partial Regex MyRegex { get; } +} + +internal partial class Outer +{ + public static Regex B() + { + return MyRegex1; + } + + internal partial class Inner + { + public static Regex Y() + { + return MyRegex1; + } + + [GeneratedRegex(""inner2"")] + private static partial Regex MyRegex1 { get; } + } + + [GeneratedRegex(""outer2"")] + private static partial Regex MyRegex1 { get; } +} +"; + + await new VerifyCS.Test + { + TestCode = test, + FixedCode = fixedSource, + NumberOfFixAllIterations = 2, + }.RunAsync(); + } + [Fact] public async Task CodeFixerSupportsNamedParameters() {