Skip to content

Commit 8011355

Browse files
MihaZupanMihaZupan
authored
Add analyzer/fixer to suggest using cached SearchValues instances (#6898)
* Initial Analyzer implementation * Code fixer * Add support for string.IndexOfAny(char[]) * Catch simple cases of array element modification * Use built-in helper instead of Linq * Also detect field assignments in ctor * Move system namespace import to helper method * Replace array creations wtih string literals * Add support for more property getter patterns * Simplify test helper * Revert Utf8String support :( * Update tests * msbuild /t:pack /v:m * Fix editor renaming * Exclude string uses on a conditional access * Add test for array field with const char reference * Add back Utf8String support * Update messages/descriptions * Add support for field initialized from literal.ToCharArray * More tests for ToCharArray * Better handle member names that start with _ * Avoid some duplication between Syntax and Operation analysis * Fix top-level statements and add logic to remove unused members * ImmutableHashSet, no OfType * Remove some duplication * Turn one analyzer test into code fixer tests * Shorten analyzer title
1 parent e5959ba commit 8011355

29 files changed

+2480
-2
lines changed

‎src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Performance/CSharpUseSearchValues.Fixer.cs‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
2+
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Diagnostics;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Analyzer.Utilities.Extensions;
9+
using Microsoft.CodeAnalysis;
10+
using Microsoft.CodeAnalysis.CodeActions;
11+
using Microsoft.CodeAnalysis.CodeFixes;
12+
using Microsoft.CodeAnalysis.CSharp;
13+
using Microsoft.CodeAnalysis.CSharp.Syntax;
14+
using Microsoft.CodeAnalysis.Operations;
15+
using Microsoft.NetCore.Analyzers.Performance;
16+
17+
namespace Microsoft.NetCore.CSharp.Analyzers.Performance
18+
{
19+
/// <inheritdoc/>
20+
[ExportCodeFixProvider(LanguageNames.CSharp)]
21+
public sealed class CSharpUseSearchValuesFixer : UseSearchValuesFixer
22+
{
23+
protected override async ValueTask<(SyntaxNode TypeDeclaration, INamedTypeSymbol? TypeSymbol, bool IsRealType)> GetTypeSymbolAsync(SemanticModel semanticModel, SyntaxNode node, CancellationToken cancellationToken)
24+
{
25+
SyntaxNode? typeDeclarationOrCompilationUnit = node.FirstAncestorOrSelf<TypeDeclarationSyntax>();
26+
27+
typeDeclarationOrCompilationUnit ??= await node.SyntaxTree.GetRootAsync(cancellationToken).ConfigureAwait(false);
28+
29+
return typeDeclarationOrCompilationUnit is TypeDeclarationSyntax typeDeclaration
30+
? (typeDeclaration, semanticModel.GetDeclaredSymbol(typeDeclaration, cancellationToken), IsRealType: true)
31+
: (typeDeclarationOrCompilationUnit, semanticModel.GetDeclaredSymbol((CompilationUnitSyntax)typeDeclarationOrCompilationUnit, cancellationToken)?.ContainingType, IsRealType: false);
32+
}
33+
34+
protected override SyntaxNode ReplaceSearchValuesFieldName(SyntaxNode node)
35+
{
36+
if (node is FieldDeclarationSyntax fieldDeclaration &&
37+
fieldDeclaration.Declaration is { } declaration &&
38+
declaration.Variables is [var declarator])
39+
{
40+
var newDeclarator = declarator.ReplaceToken(declarator.Identifier, declarator.Identifier.WithAdditionalAnnotations(RenameAnnotation.Create()));
41+
return fieldDeclaration.WithDeclaration(declaration.WithVariables(new SeparatedSyntaxList<VariableDeclaratorSyntax>().Add(newDeclarator)));
42+
}
43+
44+
return node;
45+
}
46+
47+
protected override SyntaxNode GetDeclaratorInitializer(SyntaxNode syntax)
48+
{
49+
if (syntax is VariableDeclaratorSyntax variableDeclarator)
50+
{
51+
return variableDeclarator.Initializer!.Value;
52+
}
53+
54+
if (syntax is PropertyDeclarationSyntax propertyDeclaration)
55+
{
56+
return CSharpUseSearchValuesAnalyzer.TryGetPropertyGetterExpression(propertyDeclaration)!;
57+
}
58+
59+
throw new InvalidOperationException($"Expected 'VariableDeclaratorSyntax' or 'PropertyDeclarationSyntax', got {syntax.GetType().Name}");
60+
}
61+
62+
// new[] { 'a', 'b', 'c' } => "abc"
63+
// new[] { (byte)'a', (byte)'b', (byte)'c' } => "abc"u8
64+
// "abc".ToCharArray() => "abc"
65+
protected override SyntaxNode? TryReplaceArrayCreationWithInlineLiteralExpression(IOperation operation)
66+
{
67+
if (operation is IConversionOperation conversion)
68+
{
69+
operation = conversion.Operand;
70+
}
71+
72+
if (operation is IArrayCreationOperation arrayCreation &&
73+
arrayCreation.GetElementType() is { } elementType)
74+
{
75+
bool isByte = elementType.SpecialType == SpecialType.System_Byte;
76+
77+
if (isByte &&
78+
(operation.SemanticModel?.Compilation is not CSharpCompilation compilation ||
79+
compilation.LanguageVersion < (LanguageVersion)1100)) // LanguageVersion.CSharp11
80+
{
81+
// Can't use Utf8StringLiterals
82+
return null;
83+
}
84+
85+
List<char> values = new();
86+
87+
if (arrayCreation.Syntax is ExpressionSyntax creationSyntax &&
88+
CSharpUseSearchValuesAnalyzer.IsConstantByteOrCharArrayCreationExpression(operation.SemanticModel!, creationSyntax, values, out _) &&
89+
values.Count <= 128 && // Arbitrary limit to avoid emitting huge literals
90+
!ContainsAnyComments(creationSyntax)) // Avoid removing potentially valuable comments
91+
{
92+
string valuesString = string.Concat(values);
93+
string stringLiteral = SymbolDisplay.FormatLiteral(valuesString, quote: true);
94+
95+
const SyntaxKind Utf8StringLiteralExpression = (SyntaxKind)8756;
96+
const SyntaxKind Utf8StringLiteralToken = (SyntaxKind)8520;
97+
98+
return SyntaxFactory.LiteralExpression(
99+
isByte ? Utf8StringLiteralExpression : SyntaxKind.StringLiteralExpression,
100+
SyntaxFactory.Token(
101+
leading: default,
102+
kind: isByte ? Utf8StringLiteralToken : SyntaxKind.StringLiteralToken,
103+
text: isByte ? $"{stringLiteral}u8" : stringLiteral,
104+
valueText: valuesString,
105+
trailing: default));
106+
}
107+
}
108+
else if (operation is IInvocationOperation invocation)
109+
{
110+
if (UseSearchValuesAnalyzer.IsConstantStringToCharArrayInvocation(invocation, out _))
111+
{
112+
Debug.Assert(invocation.Instance is not null);
113+
return invocation.Instance!.Syntax;
114+
}
115+
}
116+
117+
return null;
118+
}
119+
120+
private static bool ContainsAnyComments(SyntaxNode node)
121+
{
122+
foreach (SyntaxTrivia trivia in node.DescendantTrivia(node.Span))
123+
{
124+
if (trivia.Kind() is SyntaxKind.SingleLineCommentTrivia or SyntaxKind.MultiLineCommentTrivia)
125+
{
126+
return true;
127+
}
128+
}
129+
130+
return false;
131+
}
132+
}
133+
}

‎src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Performance/CSharpUseSearchValues.cs‎

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
2+
3+
using System.Collections.Generic;
4+
using Microsoft.CodeAnalysis;
5+
using Microsoft.CodeAnalysis.CSharp;
6+
using Microsoft.CodeAnalysis.CSharp.Syntax;
7+
using Microsoft.CodeAnalysis.Diagnostics;
8+
using Microsoft.CodeAnalysis.Operations;
9+
using Microsoft.NetCore.Analyzers.Performance;
10+
11+
namespace Microsoft.NetCore.CSharp.Analyzers.Performance
12+
{
13+
/// <inheritdoc/>
14+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
15+
public sealed class CSharpUseSearchValuesAnalyzer : UseSearchValuesAnalyzer
16+
{
17+
// char[] myField = new char[] { 'a', 'b', 'c' };
18+
// char[] myField = new[] { 'a', 'b', 'c' };
19+
// char[] myField = "abc".ToCharArray();
20+
// char[] myField = ConstString.ToCharArray();
21+
// byte[] myField = new[] { (byte)'a', (byte)'b', (byte)'c' };
22+
protected override bool IsConstantByteOrCharArrayVariableDeclaratorSyntax(SemanticModel semanticModel, SyntaxNode syntax, out int length)
23+
{
24+
length = 0;
25+
26+
return
27+
syntax is VariableDeclaratorSyntax variableDeclarator &&
28+
variableDeclarator.Initializer?.Value is { } initializer &&
29+
IsConstantByteOrCharArrayCreationExpression(semanticModel, initializer, values: null, out length);
30+
}
31+
32+
// ReadOnlySpan<char> myProperty => new char[] { 'a', 'b', 'c' };
33+
// ReadOnlySpan<char> myProperty => new[] { 'a', 'b', 'c' };
34+
// ReadOnlySpan<char> myProperty => "abc".ToCharArray();
35+
// ReadOnlySpan<char> myProperty => ConstString.ToCharArray();
36+
// ReadOnlySpan<byte> myProperty => new[] { (byte)'a', (byte)'b', (byte)'c' };
37+
// ReadOnlySpan<byte> myProperty => "abc"u8;
38+
// ReadOnlySpan<byte> myProperty { get => "abc"u8; }
39+
// ReadOnlySpan<byte> myProperty { get { return "abc"u8; } }
40+
protected override bool IsConstantByteOrCharReadOnlySpanPropertyDeclarationSyntax(SemanticModel semanticModel, SyntaxNode syntax, out int length)
41+
{
42+
length = 0;
43+
44+
return
45+
syntax is PropertyDeclarationSyntax propertyDeclaration &&
46+
TryGetPropertyGetterExpression(propertyDeclaration) is { } expression &&
47+
(IsConstantByteOrCharArrayCreationExpression(semanticModel, expression, values: null, out length) || IsUtf8StringLiteralExpression(expression, out length));
48+
}
49+
50+
protected override bool IsConstantByteOrCharArrayCreationSyntax(SemanticModel semanticModel, SyntaxNode syntax, out int length)
51+
{
52+
length = 0;
53+
54+
return
55+
syntax is ExpressionSyntax expression &&
56+
IsConstantByteOrCharArrayCreationExpression(semanticModel, expression, values: null, out length);
57+
}
58+
59+
internal static ExpressionSyntax? TryGetPropertyGetterExpression(PropertyDeclarationSyntax propertyDeclaration)
60+
{
61+
var expression = propertyDeclaration.ExpressionBody?.Expression;
62+
63+
if (expression is null &&
64+
propertyDeclaration.AccessorList?.Accessors is [var accessor] &&
65+
accessor.IsKind(SyntaxKind.GetAccessorDeclaration))
66+
{
67+
expression = accessor.ExpressionBody?.Expression;
68+
69+
if (expression is null &&
70+
accessor.Body?.Statements is [var statement] &&
71+
statement is ReturnStatementSyntax returnStatement)
72+
{
73+
expression = returnStatement.Expression;
74+
}
75+
}
76+
77+
return expression;
78+
}
79+
80+
// new char[] { 'a', 'b', 'c' };
81+
// new[] { 'a', 'b', 'c' };
82+
// new[] { (byte)'a', (byte)'b', (byte)'c' };
83+
// "abc".ToCharArray()
84+
// ConstString.ToCharArray()
85+
internal static bool IsConstantByteOrCharArrayCreationExpression(SemanticModel semanticModel, ExpressionSyntax expression, List<char>? values, out int length)
86+
{
87+
length = 0;
88+
89+
InitializerExpressionSyntax? arrayInitializer = null;
90+
91+
if (expression is ArrayCreationExpressionSyntax arrayCreation)
92+
{
93+
arrayInitializer = arrayCreation.Initializer;
94+
}
95+
else if (expression is ImplicitArrayCreationExpressionSyntax implicitArrayCreation)
96+
{
97+
arrayInitializer = implicitArrayCreation.Initializer;
98+
}
99+
else if (expression is InvocationExpressionSyntax invocation)
100+
{
101+
if (semanticModel.GetOperation(invocation) is IInvocationOperation invocationOperation &&
102+
IsConstantStringToCharArrayInvocation(invocationOperation, out string? value))
103+
{
104+
values?.AddRange(value);
105+
length = value.Length;
106+
return true;
107+
}
108+
}
109+
110+
if (arrayInitializer?.Expressions is { } valueExpressions)
111+
{
112+
foreach (var valueExpression in valueExpressions)
113+
{
114+
if (!TryGetByteOrCharLiteral(valueExpression, out char value))
115+
{
116+
return false;
117+
}
118+
119+
values?.Add(value);
120+
}
121+
122+
length = valueExpressions.Count;
123+
return true;
124+
}
125+
126+
return false;
127+
128+
// 'a' or (byte)'a'
129+
static bool TryGetByteOrCharLiteral(ExpressionSyntax? expression, out char value)
130+
{
131+
if (expression is not null)
132+
{
133+
if (expression is CastExpressionSyntax cast &&
134+
cast.Type is PredefinedTypeSyntax predefinedType &&
135+
predefinedType.Keyword.IsKind(SyntaxKind.ByteKeyword))
136+
{
137+
expression = cast.Expression;
138+
}
139+
140+
if (expression.IsKind(SyntaxKind.CharacterLiteralExpression) &&
141+
expression is LiteralExpressionSyntax characterLiteral &&
142+
characterLiteral.Token.Value is char charValue)
143+
{
144+
value = charValue;
145+
return true;
146+
}
147+
}
148+
149+
value = default;
150+
return false;
151+
}
152+
}
153+
154+
private static bool IsUtf8StringLiteralExpression(ExpressionSyntax expression, out int length)
155+
{
156+
const SyntaxKind Utf8StringLiteralExpression = (SyntaxKind)8756;
157+
const SyntaxKind Utf8StringLiteralToken = (SyntaxKind)8520;
158+
159+
if (expression.IsKind(Utf8StringLiteralExpression) &&
160+
expression is LiteralExpressionSyntax literal &&
161+
literal.Token.IsKind(Utf8StringLiteralToken) &&
162+
literal.Token.Value is string value)
163+
{
164+
length = value.Length;
165+
return true;
166+
}
167+
168+
length = 0;
169+
return false;
170+
}
171+
172+
protected override bool ArrayFieldUsesAreLikelyReadOnly(SyntaxNode syntax)
173+
{
174+
if (syntax is not VariableDeclaratorSyntax variableDeclarator ||
175+
variableDeclarator.Identifier.Value is not string fieldName ||
176+
syntax.FirstAncestorOrSelf<TypeDeclarationSyntax>() is not { } typeDeclaration)
177+
{
178+
return false;
179+
}
180+
181+
// An optimistic implementation that only looks for simple assignments to the field or its array elements.
182+
foreach (var member in typeDeclaration.Members)
183+
{
184+
bool isCtor = member.IsKind(SyntaxKind.ConstructorDeclaration);
185+
186+
foreach (var node in member.DescendantNodes())
187+
{
188+
if (node.IsKind(SyntaxKind.SimpleAssignmentExpression) &&
189+
node is AssignmentExpressionSyntax assignment)
190+
{
191+
if (assignment.Left.IsKind(SyntaxKind.ElementAccessExpression))
192+
{
193+
if (assignment.Left is ElementAccessExpressionSyntax elementAccess &&
194+
IsFieldReference(elementAccess.Expression, fieldName))
195+
{
196+
// s_array[42] = foo;
197+
return false;
198+
}
199+
}
200+
else if (isCtor)
201+
{
202+
if (IsFieldReference(assignment.Left, fieldName))
203+
{
204+
// s_array = foo;
205+
return false;
206+
}
207+
}
208+
}
209+
}
210+
}
211+
212+
return true;
213+
214+
static bool IsFieldReference(ExpressionSyntax expression, string fieldName) =>
215+
expression.IsKind(SyntaxKind.IdentifierName) &&
216+
expression is IdentifierNameSyntax identifierName &&
217+
identifierName.Identifier.Value is string value &&
218+
value == fieldName;
219+
}
220+
}
221+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ CA1861 | Performance | Info | AvoidConstArrays, [Documentation](https://learn.mi
2323
CA1862 | Performance | Info | RecommendCaseInsensitiveStringComparison, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1862)
2424
CA1863 | Performance | Hidden | UseCompositeFormatAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1862)
2525
CA1864 | Performance | Info | PreferDictionaryTryAddValueOverGuardedAddAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1864)
26+
CA1870 | Performance | Info | UseSearchValuesAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1870)
2627
CA2021 | Reliability | Warning | DoNotCallEnumerableCastOrOfTypeWithIncompatibleTypesAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2021)
2728

