Skip to content

Commit 833d4aa

Browse files
ithlinecaptainsafia
ithline
authored and
captainsafia
committed
Fix code generation for parsing optional array parameters in RequestDelegateGenerator (#57336)
* `EndpointParameter.ParsingBlockEmitter` property lifted out into `EndpointParameterEmitter.EmitParsingBlock` to be able to properly emit parsing logic for optional arrays. * add array tests and fixed emitter for nullable types * fix keyed service test in environments that use comma (`,`) as decimal point, because string interpolation made `12.3` as `12,3` failing the compilation. * add more tests for string optional array align array creation more closely with the reflection delegate factory * created baselines for optional array binding tests
1 parent 1c888e8 commit 833d4aa

17 files changed

+5758
-75
lines changed

‎src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointParameterEmitter.cs‎

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,26 @@ internal static void EmitFormParameterPreparation(this EndpointParameter endpoin
8989

9090
internal static void EmitParsingBlock(this EndpointParameter endpointParameter, CodeWriter codeWriter)
9191
{
92+
// parsable array
9293
if (endpointParameter.IsArray && endpointParameter.IsParsable)
9394
{
94-
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)} {endpointParameter.EmitHandlerArgument()} = new {endpointParameter.ElementType.ToDisplayString(EmitterConstants.DisplayFormat)}[{endpointParameter.EmitTempArgument()}.Length];");
95+
var createArray = $"new {endpointParameter.ElementType.ToDisplayString(EmitterConstants.DisplayFormat)}[{endpointParameter.EmitTempArgument()}.Length]";
96+
97+
// we assign a null to result parameter if it's optional array, otherwise we create new array immediately
98+
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)} {endpointParameter.EmitHandlerArgument()} = {createArray};");
99+
95100
codeWriter.WriteLine($"for (var i = 0; i < {endpointParameter.EmitTempArgument()}.Length; i++)");
96101
codeWriter.StartBlock();
97102
codeWriter.WriteLine($"var element = {endpointParameter.EmitTempArgument()}[i];");
98-
endpointParameter.ParsingBlockEmitter(codeWriter, "element", "parsed_element");
103+
104+
// emit parsing block for current array element
105+
codeWriter.WriteLine($$"""if (!{{endpointParameter.PreferredTryParseInvocation("element", "parsed_element")}})""");
106+
codeWriter.StartBlock();
107+
codeWriter.WriteLine("if (!string.IsNullOrEmpty(element))");
108+
codeWriter.StartBlock();
109+
EmitLogOrThrowException(endpointParameter, codeWriter, "element");
110+
codeWriter.EndBlock();
111+
codeWriter.EndBlock();
99112

