Skip to content
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

Enable GeneratedRegex on partial properties #102977

Merged
merged 4 commits into from
Jun 3, 2024
Merged

Enable GeneratedRegex on partial properties #102977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43743fe
Enable GeneratedRegex on partial properties
stephentoub Jun 3, 2024
b54ac5c
Merge branch 'main' into generatedregexproperty
stephentoub Jun 3, 2024
4f3d988
Address PR feedback
stephentoub Jun 3, 2024
9b67d6f
Suppress API compat warning
stephentoub Jun 3, 2024
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
4 changes: 2 additions & 2 deletions src/libraries/System.Text.RegularExpressions/gen/DiagnosticDescriptors.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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 ||
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
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;
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}

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
Open in desktop
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
Open in desktop
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>
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
</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
Open in desktop
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
Loading