dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
34.87k stars 9.85k forks source link

Should we disable asp0014 when using `WebApplication.CreateEmptyBuilder()` #50732

Open WeihanLi opened 10 months ago

WeihanLi commented 10 months ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe the problem.

Since the empty WebAplicationBuilder would not register routing, maybe we could disable the ASP0014 using top-level route registrations diagnostic?

We may use the code like below, currently we would get ASP0014 warning image

var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions());
builder.Services.AddRoutingCore();
builder.WebHost.UseKestrelCore();
builder.Logging.AddConsole();

var app = builder.Build();

app.UseRouting();
app.UseEndpoints(endpoints => 
{
    endpoints.MapGet("/", () => "Hello World");
});

app.Run();

Describe the solution you'd like

Ignore it when using the empty WebApplicationBuilder since no default global routing

Additional context

No response

captainsafia commented 10 months ago

Thanks for the bug report, @WeihanLi! Yes, we should avoid emitting this diagnostic for web apps not configured with the routing middleware.

Let me see how easy this is to fix on the analyzer side...

david-acker commented 9 months ago

I just took a quick look, but it seems like this would be relatively straight forward. Right now we're emitting the diagnostic for any method on WebApplication.

Maybe we could pass an array of excluded method symbols to the IsDisallowedMethod method in WebApplicationBuilderAnalyzer? That way we could check that the method we're analyzing isn't one we want to exclude, after we check the method's containing type here.

Edit: I took a closer look into this, and it's definitely not as straightforward as I originally thought, as discussed below.

WeihanLi commented 9 months ago

Seemed we're emitting the ASP0014 diagnostic for UseEndpoints method only, we may need to distinguish whether the WebApplication is an empty web application, if it is, we may need to exclude it, while it seems there's no property or something indicates if it's empty for now

captainsafia commented 9 months ago

Seemed we're emitting the ASP0014 diagnostic for UseEndpoints method only, we may need to distinguish whether the WebApplication is an empty web application, if it is, we may need to exclude it, while it seems there's no property or something indicates if it's empty for now

Yep! The challenge here is that we want to continue emitting the warning about UseEndpoints for non-empty WebApplicationBuilders. There's not a straightforward way for us to determine if a given builder is empty or not unless we do some flow analysis to determine if it was constructed via CreateEmptyBuilder.

captainsafia commented 6 months ago

Backlogging this for now. Was hopping I'd be able to tackle this for 9.0-preview1 but that turned out to not be the case.

PRs welcome on this though!

ghost commented 6 months ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.