100113
// In cases where we are dealing with an array of parsable nullables we need to substitute
101114
// empty strings for null values.
@@ -109,19 +122,69 @@ internal static void EmitParsingBlock(this EndpointParameter endpointParameter,
109122
}
110123
codeWriter.EndBlock();
111124
}
112-
else if (endpointParameter.IsArray && !endpointParameter.IsParsable)
125+
// array fallback
126+
else if (endpointParameter.IsArray)
113127
{
114128
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {endpointParameter.EmitHandlerArgument()} = {endpointParameter.EmitTempArgument()}!;");
115129
}
116-
else if (!endpointParameter.IsArray && endpointParameter.IsParsable)
130+
// parsable single
131+
else if (endpointParameter.IsParsable)
117132
{
118-
endpointParameter.ParsingBlockEmitter(codeWriter, endpointParameter.EmitTempArgument(), endpointParameter.EmitParsedTempArgument());
133+
var temp_argument = endpointParameter.EmitTempArgument();
134+
var output_argument = endpointParameter.EmitParsedTempArgument();
135+
136+
// emit parsing block for optional OR nullable values
137+
if (endpointParameter.IsOptional || endpointParameter.Type.NullableAnnotation == NullableAnnotation.Annotated)
138+
{
139+
var temp_argument_parsed_non_nullable = $"{temp_argument}_parsed_non_nullable";
140+
141+
codeWriter.WriteLine($"""{endpointParameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)} {output_argument} = default;""");
142+
codeWriter.WriteLine($"""if ({endpointParameter.PreferredTryParseInvocation(temp_argument, temp_argument_parsed_non_nullable)})""");
143+
codeWriter.StartBlock();
144+
codeWriter.WriteLine($"""{output_argument} = {temp_argument_parsed_non_nullable};""");
145+
codeWriter.EndBlock();
146+
codeWriter.WriteLine($"""else if (string.IsNullOrEmpty({temp_argument}))""");
147+
codeWriter.StartBlock();
148+
codeWriter.WriteLine($"""{output_argument} = {endpointParameter.DefaultValue};""");
149+
codeWriter.EndBlock();
150+
codeWriter.WriteLine("else");
151+
codeWriter.StartBlock();
152+
codeWriter.WriteLine("wasParamCheckFailure = true;");
153+
codeWriter.EndBlock();
154+
}
155+
// parsing block for non-nullable required parameters
156+
else
157+
{
158+
codeWriter.WriteLine($$"""if (!{{endpointParameter.PreferredTryParseInvocation(temp_argument, output_argument)}})""");
159+
codeWriter.StartBlock();
160+
codeWriter.WriteLine($"if (!string.IsNullOrEmpty({temp_argument}))");
161+
codeWriter.StartBlock();
162+
EmitLogOrThrowException(endpointParameter, codeWriter, temp_argument);
163+
codeWriter.EndBlock();
164+
codeWriter.EndBlock();
165+
}
166+
119167
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {endpointParameter.EmitHandlerArgument()} = {endpointParameter.EmitParsedTempArgument()}!;");
120168
}
121-
else // Not parsable, not an array.
169+
// Not parsable, not an array.
170+
else
122171
{
123172
codeWriter.WriteLine($"{endpointParameter.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {endpointParameter.EmitHandlerArgument()} = {endpointParameter.EmitTempArgument()}!;");
124173
}
174+
175+
static void EmitLogOrThrowException(EndpointParameter parameter, CodeWriter writer, string inputArgument)
176+
{
177+
if (parameter.IsArray && parameter.ElementType.NullableAnnotation == NullableAnnotation.Annotated)
178+
{
179+
writer.WriteLine("wasParamCheckFailure = true;");
180+
writer.WriteLine($@"logOrThrowExceptionHelper.RequiredParameterNotProvided({SymbolDisplay.FormatLiteral(parameter.Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), true)}, {SymbolDisplay.FormatLiteral(parameter.SymbolName, true)}, {SymbolDisplay.FormatLiteral(parameter.ToMessageString(), true)});");
181+
}
182+
else
183+
{
184+
writer.WriteLine($@"logOrThrowExceptionHelper.ParameterBindingFailed({SymbolDisplay.FormatLiteral(parameter.Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), true)}, {SymbolDisplay.FormatLiteral(parameter.SymbolName, true)}, {inputArgument});");
185+
writer.WriteLine("wasParamCheckFailure = true;");
186+
}
187+
}
125188
}
126189

127190
internal static void EmitRouteParameterPreparation(this EndpointParameter endpointParameter, CodeWriter codeWriter)

‎src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameter.cs‎

Lines changed: 17 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
using Microsoft.AspNetCore.Analyzers.Infrastructure;
1111
using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure;
1212
using Microsoft.AspNetCore.App.Analyzers.Infrastructure;
13-
using Microsoft.AspNetCore.Http.RequestDelegateGenerator.StaticRouteHandlerModel.Emitters;
1413
using Microsoft.CodeAnalysis;
1514
using Microsoft.CodeAnalysis.CSharp;
1615
using WellKnownType = Microsoft.AspNetCore.App.Analyzers.Infrastructure.WellKnownTypeData.WellKnownType;
@@ -19,7 +18,7 @@ namespace Microsoft.AspNetCore.Http.RequestDelegateGenerator.StaticRouteHandlerM
1918

