dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.35k stars 4.74k forks source link

[Analyzer] Make types declared in an executable internal #78388

Closed stephentoub closed 1 year ago

stephentoub commented 2 years ago

Various analyzers only pay attention to internal/private types and members, as changes to public API is visible. For example, analyzers/fixers that find and remove dead code, or that recommend types without derivations be sealed. Applications, rather than libraries, however, typically aren't referenced, and public types/members are likely mistakes, such that if the mistakes are corrected and the types/members have their visibility changed to internal/private, then other analyzers can start making recommendations for them.

The proposal is to add an analyzer/fixer that runs when a compilation's OutputKind is ConsoleApplication or WindowsApplication and that flags all public types, recommending they instead be made internal.

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-meta See info in area-owners.md if you want to be subscribed.

Issue Details
Various analyzers only pay attention to internal/private types and members, as changes to public API is visible. For example, analyzers/fixers that find and remove dead code, or that recommend types without derivations be sealed. Applications, rather than libraries, however, typically aren't referenced, and public types/members are likely mistakes, such that if the mistakes are corrected and the types/members have their visibility changed to internal/private, then other analyzers can start making recommendations for them. The proposal is to add an analyzer/fixer that runs when a compilation's OutputKind is ConsoleApplication or WindowsApplication and that flags all public types, recommending they instead be made internal.
Author: stephentoub
Assignees: -
Labels: `area-Meta`, `code-analyzer`, `code-fixer`
Milestone: -
bartonjs commented 1 year ago

Video

Looks good as proposed.

Given concerns with some of the templates (e.g. WinForms creating the skeleton for a new form type being default) we're recommending this analyzer be opt-in. But we're probably easily convinced that it should be something else.

Category: Maintainability (?) Visibility: None (off by default; might be nice if trimmer-style scenarios can flip it on by default)

marek-safar commented 1 year ago

The proposal is to add an analyzer/fixer that runs when a compilation's OutputKind is ConsoleApplication or WindowsApplication and that flags all public types, recommending they instead be made internal.

Could the analyzer run when the output binary contains runnable entry-point?

CollinAlpert commented 1 year ago

Should this consider the OutputKind WindowsRuntimeApplication as well?

KennethHoff commented 1 year ago

I see a lot of the newly added analyzers are opt-in.

How do I as a developer enable them, and more importantly, find which ones to enable? Basically, is there an easily digestible list somewhere?

I remember seeing a few good ones being added, so I worry that I'll forget about them and not benefit from them!

CollinAlpert commented 1 year ago

You can add an .editorconfig file to your solution and then select the severity of the rules you would like. For example: dotnet_diagnostic.CA1018.severity = warning

More information here

cremor commented 1 year ago

I assume it will also be enabled by setting <AnalysisLevel>latest-all</AnalysisLevel>?

mavasani commented 1 year ago

I assume it will also be enabled by setting <AnalysisLevel>latest-all</AnalysisLevel>?

Indeed, that is correct.