"Convert to GeneratedRegexAttribute" mangles Constants (#116149)

Copilot · danmoseley · stephentoub · web-flow · commit e661de4d7a94 · 2025-06-19T22:03:44.000-04:00
* update instructions

* Fix RegexOptions constant handling in GeneratedRegexAttribute

---------

Co-authored-by: Dan Moseley &lt;danmose@microsoft.com&gt;
Co-authored-by: Stephen Toub &lt;stoub@microsoft.com&gt;
diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
@@ -16,6 +16,10 @@ In addition to the rules enforced by `.editorconfig`, you SHOULD:
 - Trust the C# null annotations and don't add null checks when the type system says a value cannot be null.
 - Prefer `?.` if applicable (e.g. `scope?.Dispose()`).
 - Use `ObjectDisposedException.ThrowIf` where applicable.
+- When adding new unit tests, strongly prefer to add them to existing test code files rather than creating new code files.
+- If you add new code files, ensure they are listed in the csproj file (if other files in that folder are listed there) so they build.
+- When running tests, if possible use filters and check test run counts, or look at test logs, to ensure they actually ran.
+- Do not finish work with any tests commented out or disabled that were not previously commented out or disabled.
 - When writing tests, do not emit "Act", "Arrange" or "Assert" comments.
 
 ---
diff --git a/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexCodeFixer.cs b/src/libraries/System.Text.RegularExpressions/gen/UpgradeToGeneratedRegexCodeFixer.cs
@@ -281,29 +281,35 @@ static RegexOptions GetRegexOptionsFromArgument(ImmutableArray<IArgumentOperatio
                     return null;
                 }
 
-                Debug.Assert(parameterName is UpgradeToGeneratedRegexAnalyzer.OptionsArgumentName or UpgradeToGeneratedRegexAnalyzer.PatternArgumentName);
-                if (parameterName == UpgradeToGeneratedRegexAnalyzer.OptionsArgumentName)
+                // Literals and class-level field references should be preserved as-is.
+                if (argument.Value is ILiteralOperation ||
+                    argument.Value is IFieldReferenceOperation { Member: IFieldSymbol { IsConst: true } })
                 {
-                    string optionsLiteral = Literal(((RegexOptions)(int)argument.Value.ConstantValue.Value!).ToString());
-                    return SyntaxFactory.ParseExpression(optionsLiteral);
+                    return argument.Value.Syntax;
                 }
-                else if (argument.Value is ILiteralOperation literalOperation)
-                {
-                    return literalOperation.Syntax;
-                }
-                else if (argument.Value is IFieldReferenceOperation fieldReferenceOperation &&
-                    fieldReferenceOperation.Member is IFieldSymbol fieldSymbol && fieldSymbol.IsConst)
-                {
-                    return generator.Argument(fieldReferenceOperation.Syntax);
-                }
-                else if (argument.Value.ConstantValue.Value is string str && str.Contains('\\'))
-                {
-                    string escapedVerbatimText = str.Replace("\"", "\"\"");
-                    return SyntaxFactory.ParseExpression($"@\"{escapedVerbatimText}\"");
-                }
-                else
+
+                switch (parameterName)
                 {
-                    return generator.LiteralExpression(argument.Value.ConstantValue.Value);
+                    case UpgradeToGeneratedRegexAnalyzer.OptionsArgumentName:
+                        string optionsLiteral = Literal(((RegexOptions)(int)argument.Value.ConstantValue.Value!).ToString());
+                        return SyntaxFactory.ParseExpression(optionsLiteral);
+
+                    case UpgradeToGeneratedRegexAnalyzer.PatternArgumentName:
+                        if (argument.Value.ConstantValue.Value is string str && str.Contains('\\'))
+                        {
+                            // Special handling for string patterns with escaped characters
+                            string escapedVerbatimText = str.Replace("\"", "\"\"");
+                            return SyntaxFactory.ParseExpression($"@\"{escapedVerbatimText}\"");
+                        }
+                        else
+                        {
+                            // Default handling for all other patterns.
+                            return generator.LiteralExpression(argument.Value.ConstantValue.Value);
+                        }
+
+                    default:
+                        Debug.Fail($"Unknown parameter: {parameterName}");
+                        return argument.Syntax;
                 }
             }
 
