Skip to content

Commit d02078f

Browse files
stephentoubstephentoub
authored
Add PreferTypedStringBuilderAppendOverloads analyzer + fixer (#3443)
1 parent 78dd233 commit d02078f

19 files changed

+722
-1
lines changed

‎src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ Rule ID | Category | Severity | Notes
44
CA1021 | Design | Disabled | AvoidOutParameters, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca1021)
55
CA1045 | Design | Disabled | DoNotPassTypesByReference, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca1045)
66
CA1069 | Design | Info | EnumShouldNotHaveDuplicatedValues, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca1069)
7+
CA1830 | Performance | Info | PreferTypedStringBuilderAppendOverloads, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca1830)
78
CA2011 | Reliability | Info | AvoidInfiniteRecursion, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2011)
89
CA2012 | Reliability | Hidden | UseValueTasksCorrectlyAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2012)
910
CA2013 | Reliability | Warning | DoNotUseReferenceEqualsWithValueTypesAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2013)

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

+12
Original file line numberDiff line numberDiff line change
@@ -1251,4 +1251,16 @@
12511251
<data name="DoNotUseStackallocInLoopsMessage" xml:space="preserve">
12521252
<value>Potential stack overflow. Move the stackalloc out of the loop.</value>
12531253
</data>
1254+
<data name="PreferTypedStringBuilderAppendOverloadsTitle" xml:space="preserve">
1255+
<value>Prefer strongly-typed Append and Insert method overloads on StringBuilder.</value>
1256+
</data>
1257+
<data name="PreferTypedStringBuilderAppendOverloadsDescription" xml:space="preserve">
1258+
<value>StringBuilder.Append and StringBuilder.Insert provide overloads for multiple types beyond System.String. When possible, prefer the strongly-typed overloads over using ToString() and the string-based overload.</value>
1259+
</data>
1260+
<data name="PreferTypedStringBuilderAppendOverloadsMessage" xml:space="preserve">
1261+
<value>Remove the ToString call in order to use a strongly-typed StringBuilder overload.</value>
1262+
</data>
1263+
<data name="PreferTypedStringBuilderAppendOverloadsRemoveToString" xml:space="preserve">
1264+
<value>Remove the ToString call</value>
1265+
</data>
12541266
</root>

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/PreferTypedStringBuilderAppendOverloads.Fixer.cs

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
2+
3+
using System;
4+
using System.Collections.Immutable;
5+
using System.Composition;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Analyzer.Utilities;
9+
using Analyzer.Utilities.Extensions;
10+
using Microsoft.CodeAnalysis;
11+
using Microsoft.CodeAnalysis.CodeFixes;
12+
using Microsoft.CodeAnalysis.Editing;
13+
using Microsoft.CodeAnalysis.Operations;
14+
15+
namespace Microsoft.NetCore.Analyzers.Runtime
16+
{
17+
/// <summary>CA1830: Prefer strongly-typed StringBuilder.Append overloads.</summary>
18+
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
19+
public sealed class PreferTypedStringBuilderAppendOverloadsFixer : CodeFixProvider
20+
{
21+
public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(PreferTypedStringBuilderAppendOverloads.RuleId);
22+
23+
public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
24+
25+
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
26+
{
27+
Document doc = context.Document;
28+
CancellationToken cancellationToken = context.CancellationToken;
29+
SyntaxNode root = await doc.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
30+
if (root.FindNode(context.Span) is SyntaxNode expression)
31+
{
32+
string title = MicrosoftNetCoreAnalyzersResources.PreferTypedStringBuilderAppendOverloadsRemoveToString;
33+
context.RegisterCodeFix(
34+
new MyCodeAction(title,
35+
async ct =>
36+
{
37+
SemanticModel model = await doc.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
38+
if (model.GetOperationWalkingUpParentChain(expression, cancellationToken) is IArgumentOperation arg &&
39+
arg.Value is IInvocationOperation invoke &&
40+
invoke.Instance?.Syntax is SyntaxNode replacement)
41+
{
42+
DocumentEditor editor = await DocumentEditor.CreateAsync(doc, ct).ConfigureAwait(false);
43+
editor.ReplaceNode(expression, editor.Generator.Argument(replacement));
44+
return editor.GetChangedDocument();
45+
}
46+
47+
return doc;
48+
},
49+
equivalenceKey: title),
50+
context.Diagnostics);
51+
}
52+
}
53+
54+
// Needed for Telemetry (https://github.com/dotnet/roslyn-analyzers/issues/192)
55+
private sealed class MyCodeAction : DocumentChangeAction
56+
{
57+
public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument, string equivalenceKey) :
58+
base(title, createChangedDocument, equivalenceKey)
59+
{
60+
}
61+
}
62+
}
63+
}

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/PreferTypedStringBuilderAppendOverloads.cs

