Skip to content

Commit 9b3e79d

Browse files
captainsafiacaptainsafia
authored
Enable nullability in RDG and fix warnings (#47144)
* Enable nullability in RDG and fix warnings * Fix nullability for symbol resolution and semantic model * Fix nullability warnings on build * Only process handlers that take a Delegate * Fix nullability warnings redux * Add doc string on WarnOnNullable property
1 parent 0793770 commit 9b3e79d

13 files changed

+146
-408
lines changed

‎eng/targets/CSharp.Common.targets‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@
122122
(('$(TargetFrameworkIdentifier)' == '.NETStandard' AND $([MSBuild]::VersionLessThanOrEquals('$(TargetFrameworkVersion)', '2.1'))) OR '$(TargetFrameworkIdentifier)' == '.NETFramework')">
123123
<PropertyGroup>
124124
<DefineConstants>$(DefineConstants),INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
125-
<NoWarn>$(NoWarn);nullable</NoWarn>
125+
<!-- Repo-specific property to enable nullability warnings for ns2.0 -->
126+
<NoWarn Condition=" '$(WarnOnNullable)' != 'true' ">$(NoWarn);nullable</NoWarn>
126127
</PropertyGroup>
127128
<ItemGroup>
128129
<Compile Include="$(SharedSourceRoot)Nullable\NullableAttributes.cs" />

‎src/Http/Http.Extensions/gen/DiagnosticDescriptors.cs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,19 @@ internal static class DiagnosticDescriptors
2727

2828
// This is temporary. The plan is to be able to resolve all parameters to a known EndpointParameterSource.
2929
// For now, we emit a warning for the unsupported set.
30-
public static DiagnosticDescriptor UnableToResolveParameterDescriptor { get; } = new(
31-
"RDG003",
32-
new LocalizableResourceString(nameof(Resources.UnableToResolveParameter_Title), Resources.ResourceManager, typeof(Resources)),
33-
new LocalizableResourceString(nameof(Resources.UnableToResolveParameter_Message), Resources.ResourceManager, typeof(Resources)),
34-
"Usage",
35-
DiagnosticSeverity.Warning,
36-
isEnabledByDefault: true);
30+
public static DiagnosticDescriptor UnableToResolveParameterDescriptor { get; } = new(
31+
"RDG003",
32+
new LocalizableResourceString(nameof(Resources.UnableToResolveParameter_Title), Resources.ResourceManager, typeof(Resources)),
33+
new LocalizableResourceString(nameof(Resources.UnableToResolveParameter_Message), Resources.ResourceManager, typeof(Resources)),
34+
"Usage",
35+
DiagnosticSeverity.Warning,
36+
isEnabledByDefault: true);
37+
38+
public static DiagnosticDescriptor UnableToResolveAnonymousReturnType { get; } = new(
39+
"RDG004",
40+
new LocalizableResourceString(nameof(Resources.UnableToResolveAnonymousReturnType_Title), Resources.ResourceManager, typeof(Resources)),
41+
new LocalizableResourceString(nameof(Resources.UnableToResolveAnonymousReturnType_Message), Resources.ResourceManager, typeof(Resources)),
42+
"Usage",
43+
DiagnosticSeverity.Warning,
44+
isEnabledByDefault: true);
3745
}

‎src/Http/Http.Extensions/gen/Microsoft.AspNetCore.Http.Generators.csproj‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
<IsPackable>false</IsPackable>
77
<IsAnalyzersProject>true</IsAnalyzersProject>
88
<AddPublicApiAnalyzers>false</AddPublicApiAnalyzers>
9+
<Nullable>enable</Nullable>
10+
<WarnOnNullable>true</WarnOnNullable>
911
</PropertyGroup>
1012

1113
<ItemGroup>

‎src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs‎

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public sealed class RequestDelegateGenerator : IIncrementalGenerator
3030
public void Initialize(IncrementalGeneratorInitializationContext context)
3131
{
3232
var endpointsWithDiagnostics = context.SyntaxProvider.CreateSyntaxProvider(
33-
predicate: (node, _) => node is InvocationExpressionSyntax
33+
predicate: static (node, _) => node is InvocationExpressionSyntax
3434
{
3535
Expression: MemberAccessExpressionSyntax
3636
{
@@ -41,62 +41,63 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
4141
},
4242
ArgumentList: { Arguments: { Count: 2 } args }
4343
} && _knownMethods.Contains(method),
44-
transform: (context, token) =>
44+
transform: static (context, token) =>
4545
{
46-
var operation = context.SemanticModel.GetOperation(context.Node, token) as IInvocationOperation;
46+
var operation = context.SemanticModel.GetOperation(context.Node, token);
4747
var wellKnownTypes = WellKnownTypes.GetOrCreate(context.SemanticModel.Compilation);
48-
return new Endpoint(operation, wellKnownTypes);
48+
if (operation is IInvocationOperation { Arguments: { Length: 3 } parameters } invocationOperation &&
49+
invocationOperation.GetRouteHandlerArgument() is { Parameter.Type: {} delegateType } &&
50+
SymbolEqualityComparer.Default.Equals(delegateType, wellKnownTypes.Get(WellKnownTypeData.WellKnownType.System_Delegate)))
51+
{
52+
return new Endpoint(invocationOperation, wellKnownTypes, context.SemanticModel);
53+
}
54+
return null;
4955
})
56+
.Where(static endpoint => endpoint != null)
5057
.WithTrackingName(GeneratorSteps.EndpointModelStep);
5158

5259
context.RegisterSourceOutput(endpointsWithDiagnostics, (context, endpoint) =>
5360
{
54-
var (filePath, _) = endpoint.Location;
55-
foreach (var diagnostic in endpoint.Diagnostics)
61+
foreach (var diagnostic in endpoint!.Diagnostics)
5662
{
5763
context.ReportDiagnostic(diagnostic);
5864
}
5965
});
6066

6167
var endpoints = endpointsWithDiagnostics
62-
.Where(endpoint => endpoint.Diagnostics.Count == 0)
68+
.Where(endpoint => endpoint!.Diagnostics.Count == 0)
6369
.WithTrackingName(GeneratorSteps.EndpointsWithoutDiagnosicsStep);
6470

6571
var thunks = endpoints.Select((endpoint, _) =>
6672
{
6773
using var stringWriter = new StringWriter(CultureInfo.InvariantCulture);
6874
using var codeWriter = new CodeWriter(stringWriter, baseIndent: 3);
6975
codeWriter.InitializeIndent();
70-
codeWriter.WriteLine($"[{endpoint.EmitSourceKey()}] = (");
76+
codeWriter.WriteLine($"[{endpoint!.EmitSourceKey()}] = (");
7177
codeWriter.Indent++;
7278
codeWriter.WriteLine("(methodInfo, options) =>");
7379
codeWriter.StartBlock();
7480
codeWriter.WriteLine(@"Debug.Assert(options?.EndpointBuilder != null, ""EndpointBuilder not found."");");
75-
codeWriter.WriteLine($"options.EndpointBuilder.Metadata.Add(new SourceKey{endpoint.EmitSourceKey()});");
81+
codeWriter.WriteLine($"options.EndpointBuilder.Metadata.Add(new SourceKey{endpoint!.EmitSourceKey()});");
7682
codeWriter.WriteLine("return new RequestDelegateMetadataResult { EndpointMetadata = options.EndpointBuilder.Metadata.AsReadOnly() };");
7783
codeWriter.EndBlockWithComma();
7884
codeWriter.WriteLine("(del, options, inferredMetadataResult) =>");
7985
codeWriter.StartBlock();
80-
codeWriter.WriteLine($"var handler = ({endpoint.EmitHandlerDelegateCast()})del;");
86+
codeWriter.WriteLine($"var handler = ({endpoint!.EmitHandlerDelegateCast()})del;");
8187
codeWriter.WriteLine("EndpointFilterDelegate? filteredInvocation = null;");
82-
endpoint.EmitRouteOrQueryResolver(codeWriter);
83-
endpoint.EmitJsonBodyOrServicePreparation(codeWriter);
84-
endpoint.EmitJsonPreparation(codeWriter);
88+
endpoint!.EmitRouteOrQueryResolver(codeWriter);
89+
endpoint!.EmitJsonBodyOrServicePreparation(codeWriter);
90+
endpoint!.Response?.EmitJsonPreparation(codeWriter);
8591
if (endpoint.NeedsParameterArray)
8692
{
8793
codeWriter.WriteLine("var parameters = del.Method.GetParameters();");
8894
}
8995
codeWriter.WriteLineNoTabs(string.Empty);
9096
codeWriter.WriteLine("if (options?.EndpointBuilder?.FilterFactories.Count > 0)");
9197
codeWriter.StartBlock();
92-
if (endpoint.Response.IsAwaitable)
93-
{
94-
codeWriter.WriteLine("filteredInvocation = GeneratedRouteBuilderExtensionsCore.BuildFilterDelegate(async ic =>");
95-
}
96-
else
97-
{
98-
codeWriter.WriteLine("filteredInvocation = GeneratedRouteBuilderExtensionsCore.BuildFilterDelegate(ic =>");
99-
}
98+
codeWriter.WriteLine(endpoint!.Response?.IsAwaitable == true
99+
? "filteredInvocation = GeneratedRouteBuilderExtensionsCore.BuildFilterDelegate(async ic =>"
100+
: "filteredInvocation = GeneratedRouteBuilderExtensionsCore.BuildFilterDelegate(ic =>");
100101
codeWriter.StartBlock();
101102
codeWriter.WriteLine("if (ic.HttpContext.Response.StatusCode == 400)");
102103
codeWriter.StartBlock();
@@ -124,16 +125,16 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
124125
.Collect()
125126
.Select((endpoints, _) =>
126127
{
127-
var dedupedByDelegate = endpoints.Distinct(EndpointDelegateComparer.Instance);
128+
var dedupedByDelegate = endpoints.Distinct<Endpoint>(EndpointDelegateComparer.Instance);
128129
using var stringWriter = new StringWriter(CultureInfo.InvariantCulture);
129130
using var codeWriter = new CodeWriter(stringWriter, baseIndent: 2);
130131
foreach (var endpoint in dedupedByDelegate)
131132
{
132-
codeWriter.WriteLine($"internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder {endpoint.HttpMethod}(");
133+
codeWriter.WriteLine($"internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder {endpoint!.HttpMethod}(");
133134
codeWriter.Indent++;
134135
codeWriter.WriteLine("this global::Microsoft.AspNetCore.Routing.IEndpointRouteBuilder endpoints,");
135136
codeWriter.WriteLine(@"[global::System.Diagnostics.CodeAnalysis.StringSyntax(""Route"")] string pattern,");
136-
codeWriter.WriteLine($"global::{endpoint.EmitHandlerDelegateType()} handler,");
137+
codeWriter.WriteLine($"global::{endpoint!.EmitHandlerDelegateType()} handler,");
137138
codeWriter.WriteLine(@"[global::System.Runtime.CompilerServices.CallerFilePath] string filePath = """",");
138139
codeWriter.WriteLine("[global::System.Runtime.CompilerServices.CallerLineNumber]int lineNumber = 0)");
139140
codeWriter.Indent--;
@@ -143,7 +144,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
143144
codeWriter.WriteLine("endpoints,");
144145
codeWriter.WriteLine("pattern,");
145146
codeWriter.WriteLine("handler,");
146-
codeWriter.WriteLine($"{endpoint.EmitVerb()},");
147+
codeWriter.WriteLine($"{endpoint!.EmitVerb()},");
147148
codeWriter.WriteLine("filePath,");
148149
codeWriter.WriteLine("lineNumber);");
149150
codeWriter.Indent--;
@@ -157,12 +158,12 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
157158
.Collect()
158159
.Select((endpoints, _) =>
159160
{
160-
var hasJsonBodyOrService = endpoints.Any(endpoint => endpoint.EmitterContext.HasJsonBodyOrService);
161-
var hasJsonBody = endpoints.Any(endpoint => endpoint.EmitterContext.HasJsonBody);
162-
var hasRouteOrQuery = endpoints.Any(endpoint => endpoint.EmitterContext.HasRouteOrQuery);
163-
var hasBindAsync = endpoints.Any(endpoint => endpoint.EmitterContext.HasBindAsync);
164-
var hasParsable = endpoints.Any(endpoint => endpoint.EmitterContext.HasParsable);
165-
var hasJsonResponse = endpoints.Any(endpoint => endpoint.EmitterContext.HasJsonResponse);
161+
var hasJsonBodyOrService = endpoints.Any(endpoint => endpoint!.EmitterContext.HasJsonBodyOrService);
162+
var hasJsonBody = endpoints.Any(endpoint => endpoint!.EmitterContext.HasJsonBody);
163+
var hasRouteOrQuery = endpoints.Any(endpoint => endpoint!.EmitterContext.HasRouteOrQuery);
164+
var hasBindAsync = endpoints.Any(endpoint => endpoint!.EmitterContext.HasBindAsync);
165+
var hasParsable = endpoints.Any(endpoint => endpoint!.EmitterContext.HasParsable);
166+
var hasJsonResponse = endpoints.Any(endpoint => endpoint!.EmitterContext.HasJsonResponse);
166167

167168
using var stringWriter = new StringWriter(CultureInfo.InvariantCulture);
168169
using var codeWriter = new CodeWriter(stringWriter, baseIndent: 0);

‎src/Http/Http.Extensions/gen/Resources.resx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,4 +135,10 @@
135135
<data name="UnableToResolveParameter_Title" xml:space="preserve">
136136
<value>Unable to resolve parameter</value>
137137
</data>
138+
<data name="UnableToResolveAnonymousReturnType_Message" xml:space="preserve">
139+
<value>Unable to resolve anonymous return type. Compile-time endpoint generation will skip this endpoint and the endpoint will be generated at runtime.</value>
140+
</data>
141+
<data name="UnableToResolveAnonymousReturnType_Title" xml:space="preserve">
142+
<value>Unable to resolve anonymous type</value>
143+
</data>
138144
</root>

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,25 @@ namespace Microsoft.AspNetCore.Http.Generators.StaticRouteHandlerModel.Emitters;
55

66
internal static class EndpointJsonResponseEmitter
77
{
8-
internal static void EmitJsonPreparation(this Endpoint endpoint, CodeWriter codeWriter)
8+
internal static void EmitJsonPreparation(this EndpointResponse endpointResponse, CodeWriter codeWriter)
99
{
10-
if (endpoint.Response.IsSerializable)
10+
if (endpointResponse is { IsSerializable: true, ResponseType: {} responseType })
1111
{
12-
var typeName = endpoint.Response.ResponseType.ToDisplayString(EmitterConstants.DisplayFormat);
12+
var typeName = responseType.ToDisplayString(EmitterConstants.DisplayFormat);
1313

1414
codeWriter.WriteLine("var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices;");
1515
codeWriter.WriteLine("var serializerOptions = serviceProvider?.GetService<IOptions<JsonOptions>>()?.Value.SerializerOptions ?? new JsonOptions().SerializerOptions;");
1616
codeWriter.WriteLine($"var jsonTypeInfo = (JsonTypeInfo<{typeName}>)serializerOptions.GetTypeInfo(typeof({typeName}));");
1717
}
1818
}
1919

20-
internal static string EmitJsonResponse(this Endpoint endpoint)
20+
internal static string EmitJsonResponse(this EndpointResponse endpointResponse)
2121
{
22-
if (endpoint.Response.ResponseType.IsSealed || endpoint.Response.ResponseType.IsValueType)
22+
if (endpointResponse.ResponseType != null &&
23+
(endpointResponse.ResponseType.IsSealed || endpointResponse.ResponseType.IsValueType))
2324
{
2425
return $"httpContext.Response.WriteAsJsonAsync(result, jsonTypeInfo);";
2526
}
26-
else
27-
{
28-
return $"GeneratedRouteBuilderExtensionsCore.WriteToResponseAsync(result, httpContext, jsonTypeInfo, serializerOptions);";
29-
}
27+
return $"GeneratedRouteBuilderExtensionsCore.WriteToResponseAsync(result, httpContext, jsonTypeInfo, serializerOptions);";
3028
}
3129
}

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Http.Generators.StaticRouteHandlerModel;
1515

1616
internal class Endpoint
1717
{
18-
public Endpoint(IInvocationOperation operation, WellKnownTypes wellKnownTypes)
18+
public Endpoint(IInvocationOperation operation, WellKnownTypes wellKnownTypes, SemanticModel semanticModel)
1919
{
2020
Operation = operation;
2121
Location = GetLocation(operation);
@@ -30,15 +30,21 @@ public Endpoint(IInvocationOperation operation, WellKnownTypes wellKnownTypes)
3030

3131
RoutePattern = routeToken.ValueText;
3232

33-
if (!operation.TryGetRouteHandlerMethod(out var method))
33+
if (!operation.TryGetRouteHandlerMethod(semanticModel, out var method) || method == null)
3434
{
3535
Diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.UnableToResolveMethod, Operation.Syntax.GetLocation()));
3636
return;
3737
}
3838

3939
Response = new EndpointResponse(method, wellKnownTypes);
40-
EmitterContext.HasJsonResponse = !(Response.ResponseType.IsSealed || Response.ResponseType.IsValueType);
41-
IsAwaitable = Response.IsAwaitable;
40+
if (Response.IsAnonymousType)
41+
{
42+
Diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.UnableToResolveAnonymousReturnType, Operation.Syntax.GetLocation()));
43+
return;
44+
}
45+
46+
EmitterContext.HasJsonResponse = Response is not { ResponseType: { IsSealed: true } or { IsValueType: true } };
47+
IsAwaitable = Response?.IsAwaitable == true;
4248

4349
if (method.Parameters.Length == 0)
4450
{
@@ -107,7 +113,7 @@ public override int GetHashCode() =>
107113

108114
public static bool SignatureEquals(Endpoint a, Endpoint b)
109115
{
110-
if (!a.Response.WrappedResponseType.Equals(b.Response.WrappedResponseType, StringComparison.Ordinal) ||
116+
if (!string.Equals(a.Response?.WrappedResponseType, b.Response?.WrappedResponseType, StringComparison.Ordinal) ||
111117
!a.HttpMethod.Equals(b.HttpMethod, StringComparison.Ordinal) ||
112118
a.Parameters.Length != b.Parameters.Length)
113119
{
@@ -128,7 +134,7 @@ public static bool SignatureEquals(Endpoint a, Endpoint b)
128134
public static int GetSignatureHashCode(Endpoint endpoint)
129135
{
130136
var hashCode = new HashCode();
131-
hashCode.Add(endpoint.Response.WrappedResponseType);
137+
hashCode.Add(endpoint.Response?.WrappedResponseType);
132138
hashCode.Add(endpoint.HttpMethod);
133139

134140
foreach (var parameter in endpoint.Parameters)

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@ internal class EndpointResponse
1515
{
1616
public ITypeSymbol? ResponseType { get; set; }
1717
public string WrappedResponseType { get; set; }
18-
public string ContentType { get; set; }
18+
public string? ContentType { get; set; }
1919
public bool IsAwaitable { get; set; }
2020
public bool IsVoid { get; set; }
2121
public bool IsIResult { get; set; }
2222
public bool IsSerializable { get; set; }
23+
public bool IsAnonymousType { get; set; }
2324

2425
private WellKnownTypes WellKnownTypes { get; init; }
2526

@@ -31,11 +32,12 @@ internal EndpointResponse(IMethodSymbol method, WellKnownTypes wellKnownTypes)
3132
IsAwaitable = GetIsAwaitable(method);
3233
IsVoid = method.ReturnsVoid;
3334
IsIResult = GetIsIResult();
34-
IsSerializable = !IsIResult && !IsVoid && ResponseType.SpecialType != SpecialType.System_String && ResponseType.SpecialType != SpecialType.System_Object;
35+
IsSerializable = GetIsSerializable();
3536
ContentType = GetContentType(method);
37+
IsAnonymousType = method.ReturnType.IsAnonymousType;
3638
}
3739

38-
private ITypeSymbol UnwrapResponseType(IMethodSymbol method)
40+
private ITypeSymbol? UnwrapResponseType(IMethodSymbol method)
3941
{
4042
var returnType = method.ReturnType;
4143
var task = WellKnownTypes.Get(WellKnownType.System_Threading_Tasks_Task);
@@ -57,6 +59,13 @@ private ITypeSymbol UnwrapResponseType(IMethodSymbol method)
5759
return returnType;
5860
}
5961

62+
private bool GetIsSerializable() =>
63+
!IsIResult &&
64+
!IsVoid &&
65+
ResponseType != null &&
66+
ResponseType.SpecialType != SpecialType.System_String &&
67+
ResponseType.SpecialType != SpecialType.System_Object;
68+
6069
private bool GetIsIResult()
6170
{
6271
var resultType = WellKnownTypes.Get(WellKnownType.Microsoft_AspNetCore_Http_IResult);
@@ -120,7 +129,7 @@ public override bool Equals(object obj)
120129
otherEndpointResponse.IsAwaitable == IsAwaitable &&
121130
otherEndpointResponse.IsVoid == IsVoid &&
122131
otherEndpointResponse.IsIResult == IsIResult &&
123-
otherEndpointResponse.ContentType.Equals(ContentType, StringComparison.OrdinalIgnoreCase);
132+
string.Equals(otherEndpointResponse.ContentType, ContentType, StringComparison.OrdinalIgnoreCase);
124133
}
125134

126135
public override int GetHashCode() =>

0 commit comments

Comments
 (0)