dotnet/roslyn

Extend 'Simplify interpolation' to add FormattableString.Invariant

Open

#52,581 opened on Apr 12, 2021

View on GitHub
 (3 comments) (0 reactions) (0 assignees)C# (4,257 forks)batch import
Area-IDEhelp wanted

Repository metrics

Stars
 (20,414 stars)
PR merge metrics
 (Avg merge 6d 17h) (256 merged PRs in 30d)

Description

The result of following https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1310 or https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1307 is often that you end up with .ToString(CultureInfo.InvariantCulture) in your code. In https://github.com/dotnet/roslyn/pull/52564, I extended 'Simplify interpolation' to recognize that this simplification is possible without changing meaning:

FormattableString.Invariant($"{first.ToString("0000", CultureInfo.InvariantCulture)}{second.ToString("0000", CultureInfo.InvariantCulture)}")

// 💡 Simplify interpolation:
FormattableString.Invariant($"{first:0000}{second:0000}")

What do you think about having it suggest FormattableString.Invariant in the first place when it does not change meaning and it brings a simplification opportunity? For example:

$"{first.ToString("0000", CultureInfo.InvariantCulture)}{second.ToString("0000", CultureInfo.InvariantCulture)}"

// 💡 Simplify interpolation:
FormattableString.Invariant($"{first:0000}{second:0000}")

Should this be something that "Convert concatenation to interpolation" also does for you, in case you didn't realize that FormattableString.Invariant existed or could help?

first.ToString("0000", CultureInfo.InvariantCulture) + second.ToString("0000", CultureInfo.InvariantCulture)

// 💡 Convert concatenation to interpolation:
FormattableString.Invariant($"{first:0000}{second:0000}")

Where to bail

The suggestion should not be given because it would change the meaning of {42} from current-culture to invariant-culture:

$"{42}{second.ToString("0000", CultureInfo.InvariantCulture)}"

The suggestion should not be given because it would switch to producing a string instead of a FormattableString instance:

FormattableString f = $"{42}{second.ToString("0000", CultureInfo.InvariantCulture)}"

So all interpolation sections must be known to be indifferent to culture (i.e. strings themselves) or must explicitly be using invariant formatting, and the interpolated string operation must be target-typed as string rather than FormattableString.

This should also bail because it would not change meaning but would still be confusing to wrap this in FormattableString.Invariant while leaving .ToString(CultureInfo.CurrentCulture) inside:

$"{first.ToString(CultureInfo.CurrentCulture)}{second.ToString(CultureInfo.InvariantCulture)}"

These mixed-culture strings seem highly unlikely to be intentional if they do occur.

Contributor guide