Skip to content

Add ECS0005: prefer formattablestring for culture specific strings #49

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7574625
Add ECS0005 - Prefer FormattableString for culture specific strings
rjmurillo Aug 9, 2024
9031675
Add more defaults into global usings for tests
rjmurillo Aug 10, 2024
abd5f00
Add more use cases for string interpolation
rjmurillo Aug 10, 2024
b8e5352
Fix error in analyzer documentation meta
rjmurillo Aug 10, 2024
4741a33
Add documentation for ECS0005
rjmurillo Aug 10, 2024
4be9ccc
Remove unused global usings causing issues with older .NET versions
rjmurillo Aug 10, 2024
9cd02c0
Add Xunit.Abstractions to global namespace
rjmurillo Aug 10, 2024
e9b5311
Fix static analysis violations
rjmurillo Aug 10, 2024
048b365
Add support for string interpolation of strings, which is lowered to …
rjmurillo Aug 10, 2024
6e8c866
Fix documentation path for ECS0005
rjmurillo Aug 10, 2024
8b2aa44
Merge branch 'release/v0.1.0' into feature/issue-29-prefer-formattabl…
rjmurillo Aug 12, 2024
7a20c14
Update SquiggCop baselines
rjmurillo Aug 12, 2024
c4d6d30
Convert ctor format
rjmurillo Aug 12, 2024
4694bc4
Add logic to guide users based on .NET version
rjmurillo Aug 12, 2024
9080207
Update documentation to add link from .NET 6 blog
rjmurillo Aug 12, 2024
c8a3ce1
Add Code fix for ECS0005
rjmurillo Aug 12, 2024
6451656
Fix code analysis issues
rjmurillo Aug 12, 2024
26298e5
Add feature detection capablities and update FormattibleString guidan…
rjmurillo Aug 13, 2024
cd2766b
Correct code analysis warnings
rjmurillo Aug 13, 2024
9780a46
Update SquiggleCop baseline
rjmurillo Aug 13, 2024
c1bd5d1
Remove PolySharp
rjmurillo Aug 13, 2024
80d1dba
Add unit tests for determining language version from .NET version
rjmurillo Aug 13, 2024
494bfef
Update documentation for GetVersions extension method
rjmurillo Aug 13, 2024
8ca44f8
Add unit tests for feature detection
rjmurillo Aug 13, 2024
efbeb91
Update SquiggleCop baselines
rjmurillo Aug 13, 2024
a855287
Remove extra file Temp.txt
rjmurillo Aug 13, 2024
efd4ff9
Update SquiggleCop.Tool
rjmurillo Aug 14, 2024
c99180a
Update SquiggleCop baseline for tests
rjmurillo Aug 14, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .config/dotnet-tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"rollForward": false
},
"squigglecop.tool": {
"version": "1.0.8",
"version": "1.0.13",
"commands": [
"dotnet-squigglecop"
],
Expand Down
7 changes: 0 additions & 7 deletions build/targets/compiler/Compiler.props
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="PolySharp">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
</ItemGroup>

<ItemGroup>
<AssemblyAttribute Include="System.CLSCompliantAttribute">
<_Parameter1>false</_Parameter1>
Expand Down
5 changes: 2 additions & 3 deletions build/targets/compiler/Packages.props
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
<Project>
<ItemGroup>
<PackageVersion Include="PolySharp" Version="1.14.1" />
</ItemGroup>
<ItemGroup>
</ItemGroup>
</Project>
2 changes: 1 addition & 1 deletion docs/DOCUMENTING-ANALYZERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private static readonly DiagnosticDescriptor Rule = new(
category: "Maintainability",
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: true,
helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/{Id}.md");
helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{Id}.md");
```

The documentation for the rule is placed in to `docs/rules/ECS0002.md`.
Expand Down
72 changes: 72 additions & 0 deletions docs/rules/ECS0005.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# ECS0005: Prefer FormattableString for culture-specific strings

This rule is discussed in detail in [Effective C#: 50 Specific Ways to Improve your C#](https://www.oreilly.com/library/view/effective-c-50/9780134579290/). Guidance about the feature can be found on the [.NET Blog - String Interpolation in C# 10 and .NET 6](https://devblogs.microsoft.com/dotnet/string-interpolation-in-c-10-and-net-6/).

## Cause

The rule is triggered when a `string` is used for an interpolated string that could benefit from culture-specific formatting.

## Rule description

There are still valid use cases for `string.Format` and `StringBuilder.AppendFormat` where the composite format string isn't known at compile time (e.g., localized resources). However, if the string would otherwise be hardcoded into the C#, interpolated are preferred (from a performance perspective). Using `FormattableString` or `string.Create` instead of `string` for interpolated strings ensures that culture-specific formatting is correctly applied, preventing issues where the default culture may lead to incorrect string representations.

## How to fix violations

Replace the `string` with `FormattableString` where culture-specific formatting is required. For example, use `FormattableString.Invariant(...)` or `FormattableString.ToString(IFormatProvider)` as needed.

## When to suppress warnings

Suppress warnings if the default culture is explicitly intended, or the string does not require culture-specific formatting.

### Suppress a warning

If you want to suppress a single violation, add preprocessor directives to your source file to disable and then re-enable the rule.

```csharp
#pragma warning disable ECS0005
// The code that's violating the rule
#pragma warning restore ECS0005
```

To disable the rule for a file, folder, or project, set its severity to none in the [configuration file](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files).

```ini
[*.cs]
dotnet_diagnostic.ECS0005.severity = none
```

## Example of a violation

### Description

Using `string` for an interpolated string that requires culture-specific formatting.

### Code

```csharp
public string GetMessage()
{
double value = 299792.458;
return $"The speed of light is {value:N3} km/s."; // ECS0005 triggers here
}
```

## Example of how to fix

### Description

Replace `string` with `FormattableString` to handle culture-specific formatting.

### Code

```csharp
public string GetMessage()
{
double value = 299792.458;
return FormattableString.Invariant($"The speed of light is {value:N3} km/s.");
}
```

## Related rules

[ECS0004: Replace string.Format with interpolated string](./ECS0004.md)
3 changes: 2 additions & 1 deletion effectivecsharpanalyzers.sln.DotSettings
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
<s:Boolean x:Key="/Default/Environment/Filtering/ExcludeCoverageFilters/=EffectiveCSharp_002EAnalyzers_002EBenchmarks_003B_002A_003B_002A_003B_002A/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
<s:Boolean x:Key="/Default/Environment/Filtering/ExcludeCoverageFilters/=EffectiveCSharp_002EAnalyzers_002EBenchmarks_003B_002A_003B_002A_003B_002A/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Formattable/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Rule ID | Category | Severity | Notes
ECS0001 | Style | Info | PreferImplicitlyTypedLocalVariablesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/e7c151c721c3039011356d6012838f46e4b60a21/docs/rules/ECS0001.md)
ECS0002 | Maintainability | Info | PreferReadonlyOverConstAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/10c2d53afd688efe5a59097f76cb4edf33f6a474/docs/rules/ECS0002.md)
ECS0004 | Style | Info | ReplaceStringFormatAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers5da647e447fad4eb0a9e3db287e1d16cce316114/docs/rules/ECS0004.md)
ECS0005 | Globalization | Info | FormattableStringForCultureSpecificStringsAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/4741a337436bf0b94fc8274d5765a804396affd8/docs/rules/ECS0005.md)
ECS0006 | Refactoring | Info | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers6213cba8473dac61d6132e205550884eae1c94bf/docs/rules/ECS0006.md)
ECS0007 | Design | Info | ExpressCallbacksWithDelegatesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/dc194bbde030e9c40d8d9cdb1e0b5ff8919fe5a8/docs/rules/ECS0007.md)
ECS0008 | Usage | Info | EventInvocationAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/cc7d91eb81f6781851c09732db1268be7dab402b/docs/rules/ECS0008.md)
Expand Down
1 change: 1 addition & 0 deletions src/EffectiveCSharp.Analyzers/Common/Categories.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ internal static class Categories
internal static readonly string Performance = nameof(Performance);
internal static readonly string Style = nameof(Style);
internal static readonly string Usage = nameof(Usage);
internal static readonly string Globalization = nameof(Globalization);
}
142 changes: 142 additions & 0 deletions src/EffectiveCSharp.Analyzers/Common/CompilationExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
using System.Runtime.CompilerServices;

namespace EffectiveCSharp.Analyzers.Common;

internal static class CompilationExtensions
{
/// <summary>
/// Gets the language version the compiler is capable of parsing.
/// </summary>
/// <param name="compilation">The compilation.</param>
/// <returns>A <see cref="LanguageVersion"/> if there is a <see cref="CSharpParseOptions"/>; otherwise, null.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static LanguageVersion? GetLanguageVersionFromCompilation(this Compilation compilation)
{
return compilation.SyntaxTrees.FirstOrDefault()?.Options is not CSharpParseOptions parseOptions
? null
: parseOptions.LanguageVersion;
}

/// <summary>
/// Gets the specific <paramref name="typeName"/>.
/// </summary>
/// <param name="compilation">The <see cref="Compilation"/> instance.</param>
/// <param name="typeName">Fully qualified name of the type.</param>
/// <returns>True if the type can be located; otherwise, false.</returns>
/// <seealso cref="Compilation.GetTypeByMetadataName"/>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static bool SupportsType(this Compilation compilation, string typeName)
{
INamedTypeSymbol? symbol = compilation.GetTypeByMetadataName(typeName);
return symbol != null;
}

/// <summary>
/// Gets the specific <paramref name="typeName"/>.
/// </summary>
/// <param name="compilation">The <see cref="Compilation"/> instance.</param>
/// <param name="typeName">Fully qualified name of the type.</param>
/// <param name="memberName">Name of the member on the type.</param>
/// <returns>True if the type with the specified member can be located; otherwise, false.</returns>
/// <seealso cref="Compilation.GetTypeByMetadataName"/>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static bool SupportsType(this Compilation compilation, string typeName, string memberName)
{
INamedTypeSymbol? symbol = compilation.GetTypeByMetadataName(typeName);
return symbol?.GetMembers(memberName).Length > 0;
}

/// <summary>
/// Gets the version from the compilation's referenced types.
/// </summary>
/// <param name="compilation">The compilation.</param>
/// <returns>A <see cref="Version"/> if there is a <see cref="object"/> that can be located; otherwise, performs feature detection.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static Version? GetDotNetVersionFromCompilation(this Compilation compilation)
{
Version mscorlibVersion = compilation.GetSpecialType(SpecialType.System_Object).ContainingAssembly.Identity.Version;

// If we detect a .NET version newer than 4, then just return that.
if (mscorlibVersion > DotNet.Versions.DotNet40)
{
return mscorlibVersion;
}

// The assembly version of mscorlib would be `4.0.0.0` regardless of the .NET Framework version
// To differentiate, we need to sniff for specific types or methods that exist only in those .NET versions

// Check for .NET Framework 4.8+ (introduced System.Runtime.GCLargeObjectHeapCompactionMode)
bool gcLargeObjectHeapCompactionModeType = compilation.SupportsType("System.Runtime.GCLargeObjectHeapCompactionMode");

if (gcLargeObjectHeapCompactionModeType)
{
// .NET Framework 4.8+
return DotNet.Versions.DotNet48;
}

// Check for .NET Framework 4.7+ (introduced System.ValueTuple)
bool valueTupleType = compilation.SupportsType("System.ValueTuple");

if (valueTupleType)
{
// .NET Framework 4.7+
return DotNet.Versions.DotNet47;
}

// Check for .NET Framework 4.6.2+ (introduced System.AppContext.SetSwitch)
bool hasSetSwitch = compilation.SupportsType("System.AppContext", "SetSwitch");

if (hasSetSwitch)
{
// .NET Framework 4.6.2+
return DotNet.Versions.DotNet46;
}

return null;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static bool IsCSharpVersionOrLater(this Compilation compilation, LanguageVersion desiredLanguageVersion)
{
LanguageVersion? languageVersion = GetLanguageVersionFromCompilation(compilation);

return languageVersion.HasValue && languageVersion.Value >= desiredLanguageVersion;
}

/// <summary>
/// Gets the .NET runtime version and the supported compiler language version.
/// </summary>
/// <param name="compilation">The compilation.</param>
/// <returns>
/// A <see cref="Tuple{Version, LanguageVersion, LanguageVersion}"/>:
/// <list type="unordered">
/// <item>
/// <term><see cref="Version"/></term>
/// <description>Contains the version of the assembly containing the type <see cref="object"/>.</description>
/// </item>
/// <item>
/// <term><see cref="LanguageVersion" /></term>
/// <description>The documented default language version for a given .NET version from <see cref="DotNet.LangVersion.FromDotNetVersion"/>. </description>
/// </item>
/// <item>
/// <term><see cref="LanguageVersion" /></term>
/// <description>The effective language version, which the compiler uses to produce the <see cref="SyntaxTree"/>.</description>
/// </item>
/// </list>
/// </returns>
/// <remarks>
/// The .NET version and the language version can differ, for example we can downgrade the language version
/// on a newer runtime, or downgrade the runtime with a newer language. When picking up issues, we want to
/// make sure analyzers and code fix providers are finding legitimate issues and offering compatible solutions.
/// </remarks>
/// <seealso cref="DotNet.LangVersion.FromDotNetVersion"/>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static (Version? DotNetVersion, LanguageVersion? DotNetLanguageVersion, LanguageVersion? CompilerLanguageVersion) GetVersions(this Compilation compilation)
{
Version? version = GetDotNetVersionFromCompilation(compilation);
LanguageVersion? compilerLanguageVersion = GetLanguageVersionFromCompilation(compilation);
LanguageVersion? dotnetLanguageVersion = DotNet.LangVersion.FromDotNetVersion(version);

return (version, dotnetLanguageVersion, compilerLanguageVersion);
}
}
1 change: 1 addition & 0 deletions src/EffectiveCSharp.Analyzers/Common/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ internal static class DiagnosticIds
internal const string PreferImplicitlyTypedLocalVariables = "ECS0001";
internal const string PreferReadonlyOverConst = "ECS0002";
internal const string ReplaceStringFormatWithInterpolatedString = "ECS0004";
internal const string PreferFormattableStringForCultureSpecificStrings = "ECS0005";
internal const string AvoidStringlyTypedApis = "ECS0006";
internal const string ExpressCallbacksWithDelegates = "ECS0007";
internal const string UseNullConditionalOperatorForEventInvocations = "ECS0008";
Expand Down
93 changes: 93 additions & 0 deletions src/EffectiveCSharp.Analyzers/Common/DotNet.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
namespace EffectiveCSharp.Analyzers.Common;

internal static class DotNet
{
/// <summary>For use with feature detection.</summary>
/// <remarks>When using this with <see cref="CompilationExtensions.GetVersions"/>.</remarks>
internal static class Versions
{
internal static readonly Version DotNet6 = new(6, 0, 0, 0);
internal static readonly Version DotNet5 = new(5, 0, 0, 0);
internal static readonly Version DotNet46 = new(4, 6);
internal static readonly Version DotNet47 = new(4, 7);
internal static readonly Version DotNet48 = new(4, 8);
internal static readonly Version DotNet40 = new(4, 0, 0, 0);
}

/*
References
https://learn.microsoft.com/en-us/lifecycle/products/microsoft-net-framework
https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-framework
https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core
https://stackoverflow.com/questions/247621/what-are-the-correct-version-numbers-for-c/38506668#38506668

| C# Version | VS Version | .NET Version | CLR Version | Release Date | End of Support |
|------------|-------------------|---------------|-------------|--------------|----------------|
| 1.0 | 2002 | 1.0 | 1.0 | Feb 2002 | N/A |
| 1.2 | 2003 | 1.1 | 1.1 | Apr 2003 | N/A |
| 2.0 | 2005 | 2.0 | 2.0 | Nov 2005 | N/A |
| 3.0 | 2008 | 3.5 | 2.0 | Nov 2007 | N/A |
| 4.0 | 2010 | 4.0 | 4 | Apr 2010 | N/A |
| 5.0 | 2012 | 4.5 | 4 | Aug 2012 | N/A |
| 6.0 | 2015 | 4.6 | 4 | Jul 2015 | N/A |
| 7.0 | 2017 | 4.6.2 | 4 | Mar 2017 | N/A |
| 7.0 | 2017 | .NET Core 1.0 | N/A | Mar 2017 | Jun 2019 |
| 7.1 | 2017 (v15.3) | 4.6.2 | 4 | Aug 2017 | N/A |
| 7.1 | 2017 (v15.3) | .NET Core 2.0 | N/A | Aug 2017 | Oct 2018 |
| 7.2 | 2017 (v15.5) | 4.7.2 | 4 | Dec 2017 | N/A |
| 7.3 | 2017 (v15.7) | 4.7.2 | 4 | May 2018 | N/A |
| 7.3 | 2017 (v15.7) | .NET Core 2.1 | N/A | May 2018 | Aug 2021 |
| 8.0 | 2019 | 4.8 | 4 | Apr 2019 | N/A |
| 8.0 | 2019 | .NET Core 3.0 | N/A | Sep 2019 | Mar 2020 |
| 8.0 | 2019 | .NET Core 3.1 | N/A | Dec 2019 | Dec 2022 |
| 9.0 | 2020 | .NET 5 | N/A | Nov 2020 | May 2022 |
| 10.0 | 2021 | .NET 6 | N/A | Nov 2021 | Nov 2024 |
| 11.0 | 2022 (17.4) | .NET 7 | N/A | Nov 2022 | May 2024 |
| 12.0 | 2023 (17.8) | .NET 8 | N/A | Nov 2023 | Nov 2026 |
*/
internal static class LangVersion
{
internal static LanguageVersion? FromDotNetVersion(Version? version)
{
return version?.Major switch
{
3 when version.Minor == 5 => LanguageVersion.CSharp3,

// .NET Framework versions
4 when version.Minor == 8 => LanguageVersion.CSharp8,
4 when version is { Minor: 7, Build: 2 } => LanguageVersion.CSharp7_3,
4 when version is { Minor: 7, Build: 1 } => LanguageVersion.CSharp7_1,
4 when version is { Minor: 7 } => LanguageVersion.CSharp7,
4 when version is { Minor: 6, Build: 2 } => LanguageVersion.CSharp7,
4 when version is { Minor: 6 } => LanguageVersion.CSharp6,

// .NET Core versions
5 => LanguageVersion.CSharp9,
6 => LanguageVersion.CSharp10,
7 =>

// REVIEW: This should be CSharp11, but it's not available in the enum
LanguageVersion.CSharp10,
8 =>

// REVIEW: This should be CSharp12, but it's not available in the enum
LanguageVersion.CSharp10,

9 =>

// REVIEW: This should be CSharp12, but it's not available in the enum
LanguageVersion.CSharp10,

// .NET Core specific versions
3 when version.Minor == 1 => LanguageVersion.CSharp8,
3 when version.Minor == 0 => LanguageVersion.CSharp8,
2 when version.Minor == 2 => LanguageVersion.CSharp7_3,
2 when version.Minor == 1 => LanguageVersion.CSharp7_3,
2 when version.Minor == 0 => LanguageVersion.CSharp7_1,
1 when version.Minor == 1 => LanguageVersion.CSharp7,
1 when version.Minor == 0 => LanguageVersion.CSharp7,
_ => null,
};
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,8 @@
<Content Include="build\*" PackagePath="build" />
<Content Include="build\config\*" PackagePath="build\config" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="EffectiveCSharp.Analyzers.Tests" />
</ItemGroup>
</Project>
Loading
Loading