Skip to content
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

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

Closed
2 of 15 tasks
captainsafia opened this issue Oct 17, 2024 · 8 comments
Closed
2 of 15 tasks
Assignees
Labels
analyzer Indicates an issue which is related to analyzer experience api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Milestone

Comments

@captainsafia
Copy link
Member

captainsafia commented Oct 17, 2024

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:

  • Add an IVT from their application code to their test code
  • Add a public partial class Program {} declaration

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:

  • Use top-level statements
  • Don't declare the Program class as public in some other fashion
  • Don't use the old-style Program.Main declarations

This source generator was implemented in #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

  • Design
  • Documentation
  • Globalization
  • Interoperability
  • Maintainability
  • Naming
  • Performance
  • Reliability
  • Security
  • Style
  • Usage

Severity Level

  • Error
  • Warning
  • Info
  • Hidden

Usage Scenarios

using Microsoft.AspNetCore.Builder;

 var app = WebApplication.Create();

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

 app.Run();

public partial class Program {} // Emit info diagnostic here
@captainsafia captainsafia added analyzer Indicates an issue which is related to analyzer experience api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Oct 17, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Oct 17, 2024
@captainsafia captainsafia added api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Oct 17, 2024
@captainsafia captainsafia self-assigned this Oct 17, 2024
@amcasey
Copy link
Member

amcasey commented Oct 17, 2024

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
Copy link
Member Author

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

@captainsafia
Copy link
Member Author

@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
Copy link
Member

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
Copy link
Member

amcasey commented Oct 24, 2024

[API Review]

  • What brings in the analyzer?
    • It's part of the shared framework
  • Are the members of the entrypoint type also public?
    • No
  • We think Info goes in the error list but Hidden doesn't
    • It feels like we might not need this in the error list
    • Neither squiggles, as far as we know
  • Is there a code fix for the analyzer?
    • Yes, and it has fix-all
  • Title candidates:
    • Public Program partial class declaration not required
    • Unnecessary public Program class declaration
  • Message candidates:

@amcasey
Copy link
Member

amcasey commented Oct 24, 2024

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!

@amcasey amcasey added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Oct 24, 2024
@mikekistler mikekistler added this to the .NET 10 Planning milestone Oct 28, 2024
@captainsafia
Copy link
Member Author

Closing. This will ship in .NET 10 Preview 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer Indicates an issue which is related to analyzer experience api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Projects
None yet
Development

No branches or pull requests

4 participants