+121
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
2+
3+
using System.Collections.Immutable;
4+
using System.Linq;
5+
using Analyzer.Utilities;
6+
using Analyzer.Utilities.Extensions;
7+
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.Diagnostics;
9+
using Microsoft.CodeAnalysis.Operations;
10+
11+
namespace Microsoft.NetCore.Analyzers.Runtime
12+
{
13+
/// <summary>CA1830: Prefer strongly-typed StringBuilder.Append overloads.</summary>
14+
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
15+
public sealed class PreferTypedStringBuilderAppendOverloads : DiagnosticAnalyzer
16+
{
17+
internal const string RuleId = "CA1830";
18+
19+
internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(RuleId,
20+
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.PreferTypedStringBuilderAppendOverloadsTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
21+
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.PreferTypedStringBuilderAppendOverloadsMessage), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
22+
DiagnosticCategory.Performance,
23+
RuleLevel.IdeSuggestion,
24+
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.PreferTypedStringBuilderAppendOverloadsDescription), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
25+
isPortedFxCopRule: false,
26+
isDataflowRule: false);
27+
28+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
29+
30+
public sealed override void Initialize(AnalysisContext context)
31+
{
32+
context.EnableConcurrentExecution();
33+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
34+
context.RegisterCompilationStartAction(compilationContext =>
35+
{
36+
var typeProvider = WellKnownTypeProvider.GetOrCreate(compilationContext.Compilation);
37+
38+
// Get the String and StringBuilder types.
39+
if (!typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextStringBuilder, out var stringBuilderType))
40+
{
41+
return;
42+
}
43+
44+
// Get all of the one-arg StringBuilder.Append methods and two-arg Insert methods. We skip over the object-based
45+
// overloads, as there's little benefit to removing such ToString calls. And we skip over arrays, to avoid
46+
// differences of behavior between Append(char[]) and Append(char[].ToString()).
47+
var appendMethods = stringBuilderType
48+
.GetMembers("Append")
49+
.OfType<IMethodSymbol>()
50+
.WhereAsArray(s =>
51+
s.Parameters.Length == 1 &&
52+
s.Parameters[0].Type.SpecialType != SpecialType.System_Object &&
53+
s.Parameters[0].Type.TypeKind != TypeKind.Array);
54+
var insertMethods = stringBuilderType
55+
.GetMembers("Insert")
56+
.OfType<IMethodSymbol>()
57+
.WhereAsArray(s =>
58+
s.Parameters.Length == 2 &&
59+
s.Parameters[0].Type.SpecialType == SpecialType.System_Int32 &&
60+
s.Parameters[1].Type.SpecialType != SpecialType.System_Object &&
61+
s.Parameters[1].Type.TypeKind != TypeKind.Array);
62+
63+
// Get the StringBuilder.Append(string)/Insert(int, string) method, for comparison purposes.
64+
var appendStringMethod = appendMethods.FirstOrDefault(s =>
65+
s.Parameters[0].Type.SpecialType == SpecialType.System_String);
66+
var insertStringMethod = insertMethods.FirstOrDefault(s =>
67+
s.Parameters[0].Type.SpecialType == SpecialType.System_Int32 &&
68+
s.Parameters[1].Type.SpecialType == SpecialType.System_String);
69+
if (appendStringMethod is null || insertStringMethod is null)
70+
{
71+
return;
72+
}
73+
74+
// Check every invocation to see if it's StringBuilder.Append(string)/Insert(int, string).
75+
compilationContext.RegisterOperationAction(operationContext =>
76+
{
77+
var invocation = (IInvocationOperation)operationContext.Operation;
78+
79+
// We're interested in the string argument to Append(string) and Insert(int, string).
80+
// Get the argument position, or bail if this isn't either method.
81+
int stringParamIndex;
82+
if (appendStringMethod.Equals(invocation.TargetMethod))
83+
{
84+
stringParamIndex = 0;
85+
}
86+
else if (insertStringMethod.Equals(invocation.TargetMethod))
87+
{
88+
stringParamIndex = 1;
89+
}
90+
else
91+
{
92+
return;
93+
}
94+
95+
// We're only interested if the string argument is a "string ToString()" call.
96+
if (invocation.Arguments.Length != stringParamIndex + 1 ||
97+
!(invocation.Arguments[stringParamIndex] is IArgumentOperation argument) ||
98+
!(argument.Value is IInvocationOperation toStringInvoke) ||
99+
toStringInvoke.TargetMethod.Name != "ToString" ||
100+
toStringInvoke.Type.SpecialType != SpecialType.System_String ||
101+
toStringInvoke.TargetMethod.Parameters.Length != 0)
102+
{
103+
return;
104+
}
105+
106+
// We're only interested if the receiver type of that ToString call has a corresponding strongly-typed overload.
107+
IMethodSymbol? stronglyTypedAppend =
108+
(stringParamIndex == 0 ? appendMethods : insertMethods)
109+
.FirstOrDefault(s => s.Parameters[stringParamIndex].Type.Equals(toStringInvoke.TargetMethod.ReceiverType));
110+
if (stronglyTypedAppend is null)
111+
{
112+
return;
113+
}
114+
115+
// Warn.
116+
operationContext.ReportDiagnostic(toStringInvoke.CreateDiagnostic(Rule));
117+
}, OperationKind.Invocation);
118+
});
119+
}
120+
}
121+
}

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf

+20
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,26 @@
13471347
<target state="translated">Metody P/Invoke nemají být viditelné</target>
13481348
<note />
13491349
</trans-unit>
1350+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsDescription">
1351+
<source>StringBuilder.Append and StringBuilder.Insert provide overloads for multiple types beyond System.String. When possible, prefer the strongly-typed overloads over using ToString() and the string-based overload.</source>
1352+
<target state="new">StringBuilder.Append and StringBuilder.Insert provide overloads for multiple types beyond System.String. When possible, prefer the strongly-typed overloads over using ToString() and the string-based overload.</target>
1353+
<note />
1354+
</trans-unit>
1355+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsMessage">
1356+
<source>Remove the ToString call in order to use a strongly-typed StringBuilder overload.</source>
1357+
<target state="new">Remove the ToString call in order to use a strongly-typed StringBuilder overload.</target>
1358+
<note />
1359+
</trans-unit>
1360+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsRemoveToString">
1361+
<source>Remove the ToString call</source>
1362+
<target state="new">Remove the ToString call</target>
1363+
<note />
1364+
</trans-unit>
1365+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsTitle">
1366+
<source>Prefer strongly-typed Append and Insert method overloads on StringBuilder.</source>
1367+
<target state="new">Prefer strongly-typed Append and Insert method overloads on StringBuilder.</target>
1368+
<note />
1369+
</trans-unit>
13501370
<trans-unit id="ProvideCorrectArgumentsToFormattingMethodsDescription">
13511371
<source>The format argument that is passed to System.String.Format does not contain a format item that corresponds to each object argument, or vice versa.</source>
13521372
<target state="translated">Argument formátu, který se předává do System.String.Format, neobsahuje položku formátování, která odpovídá jednotlivým argumentům objektů, nebo naopak.</target>

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf

