Skip to content

Simplify UpgradeToRegexGenerator #72538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 26, 2022
Merged

Simplify UpgradeToRegexGenerator #72538

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b896c4a
Minor refactoring for UpgradeToRegexGenerator
Youssef1313 Jul 20, 2022
0dc31e0
Don't use properties bag
Youssef1313 Jul 24, 2022
686e4f4
Rename
Youssef1313 Jul 26, 2022
fcf9094
Update UpgradeToRegexGeneratorCodeFixer.cs
Youssef1313 Jul 26, 2022
61f9b44
Update src/libraries/System.Text.RegularExpressions/gen/UpgradeToRege…
joperezr Jul 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 9 additions & 33 deletions src/libraries/System.Text.RegularExpressions/gen/UpgradeToRegexGeneratorAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ public sealed class UpgradeToRegexGeneratorAnalyzer : DiagnosticAnalyzer
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);
Expand Down Expand Up @@ -107,26 +107,16 @@ private static void AnalyzeInvocation(OperationAnalysisContext context, INamedTy
// 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 (!TryValidateParametersAndExtractArgumentIndices(invocationOperation.Arguments))
Copy link
Member

@joperezr joperezr Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small NIT: The name of the method is technically not accurate any longer, now we are only doing validation of the Parameters so we could rename to ValidateParameters

{
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()));
}
}

Expand All @@ -150,37 +140,25 @@ private static void AnalyzeObjectCreation(OperationAnalysisContext context, INam
return;
}

string? patternArgumentIndex = null;
string? optionsArgumentIndex = null;

if (!TryValidateParametersAndExtractArgumentIndices(operation.Arguments, ref patternArgumentIndex, ref optionsArgumentIndex))
if (!TryValidateParametersAndExtractArgumentIndices(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 TryValidateParametersAndExtractArgumentIndices(ImmutableArray<IArgumentOperation> arguments)
{
const string timeoutArgumentName = "timeout";
const string matchTimeoutArgumentName = "matchTimeout";
const string patternArgumentName = "pattern";
const string optionsArgumentName = "options";

if (arguments == null)
{
Expand All @@ -200,19 +178,18 @@ private static bool TryValidateParametersAndExtractArgumentIndices(ImmutableArra
}

// 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))
{
Expand All @@ -225,7 +202,6 @@ private static bool TryValidateParametersAndExtractArgumentIndices(ImmutableArra
return false;
}

optionsArgumentIndex = i.ToString();
continue;
}
}
Expand Down
95 changes: 28 additions & 67 deletions src/libraries/System.Text.RegularExpressions/gen/UpgradeToRegexGeneratorCodeFixer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
Expand All @@ -24,7 +25,7 @@ namespace System.Text.RegularExpressions.Generator
/// Roslyn code fixer that will listen to SysLIB1046 diagnostics and will provide a code fix which onboards a particular Regex into
/// source generation.
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp)]
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
Copy link
Member

@joperezr joperezr Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like in other similar places we aren't adding this attribute, like:

runtime/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/ConvertToLibraryImportFixer.cs

Line 23 in 39d82c9

[ExportCodeFixProvider(LanguageNames.CSharp)]

I'm not saying adding the attribute is wrong, but it also isn't very related to this PR so we should probably live that to a follow up PR.

public sealed class UpgradeToRegexGeneratorCodeFixer : CodeFixProvider
{
private const string RegexTypeName = "System.Text.RegularExpressions.Regex";
Expand Down Expand Up @@ -155,52 +156,30 @@ private static async Task<Document> ConvertToSourceGenerator(Document document,
// 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(
Expand Down Expand Up @@ -244,57 +223,39 @@ static IEnumerable<ISymbol> GetAllMembers(ITypeSymbol? symbol)
}
}

// 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();
Expand Down