2019
internal class EndpointParameter
2120
{
22-
public EndpointParameter(Endpoint endpoint, IParameterSymbol parameter, WellKnownTypes wellKnownTypes): this(endpoint, parameter.Type, parameter.Name, wellKnownTypes)
21+
public EndpointParameter(Endpoint endpoint, IParameterSymbol parameter, WellKnownTypes wellKnownTypes) : this(endpoint, parameter.Type, parameter.Name, wellKnownTypes)
2322
{
2423
Ordinal = parameter.Ordinal;
2524
IsOptional = parameter.IsOptional();
@@ -71,22 +70,22 @@ private void ProcessEndpointParameterSource(Endpoint endpoint, ISymbol symbol, I
7170
{
7271
Source = EndpointParameterSource.Route;
7372
LookupName = GetEscapedParameterName(fromRouteAttribute, symbol.Name);
74-
IsParsable = TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter);
75-
ParsingBlockEmitter = parsingBlockEmitter;
73+
IsParsable = TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation);
74+
PreferredTryParseInvocation = preferredTryParseInvocation;
7675
}
7776
else if (attributes.TryGetAttributeImplementingInterface(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_Metadata_IFromQueryMetadata), out var fromQueryAttribute))
7877
{
7978
Source = EndpointParameterSource.Query;
8079
LookupName = GetEscapedParameterName(fromQueryAttribute, symbol.Name);
81-
IsParsable = TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter);
82-
ParsingBlockEmitter = parsingBlockEmitter;
80+
IsParsable = TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation);
81+
PreferredTryParseInvocation = preferredTryParseInvocation;
8382
}
8483
else if (attributes.TryGetAttributeImplementingInterface(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_Metadata_IFromHeaderMetadata), out var fromHeaderAttribute))
8584
{
8685
Source = EndpointParameterSource.Header;
8786
LookupName = GetEscapedParameterName(fromHeaderAttribute, symbol.Name);
88-
IsParsable = TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter);
89-
ParsingBlockEmitter = parsingBlockEmitter;
87+
IsParsable = TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation);
88+
PreferredTryParseInvocation = preferredTryParseInvocation;
9089
}
9190
else if (attributes.TryGetAttributeImplementingInterface(wellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_Metadata_IFromFormMetadata), out var fromFormAttribute))
9291
{
@@ -121,8 +120,8 @@ private void ProcessEndpointParameterSource(Endpoint endpoint, ISymbol symbol, I
121120
AssigningCode = !IsArray
122121
? $"(string?)httpContext.Request.Form[{SymbolDisplay.FormatLiteral(LookupName, true)}]"
123122
: $"httpContext.Request.Form[{SymbolDisplay.FormatLiteral(LookupName, true)}].ToArray()";
124-
IsParsable = TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter);
125-
ParsingBlockEmitter = parsingBlockEmitter;
123+
IsParsable = TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation);
124+
PreferredTryParseInvocation = preferredTryParseInvocation;
126125
}
127126
}
128127
else if (TryGetExplicitFromJsonBody(symbol, attributes, wellKnownTypes, out var isOptional))
@@ -237,11 +236,11 @@ Type is not INamedTypeSymbol namedTypeSymbol ||
237236
Source = EndpointParameterSource.Query;
238237
IsStringValues = true;
239238
}
240-
else if (TryGetParsability(Type, wellKnownTypes, out var parsingBlockEmitter))
239+
else if (TryGetParsability(Type, wellKnownTypes, out var preferredTryParseInvocation))
241240
{
242241
Source = EndpointParameterSource.RouteOrQuery;
243242
IsParsable = true;
244-
ParsingBlockEmitter = parsingBlockEmitter;
243+
PreferredTryParseInvocation = preferredTryParseInvocation;
245244
}
246245
else
247246
{
@@ -293,9 +292,9 @@ private static bool ImplementsIEndpointParameterMetadataProvider(ITypeSymbol typ
293292
// to be resolved by a specific WellKnownType
294293
public string? AssigningCode { get; set; }
295294

296-
[MemberNotNullWhen(true, nameof(ParsingBlockEmitter))]
295+
[MemberNotNullWhen(true, nameof(PreferredTryParseInvocation))]
297296
public bool IsParsable { get; set; }
298-
public Action<CodeWriter, string, string>? ParsingBlockEmitter { get; set; }
297+
public Func<string, string, string>? PreferredTryParseInvocation { get; set; }
299298
public bool IsStringValues { get; set; }
300299

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

310-
private static bool TryGetArrayElementType(ITypeSymbol type, [NotNullWhen(true)]out ITypeSymbol elementType)
309+
private static bool TryGetArrayElementType(ITypeSymbol type, [NotNullWhen(true)] out ITypeSymbol elementType)
311310
{
312311
if (type.TypeKind == TypeKind.Array)
313312
{
@@ -321,7 +320,7 @@ private static bool TryGetArrayElementType(ITypeSymbol type, [NotNullWhen(true)]
321320
}
322321
}
323322

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

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

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

378-
if (IsOptional)
379-
{
380-
parsingBlockEmitter = (writer, inputArgument, outputArgument) =>
381-
{
382-
writer.WriteLine($"""{typeSymbol.ToDisplayString(EmitterConstants.DisplayFormat)} {outputArgument} = default;""");
383-
writer.WriteLine($$"""if ({{preferredTryParseInvocation(inputArgument, $"{inputArgument}_parsed_non_nullable")}})""");
384-
writer.StartBlock();
385-
writer.WriteLine($$"""{{outputArgument}} = {{$"{inputArgument}_parsed_non_nullable"}};""");
386-
writer.EndBlock();
387-
writer.WriteLine($$"""else if (string.IsNullOrEmpty({{inputArgument}}))""");
388-
writer.StartBlock();
389-
writer.WriteLine($$"""{{outputArgument}} = {{DefaultValue}};""");
390-
writer.EndBlock();
391-
writer.WriteLine("else");
392-
writer.StartBlock();
393-
writer.WriteLine("wasParamCheckFailure = true;");
394-
writer.EndBlock();
395-
};
396-
}
397-
else
398-
{
399-
parsingBlockEmitter = (writer, inputArgument, outputArgument) =>
400-
{
401-
if (IsArray && ElementType.NullableAnnotation == NullableAnnotation.Annotated)
402-
{
403-
writer.WriteLine($$"""if (!{{preferredTryParseInvocation(inputArgument, outputArgument)}})""");
404-
writer.StartBlock();
405-
writer.WriteLine($$"""if (!string.IsNullOrEmpty({{inputArgument}}))""");
406-
writer.StartBlock();
407-
writer.WriteLine("wasParamCheckFailure = true;");
408-
writer.WriteLine($@"logOrThrowExceptionHelper.RequiredParameterNotProvided({SymbolDisplay.FormatLiteral(Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), true)}, {SymbolDisplay.FormatLiteral(SymbolName, true)}, {SymbolDisplay.FormatLiteral(this.ToMessageString(), true)});");
409-
writer.EndBlock();
410-
writer.EndBlock();
411-
}
412-
else
413-
{
414-
writer.WriteLine($$"""if (!{{preferredTryParseInvocation(inputArgument, outputArgument)}})""");
415-
writer.StartBlock();
416-
writer.WriteLine($"if (!string.IsNullOrEmpty({inputArgument}))");
417-
writer.StartBlock();
418-
writer.WriteLine($@"logOrThrowExceptionHelper.ParameterBindingFailed({SymbolDisplay.FormatLiteral(Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), true)}, {SymbolDisplay.FormatLiteral(SymbolName, true)}, {inputArgument});");
419-
writer.WriteLine("wasParamCheckFailure = true;");
420-
writer.EndBlock();
421-
writer.EndBlock();
422-
}
423-
};
424-
}
425-
426376
// Wrap the TryParse method call in an if-block and if it doesn't work set param check failure.
427377
return true;
428378
}

0 commit comments

Comments
 (0)