diff --git a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/UpgradeToGeneratedRegexAnalyzerTests.cs b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/UpgradeToGeneratedRegexAnalyzerTests.cs
@@ -256,16 +256,16 @@ public static void Main(string[] args)
     private static partial Regex MyRegex();
 }" };
 
-                // Test constructor with a local constant pattern.
+                // Test constructor with a local constant pattern (local constants are expanded).
                 yield return new object[] { @"using System.Text;
 using System.Text.RegularExpressions;
 
 public class Program
 {
     public static void Main(string[] args)
     {
-        const string pattern = @"""";
-        var isMatch = [|" + ConstructRegexInvocation(invocationType, "\"\"") + @"|]" + isMatchInvocation + @";
+        const string pattern = @""a|b"";
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "pattern") + @"|]" + isMatchInvocation + @";
     }
 }", @"using System.Text;
 using System.Text.RegularExpressions;
@@ -274,39 +274,73 @@ public partial class Program
 {
     public static void Main(string[] args)
     {
-        const string pattern = @"""";
+        const string pattern = @""a|b"";
         var isMatch = MyRegex().IsMatch("""");
     }
 
-    [GeneratedRegex("""")]
+    [GeneratedRegex(""a|b"")]
     private static partial Regex MyRegex();
 }" };
 
-                // Test constructor with a constant field pattern.
+                // Test constructor with a constant field pattern (field constants are preserved).
                 yield return new object[] { @"using System.Text;
 using System.Text.RegularExpressions;
 
 public class Program
 {
-    private const string pattern = @"""";
+    private const string Pattern = @""a|b"";
 
     public static void Main(string[] args)
     {
-        var isMatch = [|" + ConstructRegexInvocation(invocationType, "\"\"") + @"|]" + isMatchInvocation + @";
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "Pattern") + @"|]" + isMatchInvocation + @";
     }
 }", @"using System.Text;
 using System.Text.RegularExpressions;
 
 public partial class Program
 {
-    private const string pattern = @"""";
+    private const string Pattern = @""a|b"";
 
     public static void Main(string[] args)
     {
         var isMatch = MyRegex().IsMatch("""");
     }
 
-    [GeneratedRegex("""")]
+    [GeneratedRegex(Pattern)]
+    private static partial Regex MyRegex();
+}" };
+
+                // Test constructor with external constant field pattern (external field constants are preserved).
+                yield return new object[] { @"using System.Text;
+using System.Text.RegularExpressions;
+
+public class PatternConstants
+{
+    public const string EmailPattern = @""^[^@]+@[^@]+\.[^@]+$"";
+}
+
+public class Program
+{
+    public static void Main(string[] args)
+    {
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "PatternConstants.EmailPattern") + @"|]" + isMatchInvocation + @";
+    }
+}", @"using System.Text;
+using System.Text.RegularExpressions;
+
+public class PatternConstants
+{
+    public const string EmailPattern = @""^[^@]+@[^@]+\.[^@]+$"";
+}
+
+public partial class Program
+{
+    public static void Main(string[] args)
+    {
+        var isMatch = MyRegex().IsMatch("""");
+    }
+
+    [GeneratedRegex(PatternConstants.EmailPattern)]
     private static partial Regex MyRegex();
 }" };
             }
@@ -468,7 +502,39 @@ public static void Main(string[] args)
         var isMatch = MyRegex().IsMatch("""");
     }
 
-    [GeneratedRegex("""", RegexOptions.None)]
+    [GeneratedRegex("""", Options)]
+    private static partial Regex MyRegex();
+}" };
+
+                // Test options as external constant field
+                yield return new object[] { @"using System.Text.RegularExpressions;
+
+public class RegexConstants
+{
+    public const RegexOptions DefaultOptions = RegexOptions.IgnoreCase | RegexOptions.CultureInvariant;
+}
+
+public class Program
+{
+    public static void Main(string[] args)
+    {
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "\"\"", "RegexConstants.DefaultOptions") + @"|]" + isMatchInvocation + @";
+    }
+}", @"using System.Text.RegularExpressions;
+
+public class RegexConstants
+{
+    public const RegexOptions DefaultOptions = RegexOptions.IgnoreCase | RegexOptions.CultureInvariant;
+}
+
+public partial class Program
+{
+    public static void Main(string[] args)
+    {
+        var isMatch = MyRegex().IsMatch("""");
+    }
+
+    [GeneratedRegex("""", RegexConstants.DefaultOptions)]
     private static partial Regex MyRegex();
 }" };
             }
@@ -481,6 +547,186 @@ public async Task DiagnosticEmittedForConstantOptions(string test, string fixedS
             await VerifyCS.VerifyCodeFixAsync(test, fixedSource);
         }
 
+        public static IEnumerable<object[]> MixedConstantTestData()
+        {
+            foreach (InvocationType invocationType in new[] { InvocationType.Constructor, InvocationType.StaticMethods })
+            {
+                string isMatchInvocation = invocationType == InvocationType.Constructor ? @".IsMatch("""")" : string.Empty;
+                
+                // Test both pattern and options as field constants (both should be preserved)
+                yield return new object[] { @"using System.Text.RegularExpressions;
+
+public class Program
+{
+    const string MyPattern = @""[a-z]+"";
+    const RegexOptions MyOptions = RegexOptions.IgnoreCase | RegexOptions.CultureInvariant;
+
+    public static void Main(string[] args)
+    {
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "MyPattern", "MyOptions") + @"|]" + isMatchInvocation + @";
+    }
+}", @"using System.Text.RegularExpressions;
+
+public partial class Program
+{
+    const string MyPattern = @""[a-z]+"";
+    const RegexOptions MyOptions = RegexOptions.IgnoreCase | RegexOptions.CultureInvariant;
+
+    public static void Main(string[] args)
+    {
+        var isMatch = MyRegex().IsMatch("""");
+    }
+
+    [GeneratedRegex(MyPattern, MyOptions)]
+    private static partial Regex MyRegex();
+}" };
+
+                // Test pattern as field constant and options as local constant (field preserved, local expanded)
+                yield return new object[] { @"using System.Text.RegularExpressions;
+
+public class Program
+{
+    const string GlobalPattern = @""\d+"";
+
+    public static void Main(string[] args)
+    {
+        const RegexOptions localOptions = RegexOptions.Multiline;
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "GlobalPattern", "localOptions") + @"|]" + isMatchInvocation + @";
+    }
+}", @"using System.Text.RegularExpressions;
+
+public partial class Program
+{
+    const string GlobalPattern = @""\d+"";
+
+    public static void Main(string[] args)
+    {
+        const RegexOptions localOptions = RegexOptions.Multiline;
+        var isMatch = MyRegex().IsMatch("""");
+    }
+
+    [GeneratedRegex(GlobalPattern, RegexOptions.Multiline)]
+    private static partial Regex MyRegex();
+}" };
+
+                // Test pattern as local constant and options as field constant (local expanded, field preserved)
+                yield return new object[] { @"using System.Text.RegularExpressions;
+
+public class Program
+{
+    const RegexOptions DefaultOptions = RegexOptions.IgnoreCase | RegexOptions.CultureInvariant;
+
+    public static void Main(string[] args)
+    {
+        const string localPattern = @""test.*pattern"";
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "localPattern", "DefaultOptions") + @"|]" + isMatchInvocation + @";
+    }
+}", @"using System.Text.RegularExpressions;
+
+public partial class Program
+{
+    const RegexOptions DefaultOptions = RegexOptions.IgnoreCase | RegexOptions.CultureInvariant;
+
+    public static void Main(string[] args)
+    {
+        const string localPattern = @""test.*pattern"";
+        var isMatch = MyRegex().IsMatch("""");
+    }
+
+    [GeneratedRegex(""test.*pattern"", DefaultOptions)]
+    private static partial Regex MyRegex();
+}" };
+
+                // Test external constants for both pattern and options
+                yield return new object[] { @"using System.Text.RegularExpressions;
+
+public static class RegexConfig
+{
+    public const string EmailPattern = @""^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$"";
+    public const RegexOptions EmailOptions = RegexOptions.IgnoreCase | RegexOptions.CultureInvariant;
+}
+
+public class Program
+{
+    public static void Main(string[] args)
+    {
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "RegexConfig.EmailPattern", "RegexConfig.EmailOptions") + @"|]" + isMatchInvocation + @";
+    }
+}", @"using System.Text.RegularExpressions;
+
+public static class RegexConfig
+{
+    public const string EmailPattern = @""^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$"";
+    public const RegexOptions EmailOptions = RegexOptions.IgnoreCase | RegexOptions.CultureInvariant;
+}
+
+public partial class Program
+{
+    public static void Main(string[] args)
+    {
+        var isMatch = MyRegex().IsMatch("""");
+    }
+
+    [GeneratedRegex(RegexConfig.EmailPattern, RegexConfig.EmailOptions)]
+    private static partial Regex MyRegex();
+}" };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(MixedConstantTestData))]
+        public async Task DiagnosticEmittedForMixedConstants(string test, string fixedSource)
+        {
+            await VerifyCS.VerifyCodeFixAsync(test, fixedSource);
+        }
+
+        public static IEnumerable<object[]> StaticFieldConstantTestData()
+        {
+            foreach (InvocationType invocationType in new[] { InvocationType.Constructor, InvocationType.StaticMethods })
+            {
+                string isMatchInvocation = invocationType == InvocationType.Constructor ? @".IsMatch("""")" : string.Empty;
+                
+                // Test static field constants (should be preserved)
+                yield return new object[] { @"using System.Text.RegularExpressions;
+
+public class Program
+{
+    public static readonly string ReadOnlyPattern = @""readonly""; // This is not const, so won't be preserved
+    public const string ConstPattern = @""const"";
+    private static readonly RegexOptions ReadOnlyOptions = RegexOptions.IgnoreCase; // This is not const, so won't be preserved
+    private const RegexOptions ConstOptions = RegexOptions.Multiline;
+
+    public static void Main(string[] args)
+    {
+        var isMatch = [|" + ConstructRegexInvocation(invocationType, "ConstPattern", "ConstOptions") + @"|]" + isMatchInvocation + @";
+    }
+}", @"using System.Text.RegularExpressions;
+
+public partial class Program
+{
+    public static readonly string ReadOnlyPattern = @""readonly""; // This is not const, so won't be preserved
+    public const string ConstPattern = @""const"";
+    private static readonly RegexOptions ReadOnlyOptions = RegexOptions.IgnoreCase; // This is not const, so won't be preserved
+    private const RegexOptions ConstOptions = RegexOptions.Multiline;
+
+    public static void Main(string[] args)
+    {
+        var isMatch = MyRegex().IsMatch("""");
+    }
+
+    [GeneratedRegex(ConstPattern, ConstOptions)]
+    private static partial Regex MyRegex();
+}" };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(StaticFieldConstantTestData))]
+        public async Task DiagnosticEmittedForStaticFieldConstants(string test, string fixedSource)
+        {
+            await VerifyCS.VerifyCodeFixAsync(test, fixedSource);
+        }
+
         public static IEnumerable<object[]> VariableOptionsTestData()
         {
             foreach (InvocationType invocationType in new[] { InvocationType.Constructor, InvocationType.StaticMethods })