2829
### Removed Rules

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,6 +1916,18 @@ Widening and user defined conversions are not supported with generic types.</val
19161916
<data name="UseSpanClearInsteadOfFillTitle" xml:space="preserve">
19171917
<value>Prefer 'Clear' over 'Fill'</value>
19181918
</data>
1919+
<data name="UseSearchValuesTitle" xml:space="preserve">
1920+
<value>Use a cached 'SearchValues' instance</value>
1921+
</data>
1922+
<data name="UseSearchValuesMessage" xml:space="preserve">
1923+
<value>Use a cached 'SearchValues' instance for improved searching performance</value>
1924+
</data>
1925+
<data name="UseSearchValuesDescription" xml:space="preserve">
1926+
<value>Using a cached 'SearchValues' instance is more efficient than passing values to 'IndexOfAny'/'ContainsAny' directly.</value>
1927+
</data>
1928+
<data name="UseSearchValuesCodeFixTitle" xml:space="preserve">
1929+
<value>Use 'SearchValues'</value>
1930+
</data>
19191931
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesDescription" xml:space="preserve">
19201932
<value>Some built-in operators added in .NET 7 behave differently when overflowing than did the corresponding user-defined operators in .NET 6 and earlier versions. Some operators that previously threw in an unchecked context now don't throw unless wrapped within a checked context. Also, some operators that did not previously throw in a checked context now throw unless wrapped in an unchecked context.</value>
19211933
</data>

0 commit comments

Comments
 (0)