Skip to content

Commit

Permalink
Make any I(Span)Parsable type a valid base type
Browse files Browse the repository at this point in the history
  • Loading branch information
DoctorKrolic committed Oct 27, 2024
1 parent 9ab547a commit 90baf40
Show file tree
Hide file tree
Showing 11 changed files with 452 additions and 21 deletions.
5 changes: 5 additions & 0 deletions src/ArgumentParsing.Generators/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### New Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
ARGP0054 | ArgumentParsing | Info |
30 changes: 19 additions & 11 deletions src/ArgumentParsing.Generators/ArgumentParserGenerator.CodeGen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen

var propertyName = info.PropertyName;
var parseStrategy = info.ParseStrategy;
var type = info.Type;
var nullableUnderlyingType = info.NullableUnderlyingType;
var sequenceType = info.SequenceType;

Expand All @@ -479,7 +480,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
writer.Ident++;
if (sequenceType != SequenceType.None)
{
writer.WriteLine($"{info.Type} {propertyName}_val = default({info.Type});");
writer.WriteLine($"{type} {propertyName}_val = default({type});");
}
switch (parseStrategy)
{
Expand All @@ -493,7 +494,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
writer.WriteLine($"{nullableUnderlyingType} {propertyName}_underlying = default({nullableUnderlyingType});");
}
var numberStyles = parseStrategy == ParseStrategy.Integer ? "global::System.Globalization.NumberStyles.Integer" : "global::System.Globalization.NumberStyles.Float | global::System.Globalization.NumberStyles.AllowThousands";
writer.WriteLine($"if (!{nullableUnderlyingType ?? info.Type}.TryParse(val, {numberStyles}, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.WriteLine($"if (!{nullableUnderlyingType ?? type}.TryParse(val, {numberStyles}, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write("errors.Add(new global::ArgumentParsing.Results.Errors.BadOptionValueFormatError(");
Expand Down Expand Up @@ -527,7 +528,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
{
writer.WriteLine($"{nullableUnderlyingType} {propertyName}_underlying = default({nullableUnderlyingType});");
}
writer.WriteLine($"if (!global::System.Enum.TryParse<{nullableUnderlyingType ?? info.Type}>(val, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.WriteLine($"if (!global::System.Enum.TryParse<{nullableUnderlyingType ?? type}>(val, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write("errors.Add(new global::ArgumentParsing.Results.Errors.BadOptionValueFormatError(");
Expand Down Expand Up @@ -563,7 +564,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
{
writer.WriteLine($"{nullableUnderlyingType} {propertyName}_underlying = default({nullableUnderlyingType});");
}
writer.WriteLine($"if (!{nullableUnderlyingType ?? info.Type}.TryParse(val, global::System.Globalization.CultureInfo.InvariantCulture, global::System.Globalization.DateTimeStyles.None, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.WriteLine($"if (!{nullableUnderlyingType ?? type}.TryParse(val, global::System.Globalization.CultureInfo.InvariantCulture, global::System.Globalization.DateTimeStyles.None, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write("errors.Add(new global::ArgumentParsing.Results.Errors.BadOptionValueFormatError(");
Expand All @@ -579,11 +580,13 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
}
break;
case ParseStrategy.TimeSpan:
case ParseStrategy.GenericSpanParsable:
case ParseStrategy.GenericParsable:
if (nullableUnderlyingType is not null)
{
writer.WriteLine($"{nullableUnderlyingType} {propertyName}_underlying = default({nullableUnderlyingType});");
}
writer.WriteLine($"if (!global::System.TimeSpan.TryParse(val, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.WriteLine($"if (!{nullableUnderlyingType ?? type}.TryParse(val{(canUseOptimalSpanBasedAlgorithm && parseStrategy == ParseStrategy.GenericParsable ? ".ToString()" : string.Empty)}, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write("errors.Add(new global::ArgumentParsing.Results.Errors.BadOptionValueFormatError(");
Expand Down Expand Up @@ -627,6 +630,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
var propertyName = info.PropertyName;
var parseStrategy = info.ParseStrategy;
Debug.Assert(parseStrategy != ParseStrategy.None);
var type = info.Type;
var nullableUnderlyingType = info.NullableUnderlyingType;

writer.WriteLine($"case {i}:");
Expand All @@ -643,7 +647,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
writer.WriteLine($"{nullableUnderlyingType} {propertyName}_underlying = default({nullableUnderlyingType});");
}
var numberStyles = parseStrategy == ParseStrategy.Integer ? "global::System.Globalization.NumberStyles.Integer" : "global::System.Globalization.NumberStyles.Float | global::System.Globalization.NumberStyles.AllowThousands";
writer.WriteLine($"if (!{nullableUnderlyingType ?? info.Type}.TryParse(val, {numberStyles}, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.WriteLine($"if (!{nullableUnderlyingType ?? type}.TryParse(val, {numberStyles}, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write($"errors.Add(new global::ArgumentParsing.Results.Errors.BadParameterValueFormatError(");
Expand Down Expand Up @@ -683,7 +687,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
{
writer.WriteLine($"{nullableUnderlyingType} {propertyName}_underlying = default({nullableUnderlyingType});");
}
writer.WriteLine($"if (!global::System.Enum.TryParse<{nullableUnderlyingType ?? info.Type}>(arg, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.WriteLine($"if (!global::System.Enum.TryParse<{nullableUnderlyingType ?? type}>(arg, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write($"errors.Add(new global::ArgumentParsing.Results.Errors.BadParameterValueFormatError(");
Expand Down Expand Up @@ -719,7 +723,7 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
{
writer.WriteLine($"{nullableUnderlyingType} {propertyName}_underlying = default({nullableUnderlyingType});");
}
writer.WriteLine($"if (!{nullableUnderlyingType ?? info.Type}.TryParse(val, global::System.Globalization.CultureInfo.InvariantCulture, global::System.Globalization.DateTimeStyles.None, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.WriteLine($"if (!{nullableUnderlyingType ?? type}.TryParse(val, global::System.Globalization.CultureInfo.InvariantCulture, global::System.Globalization.DateTimeStyles.None, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write($"errors.Add(new global::ArgumentParsing.Results.Errors.BadParameterValueFormatError(");
Expand All @@ -735,11 +739,13 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
}
break;
case ParseStrategy.TimeSpan:
case ParseStrategy.GenericSpanParsable:
case ParseStrategy.GenericParsable:
if (nullableUnderlyingType is not null)
{
writer.WriteLine($"{nullableUnderlyingType} {propertyName}_underlying = default({nullableUnderlyingType});");
}
writer.WriteLine($"if (!global::System.TimeSpan.TryParse(val, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.WriteLine($"if (!{nullableUnderlyingType ?? type}.TryParse(val{(canUseOptimalSpanBasedAlgorithm && parseStrategy == ParseStrategy.GenericParsable ? ".ToString()" : string.Empty)}, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}{(nullableUnderlyingType is not null ? "_underlying" : "_val")}))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write($"errors.Add(new global::ArgumentParsing.Results.Errors.BadParameterValueFormatError(");
Expand Down Expand Up @@ -881,8 +887,10 @@ private static void EmitArgumentParser(SourceProductionContext context, (Argumen
writer.WriteLine($"remainingParametersBuilder.Add({propertyName}_val);");
break;
case ParseStrategy.TimeSpan:
writer.WriteLine($"global::System.TimeSpan {propertyName}_val = default(global::System.TimeSpan);");
writer.WriteLine($"if (!global::System.TimeSpan.TryParse(arg, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}_val))");
case ParseStrategy.GenericSpanParsable:
case ParseStrategy.GenericParsable:
writer.WriteLine($"{type} {propertyName}_val = default({type});");
writer.WriteLine($"if (!{type}.TryParse(arg, global::System.Globalization.CultureInfo.InvariantCulture, out {propertyName}_val))");
writer.OpenBlock();
writer.WriteLine("errors ??= new();");
writer.Write("errors.Add(new global::ArgumentParsing.Results.Errors.BadRemainingParameterValueFormatError(");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ public partial class ArgumentParserGenerator
}
}

var parseStrategy = type.GetPrimaryParseStrategy();
var parseStrategy = type.GetPrimaryParseStrategy(compilation);
return (parseStrategy, nullableUnderlyingType, sequenceType, sequenceUnderlyingType);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public sealed class OptionsTypeAnalyzer : DiagnosticAnalyzer
DiagnosticDescriptors.PropertyCannotHaveMultipleParserRoles,
DiagnosticDescriptors.InvalidHelpTextGeneratorTypeSpecifier,
DiagnosticDescriptors.InvalidIdentifierName,
DiagnosticDescriptors.CannotFindHelpTextGeneratorMethod);
DiagnosticDescriptors.CannotFindHelpTextGeneratorMethod,
DiagnosticDescriptors.ImplementISpanParsable);

public override void Initialize(AnalysisContext context)
{
Expand All @@ -69,6 +70,8 @@ public override void Initialize(AnalysisContext context)
ImmutableArrayOfTType = comp.ImmutableArrayOfTType(),
HelpTextGeneratorAttributeType = comp.HelpTextGeneratorAttributeType(),
ParseErrorCollectionType = comp.ParseErrorCollectionType(),
ISpanParsableOfTType = comp.ISpanParsableOfTType(),
IParsableOfTType = comp.IParsableOfTType(),
};

var languageVersion = ((CSharpCompilation)comp).LanguageVersion;
Expand Down Expand Up @@ -436,7 +439,7 @@ knownTypes.SystemRuntimeCompilerServicesRequiredMemberAttributeType is not null
}
}

var (parseStrategy, isNullable, sequenceType) = GetParseStrategy(propertyType, knownTypes);
var (parseStrategy, isNullable, sequenceType, underlyingType) = GetParseStrategy(propertyType, knownTypes);
var propertySyntax = (BasePropertyDeclarationSyntax?)property.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(context.CancellationToken);
var locationForTypeRelatedDiagnostics = propertySyntax?.Type.GetLocation() ?? propertyLocation;

Expand Down Expand Up @@ -474,6 +477,17 @@ knownTypes.SystemRuntimeCompilerServicesRequiredMemberAttributeType is not null
Diagnostic.Create(
DiagnosticDescriptors.PreferImmutableArrayAsSequenceType, locationForTypeRelatedDiagnostics));
}

if (knownTypes.ISpanParsableOfTType is not null &&
parseStrategy == ParseStrategy.GenericParsable &&
underlyingType?.Locations.First() is { IsInSource: true } propertyTypeSourceLocation)
{
context.ReportDiagnostic(
Diagnostic.Create(
DiagnosticDescriptors.ImplementISpanParsable,
propertyTypeSourceLocation,
property.Name, optionsType, underlyingType));
}
}
else if (isParameter)
{
Expand Down Expand Up @@ -521,7 +535,7 @@ knownTypes.SystemRuntimeCompilerServicesRequiredMemberAttributeType is not null
DiagnosticDescriptors.InvalidParameterName, propertyLocation, parameterName));
}

var (parseStrategy, isNullable, sequenceType) = GetParseStrategy(propertyType, knownTypes);
var (parseStrategy, isNullable, sequenceType, _) = GetParseStrategy(propertyType, knownTypes);
if ((parseStrategy == ParseStrategy.None || sequenceType != SequenceType.None) && propertyType.TypeKind != TypeKind.Error)
{
var propertySyntax = (BasePropertyDeclarationSyntax?)property.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(context.CancellationToken);
Expand All @@ -533,6 +547,17 @@ knownTypes.SystemRuntimeCompilerServicesRequiredMemberAttributeType is not null
propertyType));
}

if (knownTypes.ISpanParsableOfTType is not null &&
parseStrategy == ParseStrategy.GenericParsable &&
propertyType.Locations.First() is { IsInSource: true } propertyTypeSourceLocation)
{
context.ReportDiagnostic(
Diagnostic.Create(
DiagnosticDescriptors.ImplementISpanParsable,
propertyTypeSourceLocation,
property.Name, optionsType, propertyType));
}

if (!hasParameter)
{
seenParametersWithTheirRequirements.Add(parameterIndex, isRequired);
Expand Down Expand Up @@ -565,7 +590,7 @@ knownTypes.SystemRuntimeCompilerServicesRequiredMemberAttributeType is not null

declaredRemainingParameters = true;

var (parseStrategy, isNullable, sequenceType) = GetParseStrategy(propertyType, knownTypes);
var (parseStrategy, isNullable, sequenceType, underlyingType) = GetParseStrategy(propertyType, knownTypes);
var propertySyntax = (BasePropertyDeclarationSyntax?)property.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(context.CancellationToken);
var locationForTypeRelatedDiagnostics = propertySyntax?.Type.GetLocation() ?? propertyLocation;

Expand All @@ -585,6 +610,17 @@ knownTypes.SystemRuntimeCompilerServicesRequiredMemberAttributeType is not null
DiagnosticDescriptors.PreferImmutableArrayAsSequenceType, locationForTypeRelatedDiagnostics));
}

if (knownTypes.ISpanParsableOfTType is not null &&
parseStrategy == ParseStrategy.GenericParsable &&
underlyingType?.Locations.First() is { IsInSource: true } propertyTypeSourceLocation)
{
context.ReportDiagnostic(
Diagnostic.Create(
DiagnosticDescriptors.ImplementISpanParsable,
propertyTypeSourceLocation,
property.Name, optionsType, underlyingType));
}

if (requiredAttributeReference is not null)
{
var syntax = requiredAttributeReference?.GetSyntax(context.CancellationToken);
Expand Down Expand Up @@ -666,7 +702,7 @@ knownTypes.SystemRuntimeCompilerServicesRequiredMemberAttributeType is not null
}
}

static (ParseStrategy, bool IsNullable, SequenceType) GetParseStrategy(ITypeSymbol type, KnownTypes knownTypes)
static (ParseStrategy, bool IsNullable, SequenceType, ITypeSymbol? UnderlyingType) GetParseStrategy(ITypeSymbol type, KnownTypes knownTypes)
{
if (type is not INamedTypeSymbol namedType)
{
Expand Down Expand Up @@ -712,7 +748,7 @@ knownTypes.SystemRuntimeCompilerServicesRequiredMemberAttributeType is not null
}
}

return (namedType.GetPrimaryParseStrategy(), isNullable, sequenceType);
return (namedType.GetPrimaryParseStrategy(knownTypes.ISpanParsableOfTType, knownTypes.IParsableOfTType), isNullable, sequenceType, namedType);
}
}

Expand Down Expand Up @@ -743,5 +779,9 @@ private readonly struct KnownTypes
public required INamedTypeSymbol? HelpTextGeneratorAttributeType { get; init; }

public required INamedTypeSymbol? ParseErrorCollectionType { get; init; }

public required INamedTypeSymbol? ISpanParsableOfTType { get; init; }

public required INamedTypeSymbol? IParsableOfTType { get; init; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -446,4 +446,12 @@ public static class DiagnosticDescriptors
category: ArgumentParsingCategoryName,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);

public static readonly DiagnosticDescriptor ImplementISpanParsable = new(
id: "ARGP0054",
title: "Implement 'ISpanParsable' interface to improve performance",
messageFormat: "For better performance in parsing member '{0}' of options type '{1}', implement the 'ISpanParsable' interface on '{2}'",
category: ArgumentParsingCategoryName,
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: true);
}
Loading

0 comments on commit 90baf40

Please sign in to comment.