Simplify UpgradeToRegexGenerator (#72538)
authorYoussef Victor <youssefvictor00@gmail.com>
Tue, 26 Jul 2022 23:31:56 +0000 (01:31 +0200)
committerGitHub <noreply@github.com>
Tue, 26 Jul 2022 23:31:56 +0000 (16:31 -0700)
* Minor refactoring for UpgradeToRegexGenerator

* Don't use properties bag

* Rename

* Update UpgradeToRegexGeneratorCodeFixer.cs

* Update src/libraries/System.Text.RegularExpressions/gen/UpgradeToRegexGeneratorCodeFixer.cs

Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>
src/libraries/System.Text.RegularExpressions/gen/UpgradeToRegexGeneratorAnalyzer.cs
src/libraries/System.Text.RegularExpressions/gen/UpgradeToRegexGeneratorCodeFixer.cs

index 62efa98..d3f68bd 100644 (file)
@@ -27,8 +27,8 @@ namespace System.Text.RegularExpressions.Generator
         private const string RegexTypeName = "System.Text.RegularExpressions.Regex";
         private const string RegexGeneratorTypeName = "System.Text.RegularExpressions.RegexGeneratorAttribute";
 
-        internal const string PatternIndexName = "PatternIndex";
-        internal const string RegexOptionsIndexName = "RegexOptionsIndex";
+        internal const string PatternArgumentName = "pattern";
+        internal const string OptionsArgumentName = "options";
 
         /// <inheritdoc />
         public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.UseRegexSourceGeneration);
@@ -107,26 +107,16 @@ namespace System.Text.RegularExpressions.Generator
             // code fixer can later use that property bag to generate the code fix and emit the RegexGenerator attribute.
             if (staticMethodsToDetect.Contains(method))
             {
-                string? patternArgumentIndex = null;
-                string? optionsArgumentIndex = null;
-
                 // Validate that arguments pattern and options are constant and timeout was not passed in.
-                if (!TryValidateParametersAndExtractArgumentIndices(invocationOperation.Arguments, ref patternArgumentIndex, ref optionsArgumentIndex))
+                if (!ValidateParameters(invocationOperation.Arguments))
                 {
                     return;
                 }
 
-                // Create the property bag.
-                ImmutableDictionary<string, string?> properties = ImmutableDictionary.CreateRange(new[]
-                {
-                    new KeyValuePair<string, string?>(PatternIndexName, patternArgumentIndex),
-                    new KeyValuePair<string, string?>(RegexOptionsIndexName, optionsArgumentIndex)
-                });
-
                 // Report the diagnostic.
                 SyntaxNode? syntaxNodeForDiagnostic = invocationOperation.Syntax;
                 Debug.Assert(syntaxNodeForDiagnostic != null);
-                context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.UseRegexSourceGeneration, syntaxNodeForDiagnostic.GetLocation(), properties));
+                context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.UseRegexSourceGeneration, syntaxNodeForDiagnostic.GetLocation()));
             }
         }
 
@@ -150,37 +140,25 @@ namespace System.Text.RegularExpressions.Generator
                 return;
             }
 
-            string? patternArgumentIndex = null;
-            string? optionsArgumentIndex = null;
-
-            if (!TryValidateParametersAndExtractArgumentIndices(operation.Arguments, ref patternArgumentIndex, ref optionsArgumentIndex))
+            if (!ValidateParameters(operation.Arguments))
             {
                 return;
             }
 
-            // Create the property bag.
-            ImmutableDictionary<string, string?> properties = ImmutableDictionary.CreateRange(new[]
-            {
-                new KeyValuePair<string, string?>(PatternIndexName, patternArgumentIndex),
-                new KeyValuePair<string, string?>(RegexOptionsIndexName, optionsArgumentIndex)
-            });
-
             // Report the diagnostic.
             SyntaxNode? syntaxNodeForDiagnostic = operation.Syntax;
             Debug.Assert(syntaxNodeForDiagnostic is not null);
-            context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.UseRegexSourceGeneration, syntaxNodeForDiagnostic.GetLocation(), properties));
+            context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.UseRegexSourceGeneration, syntaxNodeForDiagnostic.GetLocation()));
         }
 
         /// <summary>
         /// Validates the operation arguments ensuring they all have constant values, and if so it stores the argument
         /// indices for the pattern and options. If timeout argument was used, then this returns false.
         /// </summary>
