-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Add nullable annotations to M.A.Hosting and DefaultBuilder #24571
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,9 +40,9 @@ public static IWebHost Start(RequestDelegate app) => | |
| /// <param name="url">The URL the hosted application will listen on.</param> | ||
| /// <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) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These shouldn't have been made nullable. I know you did that for chaining, but it would be better to override the warning internally. |
||
| { | ||
| var startupAssemblyName = app.GetMethodInfo().DeclaringType.GetTypeInfo().Assembly.GetName().Name; | ||
| var startupAssemblyName = app.GetMethodInfo().DeclaringType!.GetTypeInfo().Assembly.GetName().Name; | ||
| return StartWith(url: url, configureServices: null, app: appBuilder => appBuilder.Run(app), applicationName: startupAssemblyName); | ||
| } | ||
|
|
||
|
|
@@ -62,9 +62,9 @@ public static IWebHost Start(Action<IRouteBuilder> routeBuilder) => | |
| /// <param name="url">The URL the hosted application will listen on.</param> | ||
| /// <param name="routeBuilder">A delegate that configures the router for handling requests to the application.</param> | ||
| /// <returns>A started <see cref="IWebHost"/> that hosts the application.</returns> | ||
| public static IWebHost Start(string url, Action<IRouteBuilder> routeBuilder) | ||
| public static IWebHost Start(string? url, Action<IRouteBuilder> routeBuilder) | ||
| { | ||
| var startupAssemblyName = routeBuilder.GetMethodInfo().DeclaringType.GetTypeInfo().Assembly.GetName().Name; | ||
| var startupAssemblyName = routeBuilder.GetMethodInfo().DeclaringType!.GetTypeInfo().Assembly.GetName().Name; | ||
| return StartWith(url, services => services.AddRouting(), appBuilder => appBuilder.UseRouter(routeBuilder), applicationName: startupAssemblyName); | ||
| } | ||
|
|
||
|
|
@@ -84,10 +84,10 @@ public static IWebHost StartWith(Action<IApplicationBuilder> app) => | |
| /// <param name="url">The URL the hosted application will listen on.</param> | ||
| /// <param name="app">The delegate that configures the <see cref="IApplicationBuilder"/>.</param> | ||
| /// <returns>A started <see cref="IWebHost"/> that hosts the application.</returns> | ||
| public static IWebHost StartWith(string url, Action<IApplicationBuilder> app) => | ||
| public static IWebHost StartWith(string? url, Action<IApplicationBuilder> app) => | ||
| StartWith(url: url, configureServices: null, app: app, applicationName: null); | ||
|
|
||
| private static IWebHost StartWith(string url, Action<IServiceCollection> configureServices, Action<IApplicationBuilder> app, string applicationName) | ||
| private static IWebHost StartWith(string? url, Action<IServiceCollection>? configureServices, Action<IApplicationBuilder> app, string? applicationName) | ||
| { | ||
| var builder = CreateDefaultBuilder(); | ||
|
|
||
|
|
@@ -153,7 +153,7 @@ public static IWebHostBuilder CreateDefaultBuilder() => | |
| /// </remarks> | ||
| /// <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) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also only nullable for chaining. Suppress the warning in the caller instead. |
||
| { | ||
| var builder = new WebHostBuilder(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| // 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 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we no longer have the ref project, we can turn on nullability on individual files. Convenient! |
||
|
|
||
| using System; | ||
| using Microsoft.AspNetCore.Builder; | ||
| using Microsoft.AspNetCore.Http.Features; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,12 @@ | ||
| // 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 | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Reflection; | ||
|
|
@@ -24,13 +27,13 @@ namespace Microsoft.AspNetCore.Hosting | |
| public class WebHostBuilder : IWebHostBuilder | ||
| { | ||
| private readonly HostingEnvironment _hostingEnvironment; | ||
| private Action<WebHostBuilderContext, IServiceCollection> _configureServices; | ||
| private readonly IConfiguration _config; | ||
| private readonly WebHostBuilderContext _context; | ||
|
|
||
| private IConfiguration _config; | ||
| private WebHostOptions _options; | ||
| private WebHostBuilderContext _context; | ||
| private WebHostOptions? _options; | ||
| private bool _webHostBuilt; | ||
| private Action<WebHostBuilderContext, IConfigurationBuilder> _configureAppConfigurationBuilder; | ||
| private Action<WebHostBuilderContext, IServiceCollection>? _configureServices; | ||
| private Action<WebHostBuilderContext, IConfigurationBuilder>? _configureAppConfigurationBuilder; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="WebHostBuilder"/> class. | ||
|
|
@@ -78,7 +81,7 @@ public string GetSetting(string key) | |
| /// <param name="key">The key of the setting to add or replace.</param> | ||
| /// <param name="value">The value of the setting to add or replace.</param> | ||
| /// <returns>The <see cref="IWebHostBuilder"/>.</returns> | ||
| public IWebHostBuilder UseSetting(string key, string value) | ||
| public IWebHostBuilder UseSetting(string key, string? value) | ||
| { | ||
| _config[key] = value; | ||
| return this; | ||
|
|
@@ -212,7 +215,8 @@ IServiceProvider GetProviderFromFactory(IServiceCollection collection) | |
| } | ||
| } | ||
|
|
||
| private IServiceCollection BuildCommonServices(out AggregateException hostingStartupErrors) | ||
| [MemberNotNull(nameof(_options))] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. weird
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It says that the field is non-null at the end of this method call. |
||
| private IServiceCollection BuildCommonServices(out AggregateException? hostingStartupErrors) | ||
| { | ||
| hostingStartupErrors = null; | ||
|
|
||
|
|
@@ -231,7 +235,7 @@ private IServiceCollection BuildCommonServices(out AggregateException hostingSta | |
|
|
||
| foreach (var attribute in assembly.GetCustomAttributes<HostingStartupAttribute>()) | ||
| { | ||
| var hostingStartup = (IHostingStartup)Activator.CreateInstance(attribute.HostingStartupType); | ||
| var hostingStartup = (IHostingStartup)Activator.CreateInstance(attribute.HostingStartupType)!; | ||
| hostingStartup.Configure(this); | ||
| } | ||
| } | ||
|
|
@@ -330,8 +334,8 @@ private void AddApplicationServices(IServiceCollection services, IServiceProvide | |
| // NOTE: This code overrides original services lifetime. Instances would always be singleton in | ||
| // application container. | ||
| var listener = hostingServiceProvider.GetService<DiagnosticListener>(); | ||
| services.Replace(ServiceDescriptor.Singleton(typeof(DiagnosticListener), listener)); | ||
| services.Replace(ServiceDescriptor.Singleton(typeof(DiagnosticSource), listener)); | ||
| services.Replace(ServiceDescriptor.Singleton(typeof(DiagnosticListener), listener!)); | ||
| services.Replace(ServiceDescriptor.Singleton(typeof(DiagnosticSource), listener!)); | ||
| } | ||
|
|
||
| private string ResolveContentRootPath(string contentRootPath, string basePath) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| // 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 | ||
|
|
||
| using System; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
|
|
@@ -103,7 +105,7 @@ public static async Task RunAsync(this IWebHost host, CancellationToken token = | |
| } | ||
| } | ||
|
|
||
| private static async Task RunAsync(this IWebHost host, CancellationToken token, string startupMessage) | ||
| private static async Task RunAsync(this IWebHost host, CancellationToken token, string? startupMessage) | ||
| { | ||
| try | ||
| { | ||
|
|
@@ -114,8 +116,8 @@ private static async Task RunAsync(this IWebHost host, CancellationToken token, | |
|
|
||
| if (!options.SuppressStatusMessages) | ||
| { | ||
| Console.WriteLine($"Hosting environment: {hostingEnvironment.EnvironmentName}"); | ||
| Console.WriteLine($"Content root path: {hostingEnvironment.ContentRootPath}"); | ||
| Console.WriteLine($"Hosting environment: {hostingEnvironment?.EnvironmentName}"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How can this be null?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RunAsync does a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, bug 😄 |
||
| Console.WriteLine($"Content root path: {hostingEnvironment?.ContentRootPath}"); | ||
|
|
||
|
|
||
| var serverAddresses = host.ServerFeatures.Get<IServerAddressesFeature>()?.Addresses; | ||
|
|
@@ -150,18 +152,18 @@ private static async Task RunAsync(this IWebHost host, CancellationToken token, | |
|
|
||
| private static async Task WaitForTokenShutdownAsync(this IWebHost host, CancellationToken token) | ||
| { | ||
| var applicationLifetime = host.Services.GetService<IHostApplicationLifetime>(); | ||
| var applicationLifetime = host.Services.GetRequiredService<IHostApplicationLifetime>(); | ||
|
|
||
| token.Register(state => | ||
| { | ||
| ((IHostApplicationLifetime)state).StopApplication(); | ||
| ((IHostApplicationLifetime)state!).StopApplication(); | ||
| }, | ||
| applicationLifetime); | ||
|
|
||
| var waitForStop = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); | ||
| applicationLifetime.ApplicationStopping.Register(obj => | ||
| { | ||
| var tcs = (TaskCompletionSource)obj; | ||
| var tcs = (TaskCompletionSource)obj!; | ||
| tcs.TrySetResult(); | ||
| }, waitForStop); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,6 @@ public interface IServerIntegratedAuth | |
| /// <summary> | ||
| /// The name of the authentication scheme for the server authentication handler. | ||
| /// </summary> | ||
| string AuthenticationScheme { get; } | ||
| string? AuthenticationScheme { get; } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should not be null. Where did you see otherwise? |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| @ECHO OFF | ||
| SET RepoRoot=%~dp0..\.. | ||
| %RepoRoot%\build.cmd -projects %~dp0**\*.*proj %* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I bother with this? This would have null-ref (so would have using a null
IHostBuilder).