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

RS1035 is in conflict with CA1305 (and there are no docs about how to resolve the conflict) #7086

Open
bradwilson opened this issue Dec 9, 2023 · 5 comments
Assignees

Comments

@bradwilson
Copy link

bradwilson commented Dec 9, 2023

Analyzer

Diagnostic ID: RS1035: The symbol 'CultureInfo.CurrentCulture' is banned for use by analyzers: Analyzers should use the locale given by the compiler command line arguments, not the CurrentCulture

Analyzer source

NuGet Package: Microsoft.CodeAnalysis

Version: 4.8.0

Describe the bug

Analyzer rule CA1305 requires using some culture in calls to string.Format, but rule RS1035 (new in 4.8, I think?) prohibits them. These rules are in conflict.

Further, the documentation link for RS1035 is broken in the Visual Studio UI. Googling for RS1035 yields a few issues in this repo, but does not yield a link to an active documentation page, so the call to action is not implementable.

Steps To Reproduce

Use string.Format(CultureInfo.CurrentCulture, "{0}", 42); in an analyzer linked against Microsoft.CodeAnalysis 4.8.0.

Expected behavior

Instructions on what to do to resolve the conflict.

Actual behavior

No help.

@bradwilson
Copy link
Author

bradwilson commented Dec 14, 2023

The only place I could find this surfaced was here: https://github.com/dotnet/roslyn/blob/fd3194f093806a6b2577a45aa89b0fd60658eee2/src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs#L1727

I was unable to discover any way to get access to this from either AnalysisContext (for analyzers) or CodeFixContext (for fixes).

I have chosen to prefer CA1305 over RS1035 until this conflict can be resolved: https://github.com/xunit/xunit.analyzers/blob/7b8a772f5c4df014c5a3b1f34282e20be68bad08/.editorconfig#L234

@mavasani
Copy link
Contributor

mavasani commented Jan 2, 2024

@RikkiGibson

@bradwilson
Copy link
Author

Ping @RikkiGibson 😄

@RikkiGibson

This comment was marked as outdated.

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Jul 11, 2024

Revisiting. I think the actual resolution per #7167 (comment) is:

  1. Analyzers should use LocalizableResourceString
  2. Code fixers should be permitted to use CurrentCulture, while analyzers continue to warn

It would be nice to take care of this in the quality milestone, since it seems fairly common to hit it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants