Skip to content

Commit f3ec3ca

Browse files
Copilotbcollamore
andcommitted
refactor: Improve PreferInterpolatedStringAnalyzer API design and performance
Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>
1 parent bd1bd13 commit f3ec3ca

2 files changed

Lines changed: 28 additions & 23 deletions

File tree

Philips.CodeAnalysis.MaintainabilityAnalyzers/Readability/PreferInterpolatedStringAnalyzer.cs

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// © 2019 Koninklijke Philips N.V. See License.md in the project root for license information.
22

3+
using System;
34
using System.Collections.Immutable;
45
using System.Globalization;
56
using Microsoft.CodeAnalysis;
@@ -12,6 +13,13 @@ namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Readability
1213
[DiagnosticAnalyzer(LanguageNames.CSharp)]
1314
public class PreferInterpolatedStringAnalyzer : DiagnosticAnalyzerBase
1415
{
16+
[Flags]
17+
private enum ConversionResult
18+
{
19+
None = 0,
20+
CanConvert = 1,
21+
IsUnnecessary = 2
22+
}
1523
private const string Title = @"Prefer interpolated strings over string.Format";
1624
private const string MessageFormat = @"Replace string.Format with interpolated string for better readability";
1725
private const string Description = @"Interpolated strings are more readable and less error prone than string.Format";
@@ -56,13 +64,14 @@ private void OnInvocation(OperationAnalysisContext operationContext)
5664
return;
5765
}
5866

59-
if (!CanConvertToInterpolatedString(invocation, out var isUnnecessary))
67+
ConversionResult result = CanConvertToInterpolatedString(invocation);
68+
if (result == ConversionResult.None)
6069
{
6170
return;
6271
}
6372

6473
Location location = invocation.Syntax.GetLocation();
65-
if (isUnnecessary)
74+
if (result.HasFlag(ConversionResult.IsUnnecessary))
6675
{
6776
operationContext.ReportDiagnostic(Diagnostic.Create(UnnecessaryRule, location));
6877
}
@@ -79,50 +88,41 @@ private bool IsStringFormatMethod(IMethodSymbol targetMethod)
7988
targetMethod.ContainingType.SpecialType == SpecialType.System_String;
8089
}
8190

82-
private bool CanConvertToInterpolatedString(IInvocationOperation invocation, out bool isUnnecessary)
91+
private ConversionResult CanConvertToInterpolatedString(IInvocationOperation invocation)
8392
{
84-
isUnnecessary = false;
8593
if (invocation.Arguments.Length < 1)
8694
{
87-
return false;
95+
return ConversionResult.None;
8896
}
8997

9098
IOperation formatStringArgument = invocation.Arguments[0].Value;
9199

92100
if (formatStringArgument.Kind != OperationKind.Literal ||
93101
formatStringArgument.Type?.SpecialType != SpecialType.System_String)
94102
{
95-
return false;
103+
return ConversionResult.None;
96104
}
97105

98106
var formatString = (string)formatStringArgument.ConstantValue.Value;
99107

100-
if (formatString.Contains(":"))
101-
{
102-
return false;
103-
}
104-
105108
// Parse the format string to find actual placeholders, ignoring escaped braces
106109
var placeholderCount = CountFormatPlaceholders(formatString);
107-
var hasAnyBraces = formatString.Contains("{") || formatString.Contains("}");
108110

109111
if (placeholderCount == 0)
110112
{
111113
// Only flag as unnecessary if there are no braces at all
112114
// If there are escaped braces, string.Format is needed to produce literal braces
115+
var hasAnyBraces = formatString.Contains("{") || formatString.Contains("}");
113116
if (!hasAnyBraces)
114117
{
115-
isUnnecessary = true;
116-
return true;
117-
}
118-
else
119-
{
120-
// Has escaped braces but no placeholders - don't suggest conversion
121-
return false;
118+
return ConversionResult.CanConvert | ConversionResult.IsUnnecessary;
122119
}
120+
121+
// Has escaped braces but no placeholders - don't suggest conversion
122+
return ConversionResult.None;
123123
}
124124

125-
return invocation.Arguments.Length > 1;
125+
return invocation.Arguments.Length > 1 ? ConversionResult.CanConvert : ConversionResult.None;
126126
}
127127

128128
private int CountFormatPlaceholders(string formatString)
@@ -152,7 +152,12 @@ private int CountFormatPlaceholders(string formatString)
152152
{
153153
// Found valid placeholder, check if it's a simple numeric placeholder
154154
var content = formatString.Substring(i + 1, j - i - 1);
155-
if (int.TryParse(content.Trim(), NumberStyles.Integer, CultureInfo.InvariantCulture, out _))
155+
156+
// Handle format specifiers like {0:N2} by splitting on ':'
157+
var colonIndex = content.IndexOf(':');
158+
var indexPart = colonIndex >= 0 ? content.Substring(0, colonIndex) : content;
159+
160+
if (int.TryParse(indexPart.Trim(), NumberStyles.Integer, CultureInfo.InvariantCulture, out _))
156161
{
157162
placeholderCount++;
158163
}

Philips.CodeAnalysis.Test/Maintainability/Readability/PreferInterpolatedStringAnalyzerTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public void Test()
5353

5454
[TestMethod]
5555
[TestCategory(TestDefinitions.UnitTests)]
56-
public async Task NoWarningForStringWithFormatSpecifiers()
56+
public async Task WarningForStringWithFormatSpecifiers()
5757
{
5858
const string input = @"
5959
class Foo
@@ -65,7 +65,7 @@ public void Test()
6565
}
6666
}";
6767

68-
await VerifySuccessfulCompilation(input).ConfigureAwait(false);
68+
await VerifyDiagnostic(input, DiagnosticId.PreferInterpolatedString).ConfigureAwait(false);
6969
}
7070

7171
[TestMethod]

0 commit comments

Comments
 (0)