Roslyn-analyzers: CA1305 not working for string interpolation or concatenation without culture

[wizofaus]

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.6.2

Diagnostic ID

Repro steps

Write code such as
Console.WriteLine("Today's date is " + DateTime.Now + ", and a random number is " + new Random().Next);
or
Console.WriteLine($"Today's date is {DateTime.Now} and a random number is {new Random().Next()}");

and compile with CA1305 configured as an error

Expected behavior

Both use the user's current culture, and hence could produce unexpected outputs in non-US locales (or even US locale with custom date formats etc)., and should trigger the CA1305 error.

Actual behavior

Code compiles with no warnings etc.

[Evangelink]

@mavasani I am not sure what was/is expected from this ticket.

I don't think that the rule shall be reported on all concatenation nor string interpolation, as this will probably result in a lot of False Positive. There is no way to know whether a string is expected to be localized or not.

I think that the best solution might be to report on string concatenation or string interpolation that are passed as string argument of a method that as an overload with a culture argument. That's probably what was intended here but I wanted to confirm so.

Besides, I tend to think that might be useful to add a list of excluded methods so that we don't have to add a pragma for all let's say logging call that we have if we decided not to localize logs.

WDYT?

[wizofaus]

Adding .ToString() to DateTime.Now or new Random().Next() does produce the warning though, even though it effectively means the same thing. So yes, I would expect it in exactly this case.

FWIW I agree that there are clearly lots of cases where this rule can and does produce "false positives", but ensuring that correct culture is always specified (Invariant or otherwise) is better than having your software unexpectedly output the wrong format when run on a system with a different locale than the one you're developing on.

[AdamSmith-Msft]

I'd love to see an intelligent solution such as what's discussed here:
http://blog.frankbakker.net/2015/09/c-string-interpolation-best-practices.html

[Bouke]

@AdamSmith-Msft I came here to reference that blog post. See also the implementation he provided on GH: https://github.com/FrankBakkerNl/StringInterpolationAnalyzer.

[Bosch-Eli-Black]

Also, this produces a CA1305 warning:

```C#
int x = 20;

ShowUserString(string.Format("Number: {0}", x);

but this does not:

```C#
int x = 20;

ShowUserString($"Number: {x}");

[FrankBakkerNl]

I just saw my blog post and git repo from five years ago were mentioned here :-) That's cool! This was just me experimenting with this. However It would be very cool to see my ideas beeing picked up here. So please feel free to use my code / thoughts. The rule I implemented is:

If an interpolated string has any interpolations that implement IFormattable, the interpolated string SHOULD be assigned to a FormattableString. This way we are sure the developer has made an explicit choice on how the formattable interpolations should be formatted.

I had not thought about string concatinations (or any implicit ToString() calls for that matter) That would be a nice addition