Skip to content

Fix code generation for parsing optional array parameters in RequestDelegateGenerator #57336

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 8 commits into from
Sep 23, 2024
Merged

Fix code generation for parsing optional array parameters in RequestDelegateGenerator #57336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3f5cb0e
`EndpointParameter.ParsingBlockEmitter` property lifted out into `End…
ithline Aug 14, 2024
cf3710e
add array tests and fixed emitter for nullable types
ithline Aug 14, 2024
c61296e
fix keyed service test in environments that use comma (`,`) as decima…
ithline Aug 14, 2024
c571a3e
Merge branch 'dotnet:main' into rdg-optional-array
ithline Aug 14, 2024
7d313cb
add more tests for string optional array
ithline Aug 14, 2024
898e840
created baselines for optional array binding tests
ithline Aug 22, 2024
41b0fc2
Merge branch 'dotnet:main' into rdg-optional-array
ithline Sep 17, 2024
1ca8e20
Merge branch 'dotnet:main' into rdg-optional-array
ithline Sep 23, 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
75 changes: 69 additions & 6 deletions src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointParameterEmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,26 @@ internal static void EmitFormParameterPreparation(this EndpointParameter endpoin

internal static void EmitParsingBlock(this EndpointParameter endpointParameter, CodeWriter codeWriter)
{
// parsable array
if (endpointParameter.IsArray && endpointParameter.IsParsable)
{
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)} {endpointParameter.EmitHandlerArgument()} = new {endpointParameter.ElementType.ToDisplayString(EmitterConstants.DisplayFormat)}[{endpointParameter.EmitTempArgument()}.Length];");
var createArray = $"new {endpointParameter.ElementType.ToDisplayString(EmitterConstants.DisplayFormat)}[{endpointParameter.EmitTempArgument()}.Length]";

// we assign a null to result parameter if it's optional array, otherwise we create new array immediately
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)} {endpointParameter.EmitHandlerArgument()} = {createArray};");

codeWriter.WriteLine($"for (var i = 0; i < {endpointParameter.EmitTempArgument()}.Length; i++)");
codeWriter.StartBlock();
codeWriter.WriteLine($"var element = {endpointParameter.EmitTempArgument()}[i];");
endpointParameter.ParsingBlockEmitter(codeWriter, "element", "parsed_element");

// emit parsing block for current array element
codeWriter.WriteLine($$"""if (!{{endpointParameter.PreferredTryParseInvocation("element", "parsed_element")}})""");
codeWriter.StartBlock();
codeWriter.WriteLine("if (!string.IsNullOrEmpty(element))");
codeWriter.StartBlock();
EmitLogOrThrowException(endpointParameter, codeWriter, "element");
codeWriter.EndBlock();
codeWriter.EndBlock();

// In cases where we are dealing with an array of parsable nullables we need to substitute
// empty strings for null values.
Expand All @@ -109,19 +122,69 @@ internal static void EmitParsingBlock(this EndpointParameter endpointParameter,
}
codeWriter.EndBlock();
}
else if (endpointParameter.IsArray && !endpointParameter.IsParsable)
// array fallback
else if (endpointParameter.IsArray)
{
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {endpointParameter.EmitHandlerArgument()} = {endpointParameter.EmitTempArgument()}!;");
}
else if (!endpointParameter.IsArray && endpointParameter.IsParsable)
// parsable single
else if (endpointParameter.IsParsable)
{
endpointParameter.ParsingBlockEmitter(codeWriter, endpointParameter.EmitTempArgument(), endpointParameter.EmitParsedTempArgument());
var temp_argument = endpointParameter.EmitTempArgument();
var output_argument = endpointParameter.EmitParsedTempArgument();

// emit parsing block for optional OR nullable values
if (endpointParameter.IsOptional || endpointParameter.Type.NullableAnnotation == NullableAnnotation.Annotated)
{
var temp_argument_parsed_non_nullable = $"{temp_argument}_parsed_non_nullable";

codeWriter.WriteLine($"""{endpointParameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)} {output_argument} = default;""");
codeWriter.WriteLine($"""if ({endpointParameter.PreferredTryParseInvocation(temp_argument, temp_argument_parsed_non_nullable)})""");
codeWriter.StartBlock();
codeWriter.WriteLine($"""{output_argument} = {temp_argument_parsed_non_nullable};""");
codeWriter.EndBlock();
codeWriter.WriteLine($"""else if (string.IsNullOrEmpty({temp_argument}))""");
codeWriter.StartBlock();
codeWriter.WriteLine($"""{output_argument} = {endpointParameter.DefaultValue};""");
codeWriter.EndBlock();
codeWriter.WriteLine("else");
codeWriter.StartBlock();
codeWriter.WriteLine("wasParamCheckFailure = true;");
codeWriter.EndBlock();
}
// parsing block for non-nullable required parameters
else
{
codeWriter.WriteLine($$"""if (!{{endpointParameter.PreferredTryParseInvocation(temp_argument, output_argument)}})""");
codeWriter.StartBlock();
codeWriter.WriteLine($"if (!string.IsNullOrEmpty({temp_argument}))");
codeWriter.StartBlock();
EmitLogOrThrowException(endpointParameter, codeWriter, temp_argument);
codeWriter.EndBlock();
codeWriter.EndBlock();
}

codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {endpointParameter.EmitHandlerArgument()} = {endpointParameter.EmitParsedTempArgument()}!;");
}
else // Not parsable, not an array.
// Not parsable, not an array.
else
{
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {endpointParameter.EmitHandlerArgument()} = {endpointParameter.EmitTempArgument()}!;");
}

static void EmitLogOrThrowException(EndpointParameter parameter, CodeWriter writer, string inputArgument)
{
if (parameter.IsArray && parameter.ElementType.NullableAnnotation == NullableAnnotation.Annotated)
{
writer.WriteLine("wasParamCheckFailure = true;");
writer.WriteLine($@"logOrThrowExceptionHelper.RequiredParameterNotProvided({SymbolDisplay.FormatLiteral(parameter.Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), true)}, {SymbolDisplay.FormatLiteral(parameter.SymbolName, true)}, {SymbolDisplay.FormatLiteral(parameter.ToMessageString(), true)});");
}
else
{
writer.WriteLine($@"logOrThrowExceptionHelper.ParameterBindingFailed({SymbolDisplay.FormatLiteral(parameter.Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), true)}, {SymbolDisplay.FormatLiteral(parameter.SymbolName, true)}, {inputArgument});");
writer.WriteLine("wasParamCheckFailure = true;");
}
}
}

internal static void EmitRouteParameterPreparation(this EndpointParameter endpointParameter, CodeWriter codeWriter)
Expand Down
84 changes: 17 additions & 67 deletions src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Microsoft.AspNetCore.Analyzers.Infrastructure;
using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure;
using Microsoft.AspNetCore.App.Analyzers.Infrastructure;
using Microsoft.AspNetCore.Http.RequestDelegateGenerator.StaticRouteHandlerModel.Emitters;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using WellKnownType = Microsoft.AspNetCore.App.Analyzers.Infrastructure.WellKnownTypeData.WellKnownType;
Expand All @@ -19,7 +18,7 @@ namespace Microsoft.AspNetCore.Http.RequestDelegateGenerator.StaticRouteHandlerM

internal class EndpointParameter
{
public EndpointParameter(Endpoint endpoint, IParameterSymbol parameter, WellKnownTypes wellKnownTypes): this(endpoint, parameter.Type, parameter.Name, wellKnownTypes)
public EndpointParameter(Endpoint endpoint, IParameterSymbol parameter, WellKnownTypes wellKnownTypes) : this(endpoint, parameter.Type, parameter.Name, wellKnownTypes)
{
Ordinal = parameter.Ordinal;
IsOptional = parameter.IsOptional();
Expand Down Expand Up @@ -71,22 +70,22 @@ private void ProcessEndpointParameterSource(Endpoint endpoint, ISymbol symbol, I
{
Source = EndpointParameterSource.Route;
LookupName = GetEscapedParameterName(fromRouteAttribute, symbol.Name);
IsParsable = TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter);
ParsingBlockEmitter = parsingBlockEmitter;
IsParsable = TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation);
PreferredTryParseInvocation = preferredTryParseInvocation;
}
else if (attributes.TryGetAttributeImplementingInterface(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_Metadata_IFromQueryMetadata), out var fromQueryAttribute))
{
Source = EndpointParameterSource.Query;
LookupName = GetEscapedParameterName(fromQueryAttribute, symbol.Name);
IsParsable = TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter);
ParsingBlockEmitter = parsingBlockEmitter;
IsParsable = TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation);
PreferredTryParseInvocation = preferredTryParseInvocation;
}
else if (attributes.TryGetAttributeImplementingInterface(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_Metadata_IFromHeaderMetadata), out var fromHeaderAttribute))
{
Source = EndpointParameterSource.Header;
LookupName = GetEscapedParameterName(fromHeaderAttribute, symbol.Name);
IsParsable = TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter);
ParsingBlockEmitter = parsingBlockEmitter;
IsParsable = TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation);
PreferredTryParseInvocation = preferredTryParseInvocation;
}
else if (attributes.TryGetAttributeImplementingInterface(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_Metadata_IFromFormMetadata), out var fromFormAttribute))
{
Expand Down Expand Up @@ -121,8 +120,8 @@ private void ProcessEndpointParameterSource(Endpoint endpoint, ISymbol symbol, I
AssigningCode = !IsArray
? $"(string?)httpContext.Request.Form[{SymbolDisplay.FormatLiteral(LookupName, true)}]"
: $"httpContext.Request.Form[{SymbolDisplay.FormatLiteral(LookupName, true)}].ToArray()";
IsParsable = TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter);
ParsingBlockEmitter = parsingBlockEmitter;
IsParsable = TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation);
PreferredTryParseInvocation = preferredTryParseInvocation;
}
}
else if (TryGetExplicitFromJsonBody(symbol, attributes, wellKnownTypes, out var isOptional))
Expand Down Expand Up @@ -237,11 +236,11 @@ Type is not INamedTypeSymbol namedTypeSymbol ||
Source = EndpointParameterSource.Query;
IsStringValues = true;
}
else if (TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter))
else if (TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation))
{
Source = EndpointParameterSource.RouteOrQuery;
IsParsable = true;
ParsingBlockEmitter = parsingBlockEmitter;
PreferredTryParseInvocation = preferredTryParseInvocation;
}
else
{
Expand Down Expand Up @@ -293,9 +292,9 @@ private static bool ImplementsIEndpointParameterMetadataProvider(ITypeSymbol typ
// to be resolved by a specific WellKnownType
public string? AssigningCode { get; set; }

[MemberNotNullWhen(true, nameof(ParsingBlockEmitter))]
[MemberNotNullWhen(true, nameof(PreferredTryParseInvocation))]
public bool IsParsable { get; set; }
public Action<CodeWriter, string, string>? ParsingBlockEmitter { get; set; }
public Func<string, string, string>? PreferredTryParseInvocation { get; set; }
public bool IsStringValues { get; set; }

public BindabilityMethod? BindMethod { get; set; }
Expand All @@ -307,7 +306,7 @@ private static bool HasBindAsync(ITypeSymbol typeSymbol, WellKnownTypes wellKnow
return ParsabilityHelper.GetBindability(parameterType, wellKnownTypes, out bindMethod, out bindMethodSymbol) == Bindability.Bindable;
}

private static bool TryGetArrayElementType(ITypeSymbol type, [NotNullWhen(true)]out ITypeSymbol elementType)
private static bool TryGetArrayElementType(ITypeSymbol type, [NotNullWhen(true)] out ITypeSymbol elementType)
{
if (type.TypeKind == TypeKind.Array)
{
Expand All @@ -321,7 +320,7 @@ private static bool TryGetArrayElementType(ITypeSymbol type, [NotNullWhen(true)]
}
}

private bool TryGetParsability(ITypeSymbol typeSymbol, WellKnownTypes wellKnownTypes, [NotNullWhen(true)] out Action<CodeWriter, string, string>? parsingBlockEmitter)
private bool TryGetParsability(ITypeSymbol typeSymbol, WellKnownTypes wellKnownTypes, [NotNullWhen(true)] out Func<string, string, string>? preferredTryParseInvocation)
{
var parameterType = typeSymbol.UnwrapTypeSymbol(unwrapArray: true, unwrapNullable: true);

Expand All @@ -332,14 +331,14 @@ private bool TryGetParsability(ITypeSymbol typeSymbol, WellKnownTypes wellKnownT
// parsable at all we bail.
if (ParsabilityHelper.GetParsability(parameterType, wellKnownTypes, out var parsabilityMethod) != Parsability.Parsable)
{
parsingBlockEmitter = null;
preferredTryParseInvocation = null;
return false;
}

// If we are parsable we need to emit code based on the enumeration ParsabilityMethod which has a bunch of members
// which spell out the preferred TryParse usage. This switch statement makes slight variations to them based on
// which method was encountered.
Func<string, string, string>? preferredTryParseInvocation = parsabilityMethod switch
preferredTryParseInvocation = parsabilityMethod switch
{
ParsabilityMethod.IParsable => (string inputArgument, string outputArgument) => $$"""GeneratedRouteBuilderExtensionsCore.TryParseExplicit<{{parameterType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}}>({{inputArgument}}!, CultureInfo.InvariantCulture, out var {{outputArgument}})""",
ParsabilityMethod.TryParseWithFormatProvider => (string inputArgument, string outputArgument) => $$"""{{parameterType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}}.TryParse({{inputArgument}}!, CultureInfo.InvariantCulture, out var {{outputArgument}})""",
Expand Down Expand Up @@ -371,58 +370,9 @@ private bool TryGetParsability(ITypeSymbol typeSymbol, WellKnownTypes wellKnownT
// ... so for strings (null) we bail.
if (preferredTryParseInvocation == null)
{
parsingBlockEmitter = null;
return false;
}

if (IsOptional)
{
parsingBlockEmitter = (writer, inputArgument, outputArgument) =>
{
writer.WriteLine($"""{typeSymbol.ToDisplayString(EmitterConstants.DisplayFormat)} {outputArgument} = default;""");
writer.WriteLine($$"""if ({{preferredTryParseInvocation(inputArgument, $"{inputArgument}_parsed_non_nullable")}})""");
writer.StartBlock();
writer.WriteLine($$"""{{outputArgument}} = {{$"{inputArgument}_parsed_non_nullable"}};""");
writer.EndBlock();
writer.WriteLine($$"""else if (string.IsNullOrEmpty({{inputArgument}}))""");
writer.StartBlock();
writer.WriteLine($$"""{{outputArgument}} = {{DefaultValue}};""");
writer.EndBlock();
writer.WriteLine("else");
writer.StartBlock();
writer.WriteLine("wasParamCheckFailure = true;");
writer.EndBlock();
};
}
else
{
parsingBlockEmitter = (writer, inputArgument, outputArgument) =>
{
if (IsArray && ElementType.NullableAnnotation == NullableAnnotation.Annotated)
{
writer.WriteLine($$"""if (!{{preferredTryParseInvocation(inputArgument, outputArgument)}})""");
writer.StartBlock();
writer.WriteLine($$"""if (!string.IsNullOrEmpty({{inputArgument}}))""");
writer.StartBlock();
writer.WriteLine("wasParamCheckFailure = true;");
writer.WriteLine($@"logOrThrowExceptionHelper.RequiredParameterNotProvided({SymbolDisplay.FormatLiteral(Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), true)}, {SymbolDisplay.FormatLiteral(SymbolName, true)}, {SymbolDisplay.FormatLiteral(this.ToMessageString(), true)});");
writer.EndBlock();
writer.EndBlock();
}
else
{
writer.WriteLine($$"""if (!{{preferredTryParseInvocation(inputArgument, outputArgument)}})""");
writer.StartBlock();
writer.WriteLine($"if (!string.IsNullOrEmpty({inputArgument}))");
writer.StartBlock();
writer.WriteLine($@"logOrThrowExceptionHelper.ParameterBindingFailed({SymbolDisplay.FormatLiteral(Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), true)}, {SymbolDisplay.FormatLiteral(SymbolName, true)}, {inputArgument});");
writer.WriteLine("wasParamCheckFailure = true;");
writer.EndBlock();
writer.EndBlock();
}
};
}

// Wrap the TryParse method call in an if-block and if it doesn't work set param check failure.
return true;
}
Expand Down
Loading
Loading