Skip to content

Add Roslyn Analyzer for Replacing string.Format with Interpolated Strings (ECS0004) #41

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
merged 13 commits into from
Aug 2, 2024
Merged
2 changes: 1 addition & 1 deletion docs/rules/ECS0002.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# PRC001: Prefer readonly over const
# ECS0002: Prefer readonly over const

## Cause
Using `const` fields for constants that do not need to be available at compile time.
Expand Down
59 changes: 59 additions & 0 deletions docs/rules/ECS0004.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# ECS0004: Replace string.Format with interpolated string

This rule is described in detail in [Effective C#: 50 Specific Ways to Improve your C#](https://www.oreilly.com/library/view/effective-c-50/9780134579290/).

## Cause

Using `string.Format` for formatting strings instead of using interpolated strings.

## Rule description

This rule identifies the usage of `string.Format` method and suggests replacing it with C# interpolated strings. Interpolated strings enhance code readability and reduce the likelihood of runtime errors associated with mismatched format arguments.

## How to fix violations

Replace the usage of `string.Format` with an interpolated string. Interpolated strings are more readable and less error-prone compared to `string.Format`.

## When to suppress warnings

Suppress this warning if you have a specific reason to use `string.Format`, such as when dynamically constructing the format string at runtime or when maintaining compatibility with older code bases that rely on `string.Format`.

## Example of a violation

### Description

Using `string.Format` to format a string.

### Code

```csharp
class Program
{
void Main()
{
var str = string.Format("Hello, {0}!", "world");
}
}

## Example of how to fix

### Description

Replacing `string.Format` with an interpolated string.

### Code

```csharp
class Program
{
void Main()
{
var world = "world";
var str = $"Hello, {world}!";
}
}
```

## Related rules

[ECS0009: Minimize boxing and unboxing](./ECS0009.md)
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Rule ID | Category | Severity | Notes
--------|----------|----------|-------
ECS0001 | Style | Info | PreferImplicitlyTypedLocalVariablesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/e7c151c721c3039011356d6012838f46e4b60a21/docs/ECS0001.md)
ECS0002 | Maintainability | Info | PreferReadonlyOverConstAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/10c2d53afd688efe5a59097f76cb4edf33f6a474/docs/ECS0002.md)
ECS0004 | Style | Info | ReplaceStringFormatAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers5da647e447fad4eb0a9e3db287e1d16cce316114/docs/ECS0004.md)
ECS0006 | Refactoring | Info | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0006.md)
ECS0009 | Performance | Info | MinimizeBoxingUnboxingAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0009.md)
ECS1000 | Performance | Info | SpanAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/d00a4cc9f61e7d5b392894aad859e46c43a5611c/docs/ECS1000.md)
10 changes: 10 additions & 0 deletions src/EffectiveCSharp.Analyzers/Common/ITypeSymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,14 @@ or SpecialType.System_Single
or SpecialType.System_Double
or SpecialType.System_Decimal;
}

/// <summary>
/// Determines if the <paramref name="typeSymbol"/> is <see cref="string"/>.
/// </summary>
/// <param name="typeSymbol">The type.</param>
/// <returns>Return true if the type is <see cref="string"/>; otherwise, false.</returns>
internal static bool IsString(this ITypeSymbol typeSymbol)
{
return typeSymbol?.SpecialType == SpecialType.System_String;
}
}
1 change: 1 addition & 0 deletions src/EffectiveCSharp.Analyzers/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ internal static class DiagnosticIds
{
internal const string PreferImplicitlyTypedLocalVariables = "ECS0001";
internal const string PreferReadonlyOverConst = "ECS0002";
internal const string ReplaceStringFormatWithInterpolatedString = "ECS0004";
internal const string AvoidStringlyTypedApis = "ECS0006";
internal const string MinimizeBoxingUnboxing = "ECS0009";
internal const string BeAwareOfValueTypeCopyInReferenceTypes = "ECS0009";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ public class PreferExplicitTypesOnNumbersAnalyzer : DiagnosticAnalyzer
private static readonly DiagnosticDescriptor Rule = new(
id: Id,
title: "Prefer implicitly typed local variables",
description: "Use var to declare local variables for better readability and efficiency, except for built-in numeric types where explicit typing prevents potential conversion issues.",
messageFormat: "Use explicit type instead of 'var' for numeric variables",
category: "Style",
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: "Use var to declare local variables for better readability and efficiency, except for built-in numeric types where explicit typing prevents potential conversion issues.",
helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/{Id}.md");

/// <inheritdoc />
Expand Down
95 changes: 95 additions & 0 deletions src/EffectiveCSharp.Analyzers/ReplaceStringFormatAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
using System.Text.RegularExpressions;

namespace EffectiveCSharp.Analyzers;

/// <summary>
/// A <see cref="DiagnosticAnalyzer"/> for Effective C# Item #4 - Replace string.Format with interpolated string.
/// </summary>
/// <seealso cref="Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer" />
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class ReplaceStringFormatAnalyzer : DiagnosticAnalyzer
{
private const string Category = "Style";
private const string Description = "Replace string.Format with interpolated string.";
private const string DiagnosticId = DiagnosticIds.ReplaceStringFormatWithInterpolatedString;
private const string MessageFormat = "Replace '{0}' with interpolated string";
private const string Title = "Replace string.Format with interpolated string";

// We can't use source generators
private static readonly Regex PlaceholderRegex = new(
@"\{.*?\}",
RegexOptions.Compiled,
TimeSpan.FromSeconds(1));

private static readonly DiagnosticDescriptor Rule = new(
DiagnosticId,
Title,
MessageFormat,
Category,
DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: Description,
helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers{ThisAssembly.GitCommitId}/docs/{DiagnosticId}.md");

/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

/// <inheritdoc/>
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterCompilationStartAction(compilationContext =>
{
CSharpParseOptions? parseOptions = compilationContext.Compilation.SyntaxTrees.FirstOrDefault()?.Options as CSharpParseOptions;
if (parseOptions != null && parseOptions.LanguageVersion < LanguageVersion.CSharp10)
{
return;
}

compilationContext.RegisterSyntaxNodeAction(AnalyzeSyntaxNode, SyntaxKind.InvocationExpression);
});
}

private static void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context)
{
InvocationExpressionSyntax invocationExpr = (InvocationExpressionSyntax)context.Node;

SymbolInfo info = context.SemanticModel.GetSymbolInfo(invocationExpr, context.CancellationToken);
ISymbol? symbol = info.Symbol;

if (symbol is not { Name: "Format" }
|| !symbol.IsStatic
|| !symbol.ContainingType.IsString())
{
return;
}

SeparatedSyntaxList<ArgumentSyntax> argumentList = invocationExpr.ArgumentList.Arguments;

if (argumentList.Count < 2)
{
return;
}

if (argumentList[0].Expression is not LiteralExpressionSyntax formatArgument
|| !formatArgument.IsKind(SyntaxKind.StringLiteralExpression))
{
return;
}

string formatString = formatArgument.Token.ValueText;
if (!ContainsPlaceholders(formatString))
{
return;
}

Diagnostic diagnostic = invocationExpr.GetLocation().CreateDiagnostic(Rule, invocationExpr.ToString());
context.ReportDiagnostic(diagnostic);
}

private static bool ContainsPlaceholders(string formatString)
{
return PlaceholderRegex.IsMatch(formatString);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
using System.Text.RegularExpressions;

namespace EffectiveCSharp.Analyzers;

/// <summary>
/// A <see cref="CodeFixProvider"/> that provides a code fix for the <see cref="ReplaceStringFormatAnalyzer"/>.
/// </summary>
/// <seealso cref="Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider" />
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReplaceStringFormatCodeFixProvider))]
[Shared]
public class ReplaceStringFormatCodeFixProvider : CodeFixProvider
{
private const string Title = "Replace with interpolated string";

/// <inheritdoc />
public override sealed ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.ReplaceStringFormatWithInterpolatedString);

/// <inheritdoc />
public override sealed FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

/// <inheritdoc />
public override sealed async Task RegisterCodeFixesAsync(CodeFixContext context)
{
Diagnostic diagnostic = context.Diagnostics[0];
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;

SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
InvocationExpressionSyntax? invocationExpr = root?.FindNode(diagnosticSpan).FirstAncestorOrSelf<InvocationExpressionSyntax>();

context.RegisterCodeFix(
CodeAction.Create(
title: Title,
createChangedSolution: c => ReplaceWithInterpolatedStringAsync(context.Document, invocationExpr, c),
equivalenceKey: Title),
diagnostic);
}

private static bool NeedsParentheses(ExpressionSyntax expression)
{
// Check if the expression is complex and needs to be wrapped in parentheses
return expression is BinaryExpressionSyntax or ConditionalExpressionSyntax or AssignmentExpressionSyntax or CastExpressionSyntax;
}

private static string CreateInterpolatedString(string formatString, ArgumentSyntax[] arguments)
{
string result = formatString;

for (int i = 0; i < arguments.Length; i++)
{
string argumentText = arguments[i].ToString();

// Wrap in parentheses if the argument is a complex expression
if (NeedsParentheses(arguments[i].Expression))
{
argumentText = $"({argumentText})";
}

result = Regex.Replace(result, $@"\{{{i}(.*?)\}}", $"{{{argumentText}$1}}", RegexOptions.Compiled, TimeSpan.FromSeconds(1));
}

return $"$\"{result}\"";
}

private static async Task<Solution> ReplaceWithInterpolatedStringAsync(Document document, InvocationExpressionSyntax? invocationExpr, CancellationToken cancellationToken)
{
if (invocationExpr == null)
{
return document.Project.Solution;
}

LiteralExpressionSyntax? formatStringLiteral = invocationExpr.ArgumentList.Arguments.First().Expression as LiteralExpressionSyntax;
string? formatString = formatStringLiteral?.Token.ValueText;

if (string.IsNullOrEmpty(formatString))
{
return document.Project.Solution;
}

ArgumentSyntax[] arguments = invocationExpr.ArgumentList.Arguments.Skip(1).ToArray();

// Replace format placeholders with corresponding arguments in an interpolated string format
string interpolatedString = CreateInterpolatedString(formatString!, arguments);
SyntaxNode? root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
SyntaxNode? newRoot = root?.ReplaceNode(invocationExpr, SyntaxFactory.ParseExpression(interpolatedString));

return newRoot != null
? document.WithSyntaxRoot(newRoot).Project.Solution
: document.Project.Solution;
}
}
70 changes: 70 additions & 0 deletions tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0004Benchmarks.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
namespace EffectiveCSharp.Analyzers.Benchmarks;

[InProcess]
[MemoryDiagnoser]
public class Ecs0004Benchmarks
{
private static CompilationWithAnalyzers? BaselineCompilation { get; set; }

private static CompilationWithAnalyzers? TestCompilation { get; set; }

[IterationSetup]
[SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "Async setup not supported in BenchmarkDotNet.See https://github.com/dotnet/BenchmarkDotNet/issues/2442.")]
public static void SetupCompilation()
{
List<(string Name, string Content)> sources = [];
for (int index = 0; index < Constants.NumberOfCodeFiles; index++)
{
string name = $"TypeName{index}";
sources.Add((name, @$"
using System;

public class {name}
{{
public void Main()
{{
var str = string.Format(""The value of pi is {{0}}"", Math.PI.ToString(""F2""));
}}
}}
"));
}

(BaselineCompilation, TestCompilation) =
BenchmarkCSharpCompilationFactory
.CreateAsync<ReplaceStringFormatAnalyzer>(sources.ToArray())
.GetAwaiter()
.GetResult();
}

[Benchmark]
public async Task Ecs0004WithDiagnostics()
{
ImmutableArray<Diagnostic> diagnostics =
(await TestCompilation!
.GetAnalysisResultAsync(CancellationToken.None)
.ConfigureAwait(false))
.AssertValidAnalysisResult()
.GetAllDiagnostics();

if (diagnostics.Length != Constants.NumberOfCodeFiles)
{
throw new InvalidOperationException($"Expected '{Constants.NumberOfCodeFiles:N0}' analyzer diagnostics but found '{diagnostics.Length}'");
}
}

[Benchmark(Baseline = true)]
public async Task Ecs0004Baseline()
{
ImmutableArray<Diagnostic> diagnostics =
(await BaselineCompilation!
.GetAnalysisResultAsync(CancellationToken.None)
.ConfigureAwait(false))
.AssertValidAnalysisResult()
.GetAllDiagnostics();

if (diagnostics.Length != 0)
{
throw new InvalidOperationException($"Expected no analyzer diagnostics but found '{diagnostics.Length}'");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,6 @@ internal static class ReferenceAssemblyCatalog
{ Net80, ReferenceAssemblies.Net.Net80 },
{ Net90, ReferenceAssemblies.Net.Net90 },
};

public static IReadOnlySet<string> DotNetCore { get; } = new HashSet<string>([Net60, Net80, Net90], StringComparer.Ordinal);
}
Loading
Loading