+20
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,26 @@
13471347
<target state="translated">P/Invokes dürfen nicht sichtbar sein</target>
13481348
<note />
13491349
</trans-unit>
1350+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsDescription">
1351+
<source>StringBuilder.Append and StringBuilder.Insert provide overloads for multiple types beyond System.String. When possible, prefer the strongly-typed overloads over using ToString() and the string-based overload.</source>
1352+
<target state="new">StringBuilder.Append and StringBuilder.Insert provide overloads for multiple types beyond System.String. When possible, prefer the strongly-typed overloads over using ToString() and the string-based overload.</target>
1353+
<note />
1354+
</trans-unit>
1355+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsMessage">
1356+
<source>Remove the ToString call in order to use a strongly-typed StringBuilder overload.</source>
1357+
<target state="new">Remove the ToString call in order to use a strongly-typed StringBuilder overload.</target>
1358+
<note />
1359+
</trans-unit>
1360+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsRemoveToString">
1361+
<source>Remove the ToString call</source>
1362+
<target state="new">Remove the ToString call</target>
1363+
<note />
1364+
</trans-unit>
1365+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsTitle">
1366+
<source>Prefer strongly-typed Append and Insert method overloads on StringBuilder.</source>
1367+
<target state="new">Prefer strongly-typed Append and Insert method overloads on StringBuilder.</target>
1368+
<note />
1369+
</trans-unit>
13501370
<trans-unit id="ProvideCorrectArgumentsToFormattingMethodsDescription">
13511371
<source>The format argument that is passed to System.String.Format does not contain a format item that corresponds to each object argument, or vice versa.</source>
13521372
<target state="translated">Das an "System.String.Format" übergebene Formatargument enthält kein Formatelement, das den einzelnen Objektargumenten entspricht bzw. umgekehrt.</target>

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf

+20
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,26 @@
13471347
<target state="translated">Los elementos P/Invoke no deben estar visibles</target>
13481348
<note />
13491349
</trans-unit>
1350+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsDescription">
1351+
<source>StringBuilder.Append and StringBuilder.Insert provide overloads for multiple types beyond System.String. When possible, prefer the strongly-typed overloads over using ToString() and the string-based overload.</source>
1352+
<target state="new">StringBuilder.Append and StringBuilder.Insert provide overloads for multiple types beyond System.String. When possible, prefer the strongly-typed overloads over using ToString() and the string-based overload.</target>
1353+
<note />
1354+
</trans-unit>
1355+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsMessage">
1356+
<source>Remove the ToString call in order to use a strongly-typed StringBuilder overload.</source>
1357+
<target state="new">Remove the ToString call in order to use a strongly-typed StringBuilder overload.</target>
1358+
<note />
1359+
</trans-unit>
1360+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsRemoveToString">
1361+
<source>Remove the ToString call</source>
1362+
<target state="new">Remove the ToString call</target>
1363+
<note />
1364+
</trans-unit>
1365+
<trans-unit id="PreferTypedStringBuilderAppendOverloadsTitle">
1366+
<source>Prefer strongly-typed Append and Insert method overloads on StringBuilder.</source>
1367+
<target state="new">Prefer strongly-typed Append and Insert method overloads on StringBuilder.</target>
1368+
<note />
1369+
</trans-unit>
13501370
<trans-unit id="ProvideCorrectArgumentsToFormattingMethodsDescription">
13511371
<source>The format argument that is passed to System.String.Format does not contain a format item that corresponds to each object argument, or vice versa.</source>
13521372
<target state="translated">El argumento de cadena format pasado a System.String.Format no contiene un elemento de formato que corresponda a cada argumento de objeto o viceversa.</target>

0 commit comments

Comments
 (0)