search

dotnet / runtime

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

Analyzer proposal: Remove unused resources #79765

Open Youssef1313 opened 1 year ago

Youssef1313 commented 1 year ago

Background and motivation

Too much of unused resources can add to assembly size and can (in some cases) be a maintenance cost. This proposes an analyzer to detect unused resources.

API Usage

The analyzer will analyze the generated C# code for resx files. Assuming resx files named Resources1.resx and Resources2.resx, the analyzer will flag the unused static properties of type string in classes named Resources1 and Resources2.

Analyzer could be enabled by default as an IDE suggestion. Category: Maintainability (maybe Usage or Performance)? Not really sure.

Risks

NOTE: While this is not an API-specific analyzer, it was suggested that it gets reviewed by dotnet/runtime team in https://github.com/dotnet/roslyn-analyzers/pull/6338#issuecomment-1354580318

ghost commented 1 year ago

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

Issue Details
### Background and motivation Too much of unused resources can add to assembly size and can (in some cases) be a maintenance cost. This proposes an analyzer to detect unused resources. ### API Proposal Not applicable for an analyzer suggestion. ### API Usage Not applicable for an analyzer suggestion. ### Alternative Designs - ### Risks **NOTE:** While this is not an API-specific analyzer, it was suggested that it gets reviewed by dotnet/runtime team in https://github.com/dotnet/roslyn-analyzers/pull/6338#issuecomment-1354580318
Author: Youssef1313
Assignees: -
Labels: `api-suggestion`, `area-Meta`, `untriaged`
Milestone: -
ghost commented 1 year ago

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

Issue Details
### Background and motivation Too much of unused resources can add to assembly size and can (in some cases) be a maintenance cost. This proposes an analyzer to detect unused resources. ### API Proposal Not applicable for an analyzer suggestion. ### API Usage Not applicable for an analyzer suggestion. ### Alternative Designs - ### Risks **NOTE:** While this is not an API-specific analyzer, it was suggested that it gets reviewed by dotnet/runtime team in https://github.com/dotnet/roslyn-analyzers/pull/6338#issuecomment-1354580318
Author: Youssef1313
Assignees: -
Labels: `api-suggestion`, `area-Meta`, `area-System.Runtime`, `untriaged`
Milestone: -
buyaa-n commented 1 year ago

@Youssef1313 you could remove the not applicable parts. It would be good to have some examples in API Usage part like for example what the analyzer would flag and what the fix looks like. Any edge cases that need attention etc. Also, could propose the category and severity if you have any suggestions.

Youssef1313 commented 1 year ago

@buyaa-n Edited

stephentoub commented 1 year ago

While this is not an API-specific analyzer, it was suggested that it gets reviewed by dotnet/runtime team in https://github.com/dotnet/roslyn-analyzers/pull/6338

I think this is a very good analyzer to add. We struggle with this ourselves in dotnet/runtime.

danmoseley commented 1 year ago

If it's easy to add, the analyzer could look for duplicate strings with different resource IDs. I've only seen this two or three times so it's not worth it unless it's a few lines.

mavasani commented 1 year ago

Note that this analyzer would need to be a compilation end analyzer as it would need to analyze the whole compilation to identify unused resource strings/backing properties. This would mean it cannot support a code fixer or be enabled by default in the IDE for live analysis, it will be a build-only analyzer. I still think it would be a useful analyzer, but just wanted to add this context.

bartonjs commented 1 year ago

Video

This seems generally good as proposed, since it is focused on generated code (which is often ignored by other analyzers). Rather than using convention-based naming it probably wants to find the resx->cs options in the csproj and behave like the tooling would.

It's a little sad that a fixer isn't viable here, but c'est la vie.

Category: Maintainability Visibility: None (off by default) Code: TBD (by the roslyn-analyzers side)