Add nullable annotations to M.A.Hosting and DefaultBuilder #24571
Conversation
* Turn on nullability annotations for public types in M.A.Hosting * Turn on nullability annotations for Microsoft.AspNetCore and Microsoft.AspNetCore.Server.Abstractions Contributes to #5680
ghost
added
the
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| #nullable enable |
There was a problem hiding this comment.
Since we no longer have the ref project, we can turn on nullability on individual files. Convenient!
| /// <returns>The <see cref="IHostBuilder"/> for chaining.</returns> | ||
| public static IHostBuilder ConfigureWebHostDefaults(this IHostBuilder builder, Action<IWebHostBuilder> configure) | ||
| { | ||
| if (configure is null) |
There was a problem hiding this comment.
Should I bother with this? This would have null-ref (so would have using a null IHostBuilder).
| } | ||
|
|
||
| private IServiceCollection BuildCommonServices(out AggregateException hostingStartupErrors) | ||
| [MemberNotNull(nameof(_options))] |
There was a problem hiding this comment.
It says that the field is non-null at the end of this method call.
| { | ||
| Console.WriteLine($"Hosting environment: {hostingEnvironment.EnvironmentName}"); | ||
| Console.WriteLine($"Content root path: {hostingEnvironment.ContentRootPath}"); | ||
| Console.WriteLine($"Hosting environment: {hostingEnvironment?.EnvironmentName}"); |
There was a problem hiding this comment.
RunAsync does a GetService:
var hostingEnvironment = host.Services.GetService<IHostEnvironment>();
|
Hello @pranavkm! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 60 minutes, a condition that will be fulfilled in about 8 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
| /// <param name="app">A delegate that handles requests to the application.</param> | ||
| /// <returns>A started <see cref="IWebHost"/> that hosts the application.</returns> | ||
| public static IWebHost Start(string url, RequestDelegate app) | ||
| public static IWebHost Start(string? url, RequestDelegate app) |
There was a problem hiding this comment.
These shouldn't have been made nullable. I know you did that for chaining, but it would be better to override the warning internally.
| /// <param name="args">The command line args.</param> | ||
| /// <returns>The initialized <see cref="IWebHostBuilder"/>.</returns> | ||
| public static IWebHostBuilder CreateDefaultBuilder(string[] args) | ||
| public static IWebHostBuilder CreateDefaultBuilder(string[]? args) |
There was a problem hiding this comment.
Also only nullable for chaining. Suppress the warning in the caller instead.
| /// The name of the authentication scheme for the server authentication handler. | ||
| /// </summary> | ||
| string AuthenticationScheme { get; } | ||
| string? AuthenticationScheme { get; } |
There was a problem hiding this comment.
This should not be null. Where did you see otherwise?
Microsoft.AspNetCore.Server.Abstractions
Contributes to #5680