Skip to content

Commit

Permalink
Add analyzer/fixer to suggest using cached SearchValues instances (#6898
Browse files Browse the repository at this point in the history
)

* 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
  • Loading branch information
MihaZupan authored Sep 8, 2023
1 parent e5959ba commit 8011355
Show file tree
Hide file tree
Showing 29 changed files with 2,480 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.NetCore.Analyzers.Performance;

namespace Microsoft.NetCore.CSharp.Analyzers.Performance
{
/// <inheritdoc/>
[ExportCodeFixProvider(LanguageNames.CSharp)]
public sealed class CSharpUseSearchValuesFixer : UseSearchValuesFixer
{
protected override async ValueTask<(SyntaxNode TypeDeclaration, INamedTypeSymbol? TypeSymbol, bool IsRealType)> GetTypeSymbolAsync(SemanticModel semanticModel, SyntaxNode node, CancellationToken cancellationToken)
{
SyntaxNode? typeDeclarationOrCompilationUnit = node.FirstAncestorOrSelf<TypeDeclarationSyntax>();

typeDeclarationOrCompilationUnit ??= await node.SyntaxTree.GetRootAsync(cancellationToken).ConfigureAwait(false);

return typeDeclarationOrCompilationUnit is TypeDeclarationSyntax typeDeclaration
? (typeDeclaration, semanticModel.GetDeclaredSymbol(typeDeclaration, cancellationToken), IsRealType: true)
: (typeDeclarationOrCompilationUnit, semanticModel.GetDeclaredSymbol((CompilationUnitSyntax)typeDeclarationOrCompilationUnit, cancellationToken)?.ContainingType, IsRealType: false);
}

protected override SyntaxNode ReplaceSearchValuesFieldName(SyntaxNode node)
{
if (node is FieldDeclarationSyntax fieldDeclaration &&
fieldDeclaration.Declaration is { } declaration &&
declaration.Variables is [var declarator])
{
var newDeclarator = declarator.ReplaceToken(declarator.Identifier, declarator.Identifier.WithAdditionalAnnotations(RenameAnnotation.Create()));
return fieldDeclaration.WithDeclaration(declaration.WithVariables(new SeparatedSyntaxList<VariableDeclaratorSyntax>().Add(newDeclarator)));
}

return node;
}

protected override SyntaxNode GetDeclaratorInitializer(SyntaxNode syntax)
{
if (syntax is VariableDeclaratorSyntax variableDeclarator)
{
return variableDeclarator.Initializer!.Value;
}

if (syntax is PropertyDeclarationSyntax propertyDeclaration)
{
return CSharpUseSearchValuesAnalyzer.TryGetPropertyGetterExpression(propertyDeclaration)!;
}

throw new InvalidOperationException($"Expected 'VariableDeclaratorSyntax' or 'PropertyDeclarationSyntax', got {syntax.GetType().Name}");
}

// new[] { 'a', 'b', 'c' } => "abc"
// new[] { (byte)'a', (byte)'b', (byte)'c' } => "abc"u8
// "abc".ToCharArray() => "abc"
protected override SyntaxNode? TryReplaceArrayCreationWithInlineLiteralExpression(IOperation operation)
{
if (operation is IConversionOperation conversion)
{
operation = conversion.Operand;
}

if (operation is IArrayCreationOperation arrayCreation &&
arrayCreation.GetElementType() is { } elementType)
{
bool isByte = elementType.SpecialType == SpecialType.System_Byte;

if (isByte &&
(operation.SemanticModel?.Compilation is not CSharpCompilation compilation ||
compilation.LanguageVersion < (LanguageVersion)1100)) // LanguageVersion.CSharp11
{
// Can't use Utf8StringLiterals
return null;
}

List<char> values = new();

if (arrayCreation.Syntax is ExpressionSyntax creationSyntax &&
CSharpUseSearchValuesAnalyzer.IsConstantByteOrCharArrayCreationExpression(operation.SemanticModel!, creationSyntax, values, out _) &&
values.Count <= 128 && // Arbitrary limit to avoid emitting huge literals
!ContainsAnyComments(creationSyntax)) // Avoid removing potentially valuable comments
{
string valuesString = string.Concat(values);
string stringLiteral = SymbolDisplay.FormatLiteral(valuesString, quote: true);

const SyntaxKind Utf8StringLiteralExpression = (SyntaxKind)8756;
const SyntaxKind Utf8StringLiteralToken = (SyntaxKind)8520;

return SyntaxFactory.LiteralExpression(
isByte ? Utf8StringLiteralExpression : SyntaxKind.StringLiteralExpression,
SyntaxFactory.Token(
leading: default,
kind: isByte ? Utf8StringLiteralToken : SyntaxKind.StringLiteralToken,
text: isByte ? $"{stringLiteral}u8" : stringLiteral,
valueText: valuesString,
trailing: default));
}
}
else if (operation is IInvocationOperation invocation)
{
if (UseSearchValuesAnalyzer.IsConstantStringToCharArrayInvocation(invocation, out _))
{
Debug.Assert(invocation.Instance is not null);
return invocation.Instance!.Syntax;
}
}

return null;
}

private static bool ContainsAnyComments(SyntaxNode node)
{
foreach (SyntaxTrivia trivia in node.DescendantTrivia(node.Span))
{
if (trivia.Kind() is SyntaxKind.SingleLineCommentTrivia or SyntaxKind.MultiLineCommentTrivia)
{
return true;
}
}

return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.NetCore.Analyzers.Performance;

namespace Microsoft.NetCore.CSharp.Analyzers.Performance
{
/// <inheritdoc/>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpUseSearchValuesAnalyzer : UseSearchValuesAnalyzer
{
// char[] myField = new char[] { 'a', 'b', 'c' };
// char[] myField = new[] { 'a', 'b', 'c' };
// char[] myField = "abc".ToCharArray();
// char[] myField = ConstString.ToCharArray();
// byte[] myField = new[] { (byte)'a', (byte)'b', (byte)'c' };
protected override bool IsConstantByteOrCharArrayVariableDeclaratorSyntax(SemanticModel semanticModel, SyntaxNode syntax, out int length)
{
length = 0;

return
syntax is VariableDeclaratorSyntax variableDeclarator &&
variableDeclarator.Initializer?.Value is { } initializer &&
IsConstantByteOrCharArrayCreationExpression(semanticModel, initializer, values: null, out length);
}

// ReadOnlySpan<char> myProperty => new char[] { 'a', 'b', 'c' };
// ReadOnlySpan<char> myProperty => new[] { 'a', 'b', 'c' };
// ReadOnlySpan<char> myProperty => "abc".ToCharArray();
// ReadOnlySpan<char> myProperty => ConstString.ToCharArray();
// ReadOnlySpan<byte> myProperty => new[] { (byte)'a', (byte)'b', (byte)'c' };
// ReadOnlySpan<byte> myProperty => "abc"u8;
// ReadOnlySpan<byte> myProperty { get => "abc"u8; }
// ReadOnlySpan<byte> myProperty { get { return "abc"u8; } }
protected override bool IsConstantByteOrCharReadOnlySpanPropertyDeclarationSyntax(SemanticModel semanticModel, SyntaxNode syntax, out int length)
{
length = 0;

return
syntax is PropertyDeclarationSyntax propertyDeclaration &&
TryGetPropertyGetterExpression(propertyDeclaration) is { } expression &&
(IsConstantByteOrCharArrayCreationExpression(semanticModel, expression, values: null, out length) || IsUtf8StringLiteralExpression(expression, out length));
}

protected override bool IsConstantByteOrCharArrayCreationSyntax(SemanticModel semanticModel, SyntaxNode syntax, out int length)
{
length = 0;

return
syntax is ExpressionSyntax expression &&
IsConstantByteOrCharArrayCreationExpression(semanticModel, expression, values: null, out length);
}

internal static ExpressionSyntax? TryGetPropertyGetterExpression(PropertyDeclarationSyntax propertyDeclaration)
{
var expression = propertyDeclaration.ExpressionBody?.Expression;

if (expression is null &&
propertyDeclaration.AccessorList?.Accessors is [var accessor] &&
accessor.IsKind(SyntaxKind.GetAccessorDeclaration))
{
expression = accessor.ExpressionBody?.Expression;

if (expression is null &&
accessor.Body?.Statements is [var statement] &&
statement is ReturnStatementSyntax returnStatement)
{
expression = returnStatement.Expression;
}
}

return expression;
}

// new char[] { 'a', 'b', 'c' };
// new[] { 'a', 'b', 'c' };
// new[] { (byte)'a', (byte)'b', (byte)'c' };
// "abc".ToCharArray()
// ConstString.ToCharArray()
internal static bool IsConstantByteOrCharArrayCreationExpression(SemanticModel semanticModel, ExpressionSyntax expression, List<char>? values, out int length)
{
length = 0;

InitializerExpressionSyntax? arrayInitializer = null;

if (expression is ArrayCreationExpressionSyntax arrayCreation)
{
arrayInitializer = arrayCreation.Initializer;
}
else if (expression is ImplicitArrayCreationExpressionSyntax implicitArrayCreation)
{
arrayInitializer = implicitArrayCreation.Initializer;
}
else if (expression is InvocationExpressionSyntax invocation)
{
if (semanticModel.GetOperation(invocation) is IInvocationOperation invocationOperation &&
IsConstantStringToCharArrayInvocation(invocationOperation, out string? value))
{
values?.AddRange(value);
length = value.Length;
return true;
}
}

if (arrayInitializer?.Expressions is { } valueExpressions)
{
foreach (var valueExpression in valueExpressions)
{
if (!TryGetByteOrCharLiteral(valueExpression, out char value))
{
return false;
}

values?.Add(value);
}

length = valueExpressions.Count;
return true;
}

return false;

// 'a' or (byte)'a'
static bool TryGetByteOrCharLiteral(ExpressionSyntax? expression, out char value)
{
if (expression is not null)
{
if (expression is CastExpressionSyntax cast &&
cast.Type is PredefinedTypeSyntax predefinedType &&
predefinedType.Keyword.IsKind(SyntaxKind.ByteKeyword))
{
expression = cast.Expression;
}

if (expression.IsKind(SyntaxKind.CharacterLiteralExpression) &&
expression is LiteralExpressionSyntax characterLiteral &&
characterLiteral.Token.Value is char charValue)
{
value = charValue;
return true;
}
}

value = default;
return false;
}
}

private static bool IsUtf8StringLiteralExpression(ExpressionSyntax expression, out int length)
{
const SyntaxKind Utf8StringLiteralExpression = (SyntaxKind)8756;
const SyntaxKind Utf8StringLiteralToken = (SyntaxKind)8520;

if (expression.IsKind(Utf8StringLiteralExpression) &&
expression is LiteralExpressionSyntax literal &&
literal.Token.IsKind(Utf8StringLiteralToken) &&
literal.Token.Value is string value)
{
length = value.Length;
return true;
}

length = 0;
return false;
}

protected override bool ArrayFieldUsesAreLikelyReadOnly(SyntaxNode syntax)
{
if (syntax is not VariableDeclaratorSyntax variableDeclarator ||
variableDeclarator.Identifier.Value is not string fieldName ||
syntax.FirstAncestorOrSelf<TypeDeclarationSyntax>() is not { } typeDeclaration)
{
return false;
}

// An optimistic implementation that only looks for simple assignments to the field or its array elements.
foreach (var member in typeDeclaration.Members)
{
bool isCtor = member.IsKind(SyntaxKind.ConstructorDeclaration);

foreach (var node in member.DescendantNodes())
{
if (node.IsKind(SyntaxKind.SimpleAssignmentExpression) &&
node is AssignmentExpressionSyntax assignment)
{
if (assignment.Left.IsKind(SyntaxKind.ElementAccessExpression))
{
if (assignment.Left is ElementAccessExpressionSyntax elementAccess &&
IsFieldReference(elementAccess.Expression, fieldName))
{
// s_array[42] = foo;
return false;
}
}
else if (isCtor)
{
if (IsFieldReference(assignment.Left, fieldName))
{
// s_array = foo;
return false;
}
}
}
}
}

return true;

static bool IsFieldReference(ExpressionSyntax expression, string fieldName) =>
expression.IsKind(SyntaxKind.IdentifierName) &&
expression is IdentifierNameSyntax identifierName &&
identifierName.Identifier.Value is string value &&
value == fieldName;
}
}
}
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ CA1861 | Performance | Info | AvoidConstArrays, [Documentation](https://learn.mi
CA1862 | Performance | Info | RecommendCaseInsensitiveStringComparison, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1862)
CA1863 | Performance | Hidden | UseCompositeFormatAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1862)
CA1864 | Performance | Info | PreferDictionaryTryAddValueOverGuardedAddAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1864)
CA1870 | Performance | Info | UseSearchValuesAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1870)
CA2021 | Reliability | Warning | DoNotCallEnumerableCastOrOfTypeWithIncompatibleTypesAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2021)

### Removed Rules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1916,6 +1916,18 @@ Widening and user defined conversions are not supported with generic types.</val
<data name="UseSpanClearInsteadOfFillTitle" xml:space="preserve">
<value>Prefer 'Clear' over 'Fill'</value>
</data>
<data name="UseSearchValuesTitle" xml:space="preserve">
<value>Use a cached 'SearchValues' instance</value>
</data>
<data name="UseSearchValuesMessage" xml:space="preserve">
<value>Use a cached 'SearchValues' instance for improved searching performance</value>
</data>
<data name="UseSearchValuesDescription" xml:space="preserve">
<value>Using a cached 'SearchValues' instance is more efficient than passing values to 'IndexOfAny'/'ContainsAny' directly.</value>
</data>
<data name="UseSearchValuesCodeFixTitle" xml:space="preserve">
<value>Use 'SearchValues'</value>
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesDescription" xml:space="preserve">
<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>
</data>
Expand Down
Loading

0 comments on commit 8011355

Please sign in to comment.