-        private static bool TryValidateParametersAndExtractArgumentIndices(ImmutableArray<IArgumentOperation> arguments, ref string? patternArgumentIndex, ref string? optionsArgumentIndex)
+        private static bool ValidateParameters(ImmutableArray<IArgumentOperation> arguments)
         {
             const string timeoutArgumentName = "timeout";
             const string matchTimeoutArgumentName = "matchTimeout";
-            const string patternArgumentName = "pattern";
-            const string optionsArgumentName = "options";
 
             if (arguments == null)
             {
@@ -200,19 +178,18 @@ namespace System.Text.RegularExpressions.Generator
                 }
 
                 // If the argument is the pattern, then we validate that it is constant and we store the index.
-                if (argumentName.Equals(patternArgumentName, StringComparison.OrdinalIgnoreCase))
+                if (argumentName.Equals(PatternArgumentName, StringComparison.OrdinalIgnoreCase))
                 {
                     if (!IsConstant(argument))
                     {
                         return false;
                     }
 
-                    patternArgumentIndex = i.ToString();
                     continue;
                 }
 
                 // If the argument is the options, then we validate that it is constant, that it doesn't have RegexOptions.NonBacktracking, and we store the index.
-                if (argumentName.Equals(optionsArgumentName, StringComparison.OrdinalIgnoreCase))
+                if (argumentName.Equals(OptionsArgumentName, StringComparison.OrdinalIgnoreCase))
                 {
                     if (!IsConstant(argument))
                     {
@@ -225,7 +202,6 @@ namespace System.Text.RegularExpressions.Generator
                         return false;
                     }
 
-                    optionsArgumentIndex = i.ToString();
                     continue;
                 }
             }
index f9c888a..518bf6d 100644 (file)
@@ -155,52 +155,30 @@ namespace System.Text.RegularExpressions.Generator
             // We generate a new invocation node to call our new partial method, and use it to replace the nodeToFix.
             DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
             SyntaxGenerator generator = editor.Generator;
-            ImmutableDictionary<string, string?> properties = diagnostic.Properties;
 
             // Generate the modified type declaration depending on whether the callsite was a Regex constructor call
             // or a Regex static method invocation.
+            SyntaxNode replacement = generator.InvocationExpression(generator.IdentifierName(methodName));
+            ImmutableArray<IArgumentOperation> operationArguments;
             if (operation is IInvocationOperation invocationOperation) // When using a Regex static method
             {
-                ImmutableArray<IArgumentOperation> arguments = invocationOperation.Arguments;
+                operationArguments = invocationOperation.Arguments;
+                IEnumerable<SyntaxNode> arguments = operationArguments
+                    .Where(arg => arg.Parameter.Name is not (UpgradeToRegexGeneratorAnalyzer.OptionsArgumentName or UpgradeToRegexGeneratorAnalyzer.PatternArgumentName))
+                    .Select(arg => arg.Syntax);
 
-                // Parse the idices for where to get the arguments from.
-                int?[] indices = new[]
-                {
-                    TryParseInt32(properties, UpgradeToRegexGeneratorAnalyzer.PatternIndexName),
-                    TryParseInt32(properties, UpgradeToRegexGeneratorAnalyzer.RegexOptionsIndexName)
-                };
-
-                foreach (int? index in indices.Where(value => value != null).OrderByDescending(value => value))
-                {
-                    arguments = arguments.RemoveAt(index.GetValueOrDefault());
-                }
-
-                SyntaxNode createRegexMethod = generator.InvocationExpression(generator.IdentifierName(methodName));
-                SyntaxNode method = generator.InvocationExpression(generator.MemberAccessExpression(createRegexMethod, invocationOperation.TargetMethod.Name), arguments.Select(arg => arg.Syntax).ToArray());
-
-                newTypeDeclarationOrCompilationUnit = newTypeDeclarationOrCompilationUnit.ReplaceNode(nodeToFix, WithTrivia(method, nodeToFix));
+                replacement = generator.InvocationExpression(generator.MemberAccessExpression(replacement, invocationOperation.TargetMethod.Name), arguments);
             }
-            else // When using a Regex constructor
+            else
             {
-                SyntaxNode invokeMethod = generator.InvocationExpression(generator.IdentifierName(methodName));
-                newTypeDeclarationOrCompilationUnit = newTypeDeclarationOrCompilationUnit.ReplaceNode(nodeToFix, WithTrivia(invokeMethod, nodeToFix));
+                operationArguments = ((IObjectCreationOperation)operation).Arguments;
             }
 
-            // Initialize the inputs for the RegexGenerator attribute.
-            SyntaxNode? patternValue = null;
-            SyntaxNode? regexOptionsValue = null;
+            newTypeDeclarationOrCompilationUnit = newTypeDeclarationOrCompilationUnit.ReplaceNode(nodeToFix, WithTrivia(replacement, nodeToFix));
 
-            // Try to get the pattern and RegexOptions values out from the diagnostic's property bag.
-            if (operation is IObjectCreationOperation objectCreationOperation) // When using the Regex constructors
-            {
-                patternValue = GetNode((objectCreationOperation).Arguments, properties, UpgradeToRegexGeneratorAnalyzer.PatternIndexName, generator, useOptionsMemberExpression: false, compilation, cancellationToken);
-                regexOptionsValue = GetNode((objectCreationOperation).Arguments, properties, UpgradeToRegexGeneratorAnalyzer.RegexOptionsIndexName, generator, useOptionsMemberExpression: true, compilation, cancellationToken);
-            }
-            else if (operation is IInvocationOperation invocation) // When using the Regex static methods.
-            {
-                patternValue = GetNode(invocation.Arguments, properties, UpgradeToRegexGeneratorAnalyzer.PatternIndexName, generator, useOptionsMemberExpression: false, compilation, cancellationToken);
-                regexOptionsValue = GetNode(invocation.Arguments, properties, UpgradeToRegexGeneratorAnalyzer.RegexOptionsIndexName, generator, useOptionsMemberExpression: true, compilation, cancellationToken);
-            }
+            // Initialize the inputs for the RegexGenerator attribute.
+            SyntaxNode? patternValue = GetNode(operationArguments, generator, UpgradeToRegexGeneratorAnalyzer.PatternArgumentName);
+            SyntaxNode? regexOptionsValue = GetNode(operationArguments, generator, UpgradeToRegexGeneratorAnalyzer.OptionsArgumentName);
 
             // Generate the new static partial method
             MethodDeclarationSyntax newMethod = (MethodDeclarationSyntax)generator.MethodDeclaration(
@@ -244,57 +222,39 @@ namespace System.Text.RegularExpressions.Generator
                 }
             }
 
-            // Helper method that searches the passed in property bag for the property with the passed in name, and if found, it converts the
-            // value to an int.
-            static int? TryParseInt32(ImmutableDictionary<string, string?> properties, string name)
-            {
-                if (!properties.TryGetValue(name, out string? value))
-                {
-                    return null;
-                }
-
-                if (!int.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out int result))
-                {
-                    return null;
-                }
-
-                return result;
-            }
-
-            // Helper method that looks int the properties bag for the index of the passed in propertyname, and then returns that index from the args parameter.
-            static SyntaxNode? GetNode(ImmutableArray<IArgumentOperation> args, ImmutableDictionary<string, string?> properties, string propertyName, SyntaxGenerator generator, bool useOptionsMemberExpression, Compilation compilation, CancellationToken cancellationToken)
+            // Helper method that looks generates the node for pattern argument or options argument.
+            static SyntaxNode? GetNode(ImmutableArray<IArgumentOperation> arguments, SyntaxGenerator generator, string parameterName)
             {
-                int? index = TryParseInt32(properties, propertyName);
-                if (index == null)
+                var argument = arguments.SingleOrDefault(arg => arg.Parameter.Name == parameterName);
+                if (argument is null)
                 {
                     return null;
                 }
 
-                if (!useOptionsMemberExpression)
+                Debug.Assert(parameterName is UpgradeToRegexGeneratorAnalyzer.OptionsArgumentName or UpgradeToRegexGeneratorAnalyzer.PatternArgumentName);
+                if (parameterName == UpgradeToRegexGeneratorAnalyzer.OptionsArgumentName)
                 {
-                    return generator.LiteralExpression(args[index.Value].Value.ConstantValue.Value);
+                    string optionsLiteral = Literal(((RegexOptions)(int)argument.Value.ConstantValue.Value).ToString());
+                    return SyntaxFactory.ParseExpression(optionsLiteral);
                 }
                 else
                 {
-                    RegexOptions options = (RegexOptions)(int)args[index.Value].Value.ConstantValue.Value;
-                    string optionsLiteral = Literal(options);
-                    return SyntaxFactory.ParseExpression(optionsLiteral).SyntaxTree.GetRoot(cancellationToken);
+                    return generator.LiteralExpression(argument.Value.ConstantValue.Value);
                 }
             }
 
-            static string Literal(RegexOptions options)
+            static string Literal(string stringifiedRegexOptions)
             {
-                string s = options.ToString();
-                if (int.TryParse(s, NumberStyles.Integer, CultureInfo.InvariantCulture, out _))
+                if (int.TryParse(stringifiedRegexOptions, NumberStyles.Integer, CultureInfo.InvariantCulture, out int options))
                 {
                     // The options were formatted as an int, which means the runtime couldn't
                     // produce a textual representation.  So just output casting the value as an int.
-                    return $"(RegexOptions)({(int)options})";
+                    return $"(RegexOptions)({options})";
                 }
 
                 // Parse the runtime-generated "Option1, Option2" into each piece and then concat
                 // them back together.
-                string[] parts = s.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
+                string[] parts = stringifiedRegexOptions.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
                 for (int i = 0; i < parts.Length; i++)
                 {
                     parts[i] = "RegexOptions." + parts[i].Trim();