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
35.43k stars 10.01k forks source link

Provide information diagnostic when `public partial class Program { }` declaration is found in code #58488

Open captainsafia opened 2 weeks ago

captainsafia commented 2 weeks ago

Background and Motivation

With the introduction of top-level statements in .NET 6, there is not longer an explicit Program class defined in user's ASP.NET Core applications. Instead, we rely on the Program class generated by the compiler, which has a default visibility of internal.

This introduces an annoying hurdle for users who want to write integration tests on top of our WebApplicationFactory which requires a public entrypoint as a generic type argument.

To work around this, we document that users can either:

The first approach runs the risk of the user having to expose truly internal types to their test code. The second approach is hard to discover.

To resolve this issue, we introduced a new source generator to the shared framework that will emit the public partial class Program {} declaration to applications that:

This source generator was implemented in https://github.com/dotnet/aspnetcore/pull/58199. Based on the code generation that we introduced here, users can get rid of the explicit public partial class Program { } declarations in their source code and rely on the new default behavior. We propose introducing an analyzer to find these explicit declarations and a code fixer to remove them.

Proposed Analyzer

Analyzer Behavior and Message

Title

Explicit class declaration not required

Message

Using public partial class Program { } to make generated Program class public is no longer required. See https://aka.ms/aspnetcore-warnings/ASP0027 for more details.

Category

Severity Level

Usage Scenarios

using Microsoft.AspNetCore.Builder;

 var app = WebApplication.Create();

 app.MapGet("/", () => "Hello, World!");

 app.Run();

public partial class Program {} // Emit info diagnostic here
amcasey commented 2 weeks ago

Isn't there a special category/level for analyzers that are flagging redundant code? It might be Hidden but I think there might actually be a bool to tell the editor to grey out the span, rather than squiggling it. Otherwise, LGTM.

captainsafia commented 1 week ago

Isn't there a special category/lever for analyzers that are flagging redundant code? It might be Hidden but I think there might actually be a bool to tell the editor to grey out the span, rather than squiggling it. Otherwise, LGTM.

This is the full set of available severities. I think our "Hidden" above aligns with the "None" below.

Image

amcasey commented 1 week ago

This is what I was thinking of: https://github.com/dotnet/roslyn/blob/048a23fcea1ef7346d2a59d4e4345dbb1fb780b9/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs#L14

captainsafia commented 1 week ago

@amcasey Ah I see -- it looks like this is a tag that can be added in addition to the severity level of the property. I can incorporate this into the PR.

danroth27 commented 1 week ago

Minor nit on the message text:

Using public partial class Program { } to make the generated Program class public is no longer required. See https://aka.ms/aspnetcore-warnings/ASP0027 for more details.

amcasey commented 1 week ago

[API Review]

amcasey commented 1 week ago

Title Unnecessary public Program class declaration Message Using public partial class Program { } to make the generated Program class public is no longer required. See https://aka.ms/aspnetcore-warnings/ASP0027 for more details. Severity Hidden or Info, TBD

API Approved!