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 proposal: Remove unused resources #79765

Open
Youssef1313 opened this issue Dec 16, 2022 · 8 comments
Open

Analyzer proposal: Remove unused resources #79765

Youssef1313 opened this issue Dec 16, 2022 · 8 comments
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer
Milestone

Comments

@Youssef1313
Copy link
Member

Youssef1313 commented Dec 16, 2022

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

  • IVTs?
  • Reflection?
  • Suppressions can't be done for a single violation via the well-known methods. An .editorconfig option will be exposed to suppress violations.

NOTE: While this is not an API-specific analyzer, it was suggested that it gets reviewed by dotnet/runtime team in dotnet/roslyn-analyzers#6338 (comment)

@Youssef1313 Youssef1313 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 16, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 16, 2022
@ghost
Copy link

ghost commented Dec 16, 2022

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 dotnet/roslyn-analyzers#6338 (comment)

Author: Youssef1313
Assignees: -
Labels:

api-suggestion, area-Meta, untriaged

Milestone: -

@ghost
Copy link

ghost commented Dec 16, 2022

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 dotnet/roslyn-analyzers#6338 (comment)

Author: Youssef1313
Assignees: -
Labels:

api-suggestion, area-Meta, area-System.Runtime, untriaged

Milestone: -

@dakersnar dakersnar added code-analyzer Marks an issue that suggests a Roslyn analyzer and removed area-Meta labels Dec 16, 2022
@buyaa-n
Copy link
Member

buyaa-n commented Dec 16, 2022

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

@buyaa-n Edited

@stephentoub
Copy link
Member

While this is not an API-specific analyzer, it was suggested that it gets reviewed by dotnet/runtime team in dotnet/roslyn-analyzers#6338

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

@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 3, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 3, 2023
@stephentoub stephentoub added the code-fixer Marks an issue that suggests a Roslyn code fixer label Jan 3, 2023
@danmoseley
Copy link
Member

danmoseley commented Jan 3, 2023

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.

@buyaa-n buyaa-n added this to the 8.0.0 milestone Jan 3, 2023
@mavasani
Copy link
Member

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

bartonjs commented Mar 7, 2023

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)

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed code-fixer Marks an issue that suggests a Roslyn code fixer api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 7, 2023
@tannergooding tannergooding modified the milestones: 8.0.0, Future Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer
Projects
None yet
Development

No branches or pull requests

8 participants