Skip to content

Commit

Permalink
Enable GeneratedRegex on partial properties (#102977)
Browse files Browse the repository at this point in the history 
* Enable GeneratedRegex on partial properties

* Address PR feedback

* Suppress API compat warning
  • Loading branch information
@stephentoub
stephentoub authored Jun 3, 2024
1 parent c286a8e commit 2bf492b
Show file tree
Hide file tree
Showing 23 changed files with 527 additions and 127 deletions.
4 changes: 2 additions & 2 deletions src/libraries/System.Text.RegularExpressions/gen/DiagnosticDescriptors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ internal static class DiagnosticDescriptors
isEnabledByDefault: true,
customTags: WellKnownDiagnosticTags.NotConfigurable);

public static DiagnosticDescriptor RegexMethodMustHaveValidSignature { get; } = DiagnosticDescriptorHelper.Create(
public static DiagnosticDescriptor RegexMemberMustHaveValidSignature { get; } = DiagnosticDescriptorHelper.Create(
id: "SYSLIB1043",
title: new LocalizableResourceString(nameof(SR.InvalidGeneratedRegexAttributeTitle), SR.ResourceManager, typeof(FxResources.System.Text.RegularExpressions.Generator.SR)),
messageFormat: new LocalizableResourceString(nameof(SR.RegexMethodMustHaveValidSignatureMessage), SR.ResourceManager, typeof(FxResources.System.Text.RegularExpressions.Generator.SR)),
messageFormat: new LocalizableResourceString(nameof(SR.RegexMemberMustHaveValidSignatureMessage), SR.ResourceManager, typeof(FxResources.System.Text.RegularExpressions.Generator.SR)),
category: Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
Expand Down
11 changes: 8 additions & 3 deletions src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ private static void EmitRegexPartialMethod(RegexMethod regexMethod, IndentedText
writer.WriteLine($"/// </code>");
writer.WriteLine($"/// </remarks>");
writer.WriteLine($"[global::System.CodeDom.Compiler.{s_generatedCodeAttribute}]");
writer.WriteLine($"{regexMethod.Modifiers} global::System.Text.RegularExpressions.Regex {regexMethod.MethodName}() => global::{GeneratedNamespace}.{regexMethod.GeneratedName}.Instance;");
writer.Write($"{regexMethod.Modifiers} global::System.Text.RegularExpressions.Regex{(regexMethod.NullableRegex ? "?" : "")} {regexMethod.MemberName}");
if (!regexMethod.IsProperty)
{
writer.Write("()");
}
writer.WriteLine($" => global::{GeneratedNamespace}.{regexMethod.GeneratedName}.Instance;");

// Unwind all scopes
while (writer.Indent != 0)
Expand All @@ -89,7 +94,7 @@ private static void EmitRegexLimitedBoilerplate(
if (langVer >= LanguageVersion.CSharp11)
{
visibility = "file";
writer.WriteLine($"/// <summary>Caches a <see cref=\"Regex\"/> instance for the {rm.MethodName} method.</summary>");
writer.WriteLine($"/// <summary>Caches a <see cref=\"Regex\"/> instance for the {rm.MemberName} method.</summary>");
}
else
{
Expand Down Expand Up @@ -119,7 +124,7 @@ private static void EmitRegexLimitedBoilerplate(
private static void EmitRegexDerivedImplementation(
IndentedTextWriter writer, RegexMethod rm, string runnerFactoryImplementation, bool allowUnsafe)
{
writer.WriteLine($"/// <summary>Custom <see cref=\"Regex\"/>-derived type for the {rm.MethodName} method.</summary>");
writer.WriteLine($"/// <summary>Custom <see cref=\"Regex\"/>-derived type for the {rm.MemberName} method.</summary>");
writer.WriteLine($"[{s_generatedCodeAttribute}]");
if (allowUnsafe)
{
Expand Down
86 changes: 59 additions & 27 deletions src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Threading;
Expand All @@ -25,7 +26,16 @@ public partial class RegexGenerator
private static object? GetRegexMethodDataOrFailureDiagnostic(
GeneratorAttributeSyntaxContext context, CancellationToken cancellationToken)
{
var methodSyntax = (MethodDeclarationSyntax)context.TargetNode;
if (context.TargetNode is IndexerDeclarationSyntax or AccessorDeclarationSyntax)
{
// We allow these to be used as a target node for the sole purpose
// of being able to flag invalid use when [GeneratedRegex] is applied incorrectly.
// Otherwise, if the ForAttributeWithMetadataName call excluded these, [GeneratedRegex]
// could be applied to them and we wouldn't be able to issue a diagnostic.
return new DiagnosticData(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, GetComparableLocation(context.TargetNode));
}

var memberSyntax = (MemberDeclarationSyntax)context.TargetNode;
SemanticModel sm = context.SemanticModel;

Compilation compilation = sm.Compilation;
Expand All @@ -37,34 +47,34 @@ public partial class RegexGenerator
return null;
}

TypeDeclarationSyntax? typeDec = methodSyntax.Parent as TypeDeclarationSyntax;
TypeDeclarationSyntax? typeDec = memberSyntax.Parent as TypeDeclarationSyntax;
if (typeDec is null)
{
return null;
}

IMethodSymbol? regexMethodSymbol = context.TargetSymbol as IMethodSymbol;
if (regexMethodSymbol is null)
ISymbol? regexMemberSymbol = context.TargetSymbol is IMethodSymbol or IPropertySymbol ? context.TargetSymbol : null;
if (regexMemberSymbol is null)
{
return null;
}

ImmutableArray<AttributeData> boundAttributes = context.Attributes;
if (boundAttributes.Length != 1)
{
return new DiagnosticData(DiagnosticDescriptors.MultipleGeneratedRegexAttributes, GetComparableLocation(methodSyntax));
return new DiagnosticData(DiagnosticDescriptors.MultipleGeneratedRegexAttributes, GetComparableLocation(memberSyntax));
}
AttributeData generatedRegexAttr = boundAttributes[0];

if (generatedRegexAttr.ConstructorArguments.Any(ca => ca.Kind == TypedConstantKind.Error))
{
return new DiagnosticData(DiagnosticDescriptors.InvalidGeneratedRegexAttribute, GetComparableLocation(methodSyntax));
return new DiagnosticData(DiagnosticDescriptors.InvalidGeneratedRegexAttribute, GetComparableLocation(memberSyntax));
}

ImmutableArray<TypedConstant> items = generatedRegexAttr.ConstructorArguments;
if (items.Length is 0 or > 4)
{
return new DiagnosticData(DiagnosticDescriptors.InvalidGeneratedRegexAttribute, GetComparableLocation(methodSyntax));
return new DiagnosticData(DiagnosticDescriptors.InvalidGeneratedRegexAttribute, GetComparableLocation(memberSyntax));
}

string? pattern = items[0].Value as string;
Expand Down Expand Up @@ -96,16 +106,36 @@ public partial class RegexGenerator

if (pattern is null || cultureName is null)
{
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(methodSyntax), "(null)");
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "(null)");
}

if (!regexMethodSymbol.IsPartialDefinition ||
regexMethodSymbol.IsAbstract ||
regexMethodSymbol.Parameters.Length != 0 ||
regexMethodSymbol.Arity != 0 ||
!SymbolEqualityComparer.Default.Equals(regexMethodSymbol.ReturnType, regexSymbol))
bool nullableRegex;
if (regexMemberSymbol is IMethodSymbol regexMethodSymbol)
{
return new DiagnosticData(DiagnosticDescriptors.RegexMethodMustHaveValidSignature, GetComparableLocation(methodSyntax));
if (!regexMethodSymbol.IsPartialDefinition ||
regexMethodSymbol.IsAbstract ||
regexMethodSymbol.Parameters.Length != 0 ||
regexMethodSymbol.Arity != 0 ||
!SymbolEqualityComparer.Default.Equals(regexMethodSymbol.ReturnType, regexSymbol))
{
return new DiagnosticData(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, GetComparableLocation(memberSyntax));
}

nullableRegex = regexMethodSymbol.ReturnNullableAnnotation == NullableAnnotation.Annotated;
}
else
{
Debug.Assert(regexMemberSymbol is IPropertySymbol);
IPropertySymbol regexPropertySymbol = (IPropertySymbol)regexMemberSymbol;
if (!memberSyntax.Modifiers.Any(SyntaxKind.PartialKeyword) || // TODO: Switch to using regexPropertySymbol.IsPartialDefinition when available
regexPropertySymbol.IsAbstract ||
regexPropertySymbol.SetMethod is not null ||
!SymbolEqualityComparer.Default.Equals(regexPropertySymbol.Type, regexSymbol))
{
return new DiagnosticData(DiagnosticDescriptors.RegexMemberMustHaveValidSignature, GetComparableLocation(memberSyntax));
}

nullableRegex = regexPropertySymbol.NullableAnnotation == NullableAnnotation.Annotated;
}

RegexOptions regexOptions = options is not null ? (RegexOptions)options : RegexOptions.None;
Expand All @@ -124,15 +154,15 @@ public partial class RegexGenerator
}
catch (Exception e)
{
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(methodSyntax), e.Message);
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), e.Message);
}

if ((regexOptionsWithPatternOptions & RegexOptions.IgnoreCase) != 0 && !string.IsNullOrEmpty(cultureName))
{
if ((regexOptions & RegexOptions.CultureInvariant) != 0)
{
// User passed in both a culture name and set RegexOptions.CultureInvariant which causes an explicit conflict.
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(methodSyntax), "cultureName");
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "cultureName");
}

try
Expand All @@ -141,7 +171,7 @@ public partial class RegexGenerator
}
catch (CultureNotFoundException)
{
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(methodSyntax), "cultureName");
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "cultureName");
}
}

Expand All @@ -159,17 +189,17 @@ public partial class RegexGenerator
RegexOptions.Singleline;
if ((regexOptions & ~SupportedOptions) != 0)
{
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(methodSyntax), "options");
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "options");
}

// Validate the timeout
if (matchTimeout is 0 or < -1)
{
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(methodSyntax), "matchTimeout");
return new DiagnosticData(DiagnosticDescriptors.InvalidRegexArguments, GetComparableLocation(memberSyntax), "matchTimeout");
}

// Determine the namespace the class is declared in, if any
string? ns = regexMethodSymbol.ContainingType?.ContainingNamespace?.ToDisplayString(
string? ns = regexMemberSymbol.ContainingType?.ContainingNamespace?.ToDisplayString(
SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted));

var regexType = new RegexType(
Expand All @@ -183,9 +213,11 @@ public partial class RegexGenerator

var result = new RegexPatternAndSyntax(
regexType,
GetComparableLocation(methodSyntax),
regexMethodSymbol.Name,
methodSyntax.Modifiers.ToString(),
IsProperty: regexMemberSymbol is IPropertySymbol,
GetComparableLocation(memberSyntax),
regexMemberSymbol.Name,
memberSyntax.Modifiers.ToString(),
nullableRegex,
pattern,
regexOptions,
matchTimeout,
Expand Down Expand Up @@ -217,18 +249,18 @@ SyntaxKind.RecordStructDeclaration or

// Get a Location object that doesn't store a reference to the compilation.
// That allows it to compare equally across compilations.
static Location GetComparableLocation(MethodDeclarationSyntax method)
static Location GetComparableLocation(SyntaxNode syntax)
{
var location = method.GetLocation();
var location = syntax.GetLocation();
return Location.Create(location.SourceTree?.FilePath ?? string.Empty, location.SourceSpan, location.GetLineSpan().Span);
}
}

/// <summary>Data about a regex directly from the GeneratedRegex attribute.</summary>
internal sealed record RegexPatternAndSyntax(RegexType DeclaringType, Location DiagnosticLocation, string MethodName, string Modifiers, string Pattern, RegexOptions Options, int? MatchTimeout, CultureInfo Culture, CompilationData CompilationData);
internal sealed record RegexPatternAndSyntax(RegexType DeclaringType, bool IsProperty, Location DiagnosticLocation, string MemberName, string Modifiers, bool NullableRegex, string Pattern, RegexOptions Options, int? MatchTimeout, CultureInfo Culture, CompilationData CompilationData);

/// <summary>Data about a regex, including a fully parsed RegexTree and subsequent analysis.</summary>
internal sealed record RegexMethod(RegexType DeclaringType, Location DiagnosticLocation, string MethodName, string Modifiers, string Pattern, RegexOptions Options, int? MatchTimeout, RegexTree Tree, AnalysisResults Analysis, CompilationData CompilationData)
internal sealed record RegexMethod(RegexType DeclaringType, bool IsProperty, Location DiagnosticLocation, string MemberName, string Modifiers, bool NullableRegex, string Pattern, RegexOptions Options, int? MatchTimeout, RegexTree Tree, AnalysisResults Analysis, CompilationData CompilationData)
{
public string? GeneratedName { get; set; }
public bool IsDuplicate { get; set; }
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
// if there are no changes.
.ForAttributeWithMetadataName(
GeneratedRegexAttributeName,
(node, _) => node is MethodDeclarationSyntax,
(node, _) => node is MethodDeclarationSyntax or PropertyDeclarationSyntax or IndexerDeclarationSyntax or AccessorDeclarationSyntax,
GetRegexMethodDataOrFailureDiagnostic)

// Filter out any parsing errors that resulted in null objects being returned.
Expand All @@ -73,7 +73,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
{
RegexTree regexTree = RegexParser.Parse(method.Pattern, method.Options | RegexOptions.Compiled, method.Culture); // make sure Compiled is included to get all optimizations applied to it
AnalysisResults analysis = RegexTreeAnalyzer.Analyze(regexTree);
return new RegexMethod(method.DeclaringType, method.DiagnosticLocation, method.MethodName, method.Modifiers, method.Pattern, method.Options, method.MatchTimeout, regexTree, analysis, method.CompilationData);
return new RegexMethod(method.DeclaringType, method.IsProperty, method.DiagnosticLocation, method.MemberName, method.Modifiers, method.NullableRegex, method.Pattern, method.Options, method.MatchTimeout, regexTree, analysis, method.CompilationData);
}
catch (Exception e)
{
Expand Down Expand Up @@ -201,7 +201,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
else
{
regexMethod.IsDuplicate = false;
regexMethod.GeneratedName = $"{regexMethod.MethodName}_{id++}";
regexMethod.GeneratedName = $"{regexMethod.MemberName}_{id++}";
emittedExpressions.Add(key, regexMethod);
}

Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Text.RegularExpressions/gen/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@
<data name="InvalidRegexArgumentsMessage" xml:space="preserve">
<value>The specified regex is invalid. '{0}'</value>
</data>
<data name="RegexMethodMustHaveValidSignatureMessage" xml:space="preserve">
<value>GeneratedRegexAttribute method must be partial, parameterless, non-generic, non-abstract, and return Regex.</value>
<data name="RegexMemberMustHaveValidSignatureMessage" xml:space="preserve">
<value>GeneratedRegexAttribute method or property must be partial, parameterless, non-generic, non-abstract, and return Regex. If a property, it must also be get-only.</value>
</data>
<data name="LimitedSourceGenerationTitle" xml:space="preserve">
<value>Regex generator limitation reached.</value>
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.Text.RegularExpressions/gen/Resources/xlf/Strings.cs.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@
<target state="translated">Vypršel časový limit modulu Regex při pokusu o porovnání vzoru se vstupním řetězcem. K tomu může dojít z celé řady důvodů, mezi které patří velká velikost vstupních dat nebo nadměrné zpětné navracení způsobené vloženými kvantifikátory, zpětnými odkazy a dalšími faktory.</target>
<note />
</trans-unit>
<trans-unit id="RegexMethodMustHaveValidSignatureMessage">
<source>GeneratedRegexAttribute method must be partial, parameterless, non-generic, non-abstract, and return Regex.</source>
<target state="translated">Metoda GeneratedRegexAttribute musí být částečná, bez parametrů, neobecná, neabstraktní a návratová metoda Regex.</target>
<trans-unit id="RegexMemberMustHaveValidSignatureMessage">
<source>GeneratedRegexAttribute method or property must be partial, parameterless, non-generic, non-abstract, and return Regex. If a property, it must also be get-only.</source>
<target state="needs-review-translation">Metoda GeneratedRegexAttribute musí být částečná, bez parametrů, neobecná, neabstraktní a návratová metoda Regex.</target>
<note />
</trans-unit>
<trans-unit id="ReversedCharacterRange">
Expand Down
Loading

0 comments on commit 2bf492b

Please sign in to comment.