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

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

Closed
stephentoub opened this issue Nov 15, 2022 · 8 comments · Fixed by dotnet/roslyn-analyzers#6820
Closed

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

stephentoub opened this issue Nov 15, 2022 · 8 comments · Fixed by dotnet/roslyn-analyzers#6820
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Meta code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer
Milestone

Comments

@stephentoub
Copy link
Member

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.

@stephentoub stephentoub added code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer labels Nov 15, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 15, 2022
@ghost
Copy link

ghost commented Nov 15, 2022

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: -

@buyaa-n buyaa-n added this to the 8.0.0 milestone Nov 16, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 16, 2022
@stephentoub stephentoub added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 21, 2022
@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jan 24, 2023
@bartonjs
Copy link
Member

bartonjs commented Mar 7, 2023

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)

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 7, 2023
@marek-safar
Copy link
Contributor

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?

@buyaa-n buyaa-n added the help wanted [up-for-grabs] Good issue for external contributors label May 4, 2023
@CollinAlpert
Copy link
Contributor

Should this consider the OutputKind WindowsRuntimeApplication as well?

@buyaa-n buyaa-n removed the help wanted [up-for-grabs] Good issue for external contributors label Jul 31, 2023
@ericstj ericstj modified the milestones: 8.0.0, 9.0.0 Aug 11, 2023
@KennethHoff
Copy link

KennethHoff commented Sep 14, 2023

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

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

cremor commented Sep 15, 2023

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

@mavasani
Copy link

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

Indeed, that is correct.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Meta code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants