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

Codefixer should not use CultureInfo.CurrentUICulture #104802

Open
vitek-karas opened this issue Feb 6, 2023 · 4 comments
Open

Codefixer should not use CultureInfo.CurrentUICulture #104802

vitek-karas opened this issue Feb 6, 2023 · 4 comments
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Milestone

Comments

@vitek-karas
Copy link
Member

This is all about dotnet/roslyn#66566.

Once there's the right API to use, the code fixer should switch to it.

vitek-karas referenced this issue Feb 6, 2023
@bradwilson
Copy link

Analyzers are strongly suggested to get their UI from the compiler switch by using LocalizableResourceString, because the context in which they're executed is the compiler.

Fixers are not executed in the context of the compiler. They're executed in the context of a UI like Visual Studio or VS Code. As such, using CultureInfo.CurrentCulture should not have been prohibited by RS1035, and is actually the correct behavior to align with being displayed in a UI context. (This is what I've been told by people on the Roslyn team after discovering RS1035 raising issues in fixers.)

See: dotnet/roslyn-analyzers#7086 (comment)

@vitek-karas
Copy link
Member Author

/fyi: @sbomer

@sbomer sbomer transferred this issue from dotnet/linker Jul 12, 2024
@sbomer sbomer added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Jul 12, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jul 12, 2024
@sbomer sbomer added this to AppModel Jul 12, 2024
Copy link
Contributor

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

@sbomer sbomer added this to the 9.0.0 milestone Jul 12, 2024
@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Jul 24, 2024
@sbomer
Copy link
Member

sbomer commented Jul 29, 2024

We are following the guidance, so this issue only tracks removing this suppression for RS1035 once it is no longer produced for code fixers:

#pragma warning disable RS1035 // Do not use APIs banned for analyzers - https://github.com/dotnet/linker/issues/3197
var ruleTitle = diagnostic.Descriptor.Title.ToString (CultureInfo.CurrentUICulture);
#pragma warning restore RS1035 // Do not use APIs banned for analyzers

@sbomer sbomer modified the milestones: 9.0.0, Future Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Projects
Status: No status
Development

No branches or pull requests

4 participants