Skip to content

Commit 10632e6

Browse files
Youssef1313Youssef1313
authored
Implement Use 'StartsWith' instead of 'IndexOf' analyzer (#6295)
* Implement Use 'StartsWith' instead of 'IndexOf' analyzer * Support fix all * Write a test * address feedback, more tests * Remove unnecessary using * Simplify * Refactor * wip * Redundant comment * Remove unused using directive * Handle some scenarios, fix tests, and add more tests
1 parent 79cc002 commit 10632e6

24 files changed

+1463
-3
lines changed

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
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.Composition;
4+
using Analyzer.Utilities;
5+
using Microsoft.CodeAnalysis;
6+
using Microsoft.CodeAnalysis.CodeFixes;
7+
using Microsoft.CodeAnalysis.CSharp;
8+
using Microsoft.CodeAnalysis.CSharp.Syntax;
9+
using Microsoft.CodeAnalysis.Editing;
10+
using Microsoft.NetCore.Analyzers.Performance;
11+
12+
namespace Microsoft.NetCore.CSharp.Analyzers.Performance
13+
{
14+
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
15+
public sealed class CSharpUseStartsWithInsteadOfIndexOfComparisonWithZeroCodeFix : UseStartsWithInsteadOfIndexOfComparisonWithZeroCodeFix
16+
{
17+
protected override SyntaxNode AppendElasticMarker(SyntaxNode replacement)
18+
=> replacement.WithTrailingTrivia(SyntaxFactory.ElasticMarker);
19+
20+
protected override SyntaxNode HandleCharStringComparisonOverload(SyntaxGenerator generator, SyntaxNode instance, SyntaxNode[] arguments, bool shouldNegate)
21+
{
22+
// For 'x.IndexOf(ch, stringComparison)', we switch to 'x.AsSpan().StartsWith(stackalloc char[1] { ch }, stringComparison)'
23+
var (argumentSyntax, index) = GetCharacterArgumentAndIndex(arguments);
24+
arguments[index] = argumentSyntax.WithExpression(SyntaxFactory.StackAllocArrayCreationExpression(
25+
SyntaxFactory.ArrayType(
26+
(TypeSyntax)generator.TypeExpression(SpecialType.System_Char),
27+
SyntaxFactory.SingletonList(SyntaxFactory.ArrayRankSpecifier(SyntaxFactory.SingletonSeparatedList((ExpressionSyntax)generator.LiteralExpression(1))))),
28+
SyntaxFactory.InitializerExpression(SyntaxKind.ArrayInitializerExpression, SyntaxFactory.SingletonSeparatedList(argumentSyntax.Expression))
29+
));
30+
instance = generator.InvocationExpression(generator.MemberAccessExpression(instance, "AsSpan")).WithAdditionalAnnotations(new SyntaxAnnotation("SymbolId", "System.MemoryExtensions")).WithAddImportsAnnotation();
31+
return CreateStartsWithInvocationFromArguments(generator, instance, arguments, shouldNegate);
32+
}
33+
34+
private static (ArgumentSyntax Argument, int Index) GetCharacterArgumentAndIndex(SyntaxNode[] arguments)
35+
{
36+
var firstArgument = (ArgumentSyntax)arguments[0];
37+
if (firstArgument.NameColon is null or { Name.Identifier.Value: "value" })
38+
{
39+
return (firstArgument, 0);
40+
}
41+
42+
return ((ArgumentSyntax)arguments[1], 1);
43+
}
44+
}
45+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ CA1512 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](http
1010
CA1513 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1513)
1111
CA1856 | Performance | Error | ConstantExpectedAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1856)
1212
CA1857 | Performance | Warning | ConstantExpectedAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1857)
13+
CA1858 | Performance | Info | UseStartsWithInsteadOfIndexOfComparisonWithZero, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1858)
1314

1415
### Removed Rules
1516

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2031,6 +2031,18 @@
20312031
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesConversionThrowsMessage" xml:space="preserve">
20322032
<value>Starting with .NET 7 the explicit conversion '{0}' will throw when overflowing in a checked context. Wrap the expression with an 'unchecked' statement to restore the .NET 6 behavior.</value>
20332033
</data>
2034+
<data name="UseStartsWithInsteadOfIndexOfComparisonWithZeroCodeFixTitle" xml:space="preserve">
2035+
<value>Use 'StartsWith'</value>
2036+
</data>
2037+
<data name="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription" xml:space="preserve">
2038+
<value>It is both clearer and faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</value>
2039+
</data>
2040+
<data name="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage" xml:space="preserve">
2041+
<value>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</value>
2042+
</data>
2043+
<data name="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle" xml:space="preserve">
2044+
<value>Use 'StartsWith' instead of 'IndexOf'</value>
2045+
</data>
20342046
<data name="UseArgumentNullExceptionThrowHelperTitle" xml:space="preserve">
20352047
<value>Use ArgumentNullException throw helper</value>
20362048
</data>
@@ -2052,5 +2064,4 @@
20522064
<data name="UseThrowHelperFix" xml:space="preserve">
20532065
<value>Use '{0}.{1}'</value>
20542066
</data>
2055-
2056-
</root>
2067+
</root>

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseStartsWithInsteadOfIndexOfComparisonWithZero.Fixer.cs‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
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.Immutable;
4+
using System.Diagnostics;
5+
using System.Threading.Tasks;
6+
using Microsoft.CodeAnalysis;
7+
using Microsoft.CodeAnalysis.CodeActions;
8+
using Microsoft.CodeAnalysis.CodeFixes;
9+
using Microsoft.CodeAnalysis.Editing;
10+
11+
namespace Microsoft.NetCore.Analyzers.Performance
12+
{
13+
public abstract class UseStartsWithInsteadOfIndexOfComparisonWithZeroCodeFix : CodeFixProvider
14+
{
15+
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(UseStartsWithInsteadOfIndexOfComparisonWithZero.RuleId);
16+
17+
public override FixAllProvider GetFixAllProvider()
18+
=> WellKnownFixAllProviders.BatchFixer;
19+
20+
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
21+
{
22+
var document = context.Document;
23+
var diagnostic = context.Diagnostics[0];
24+
var root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
25+
var node = root.FindNode(context.Span, getInnermostNodeForTie: true);
26+
27+
context.RegisterCodeFix(
28+
CodeAction.Create(MicrosoftNetCoreAnalyzersResources.UseStartsWithInsteadOfIndexOfComparisonWithZeroCodeFixTitle,
29+
createChangedDocument: cancellationToken =>
30+
{
31+
var instance = root.FindNode(diagnostic.AdditionalLocations[0].SourceSpan);
32+
var arguments = new SyntaxNode[diagnostic.AdditionalLocations.Count - 1];
33+
for (int i = 1; i < diagnostic.AdditionalLocations.Count; i++)
34+
{
35+
arguments[i - 1] = root.FindNode(diagnostic.AdditionalLocations[i].SourceSpan);
36+
}
37+
38+
var generator = SyntaxGenerator.GetGenerator(document);
39+
var shouldNegate = diagnostic.Properties.TryGetValue(UseStartsWithInsteadOfIndexOfComparisonWithZero.ShouldNegateKey, out _);
40+
var compilationHasStartsWithCharOverload = diagnostic.Properties.TryGetKey(UseStartsWithInsteadOfIndexOfComparisonWithZero.CompilationHasStartsWithCharOverloadKey, out _);
41+
_ = diagnostic.Properties.TryGetValue(UseStartsWithInsteadOfIndexOfComparisonWithZero.ExistingOverloadKey, out var overloadValue);
42+
switch (overloadValue)
43+
{
44+
// For 'IndexOf(string)' and 'IndexOf(string, stringComparison)', we replace with StartsWith(same arguments)
45+
case UseStartsWithInsteadOfIndexOfComparisonWithZero.OverloadString:
46+
case UseStartsWithInsteadOfIndexOfComparisonWithZero.OverloadString_StringComparison:
47+
return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(node, CreateStartsWithInvocationFromArguments(generator, instance, arguments, shouldNegate))));
48+
49+
// For 'a.IndexOf(ch, stringComparison)':
50+
// C#: Use 'a.AsSpan().StartsWith(stackalloc char[1] { ch }, stringComparison)'
51+
// https://learn.microsoft.com/dotnet/api/system.memoryextensions.startswith?view=net-7.0#system-memoryextensions-startswith(system-readonlyspan((system-char))-system-readonlyspan((system-char))-system-stringcomparison)
52+
// VB: Use a.StartsWith(c.ToString(), stringComparison)
53+
case UseStartsWithInsteadOfIndexOfComparisonWithZero.OverloadChar_StringComparison:
54+
return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(node, HandleCharStringComparisonOverload(generator, instance, arguments, shouldNegate))));
55+
56+
// If 'StartsWith(char)' is available, use it. Otherwise check '.Length > 0 && [0] == ch'
57+
// For negation, we use '.Length == 0 || [0] != ch'
58+
case UseStartsWithInsteadOfIndexOfComparisonWithZero.OverloadChar:
59+
if (compilationHasStartsWithCharOverload)
60+
{
61+
return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(node, CreateStartsWithInvocationFromArguments(generator, instance, arguments, shouldNegate))));
62+
}
63+
64+
var lengthAccess = generator.MemberAccessExpression(instance, "Length");
65+
var zeroLiteral = generator.LiteralExpression(0);
66+
67+
var indexed = generator.ElementAccessExpression(instance, zeroLiteral);
68+
var ch = root.FindNode(arguments[0].Span, getInnermostNodeForTie: true);
69+
70+
var replacement = shouldNegate
71+
? generator.LogicalOrExpression(
72+
generator.ValueEqualsExpression(lengthAccess, zeroLiteral),
73+
generator.ValueNotEqualsExpression(indexed, ch))
74+
: generator.LogicalAndExpression(
75+
generator.GreaterThanExpression(lengthAccess, zeroLiteral),
76+
generator.ValueEqualsExpression(indexed, ch));
77+
78+
return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(node, AppendElasticMarker(replacement))));
79+
80+
default:
81+
Debug.Fail("This should never happen.");
82+
return Task.FromResult(document);
83+
}
84+
},
85+
equivalenceKey: nameof(MicrosoftNetCoreAnalyzersResources.UseStartsWithInsteadOfIndexOfComparisonWithZeroCodeFixTitle)),
86+
context.Diagnostics);
87+
}
88+
89+
protected abstract SyntaxNode HandleCharStringComparisonOverload(SyntaxGenerator generator, SyntaxNode instance, SyntaxNode[] arguments, bool shouldNegate);
90+
protected abstract SyntaxNode AppendElasticMarker(SyntaxNode replacement);
91+
92+
protected static SyntaxNode CreateStartsWithInvocationFromArguments(SyntaxGenerator generator, SyntaxNode instance, SyntaxNode[] arguments, bool shouldNegate)
93+
{
94+
var expression = generator.InvocationExpression(generator.MemberAccessExpression(instance, "StartsWith"), arguments);
95+
return shouldNegate ? generator.LogicalNotExpression(expression) : expression;
96+
}
97+
}
98+
}

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseStartsWithInsteadOfIndexOfComparisonWithZero.cs‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
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.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.Performance
12+
{
13+
using static MicrosoftNetCoreAnalyzersResources;
14+
15+
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
16+
public sealed class UseStartsWithInsteadOfIndexOfComparisonWithZero : DiagnosticAnalyzer
17+
{
18+
internal const string RuleId = "CA1858";
19+
internal const string ShouldNegateKey = "ShouldNegate";
20+
internal const string CompilationHasStartsWithCharOverloadKey = "CompilationHasStartsWithCharOverload";
21+
22+
internal const string ExistingOverloadKey = "ExistingOverload";
23+
24+
internal const string OverloadString = "String";
25+
internal const string OverloadString_StringComparison = "String,StringComparison";
26+
internal const string OverloadChar = "Char";
27+
internal const string OverloadChar_StringComparison = "Char,StringComparison";
28+
29+
internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
30+
id: RuleId,
31+
title: CreateLocalizableResourceString(nameof(UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle)),
32+
messageFormat: CreateLocalizableResourceString(nameof(UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage)),
33+
category: DiagnosticCategory.Performance,
34+
ruleLevel: RuleLevel.IdeSuggestion,
35+
description: CreateLocalizableResourceString(nameof(UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription)),
36+
isPortedFxCopRule: false,
37+
isDataflowRule: false
38+
);
39+
40+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
41+
42+
public override void Initialize(AnalysisContext context)
43+
{
44+
context.EnableConcurrentExecution();
45+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
46+
47+
context.RegisterCompilationStartAction(context =>
48+
{
49+
var stringType = context.Compilation.GetSpecialType(SpecialType.System_String);
50+
var hasAnyStartsWith = false;
51+
var hasStartsWithCharOverload = false;
52+
foreach (var startsWithMethod in stringType.GetMembers("StartsWith").OfType<IMethodSymbol>())
53+
{
54+
hasAnyStartsWith = true;
55+
if (startsWithMethod.Parameters is [{ Type.SpecialType: SpecialType.System_Char }])
56+
{
57+
hasStartsWithCharOverload = true;
58+
break;
59+
}
60+
}
61+
62+
if (!hasAnyStartsWith)
63+
{
64+
return;
65+
}
66+
67+
var indexOfMethodsBuilder = ImmutableArray.CreateBuilder<(IMethodSymbol IndexOfSymbol, string OverloadPropertyValue)>();
68+
foreach (var indexOfMethod in stringType.GetMembers("IndexOf").OfType<IMethodSymbol>())
69+
{
70+
if (indexOfMethod.Parameters is [{ Type.SpecialType: SpecialType.System_String }])
71+
{
72+
indexOfMethodsBuilder.Add((indexOfMethod, OverloadString));
73+
}
74+
else if (indexOfMethod.Parameters is [{ Type.SpecialType: SpecialType.System_Char }])
75+
{
76+
indexOfMethodsBuilder.Add((indexOfMethod, OverloadChar));
77+
}
78+
else if (indexOfMethod.Parameters is [{ Type.SpecialType: SpecialType.System_String }, { Name: "comparisonType" }])
79+
{
80+
indexOfMethodsBuilder.Add((indexOfMethod, OverloadString_StringComparison));
81+
}
82+
else if (indexOfMethod.Parameters is [{ Type.SpecialType: SpecialType.System_Char }, { Name: "comparisonType" }])
83+
{
84+
indexOfMethodsBuilder.Add((indexOfMethod, OverloadChar_StringComparison));
85+
}
86+
}
87+
88+
if (indexOfMethodsBuilder.Count == 0)
89+
{
90+
return;
91+
}
92+
93+
var indexOfMethods = indexOfMethodsBuilder.ToImmutable();
94+
95+
context.RegisterOperationAction(context =>
96+
{
97+
var binaryOperation = (IBinaryOperation)context.Operation;
98+
if (binaryOperation.OperatorKind is not (BinaryOperatorKind.Equals or BinaryOperatorKind.NotEquals))
99+
{
100+
return;
101+
}
102+
103+
if (IsIndexOfComparedWithZero(binaryOperation.LeftOperand, binaryOperation.RightOperand, indexOfMethods, out var additionalLocations, out var properties) ||
104+
IsIndexOfComparedWithZero(binaryOperation.RightOperand, binaryOperation.LeftOperand, indexOfMethods, out additionalLocations, out properties))
105+
{
106+
if (binaryOperation.OperatorKind == BinaryOperatorKind.NotEquals)
107+
{
108+
properties = properties.Add(ShouldNegateKey, "");
109+
}
110+
111+
if (hasStartsWithCharOverload)
112+
{
113+
properties = properties.Add(CompilationHasStartsWithCharOverloadKey, "");
114+
}
115+
116+
context.ReportDiagnostic(binaryOperation.CreateDiagnostic(Rule, additionalLocations, properties));
117+
}
118+
}, OperationKind.Binary);
119+
});
120+
}
121+
122+
private static bool IsIndexOfComparedWithZero(
123+
IOperation left, IOperation right,
124+
ImmutableArray<(IMethodSymbol Symbol, string OverloadPropertyValue)> indexOfMethods,
125+
out ImmutableArray<Location> additionalLocations,
126+
out ImmutableDictionary<string, string?> properties)
127+
{
128+
properties = ImmutableDictionary<string, string?>.Empty;
129+
130+
if (right.ConstantValue is { HasValue: true, Value: 0 } &&
131+
left is IInvocationOperation invocation)
132+
{
133+
foreach (var (indexOfMethod, overloadPropertyValue) in indexOfMethods)
134+
{
135+
if (indexOfMethod.Equals(invocation.TargetMethod, SymbolEqualityComparer.Default))
136+
{
137+
var locationsBuilder = ImmutableArray.CreateBuilder<Location>();
138+
locationsBuilder.Add(invocation.Instance.Syntax.GetLocation());
139+
locationsBuilder.AddRange(invocation.Arguments.Select(arg => arg.Syntax.GetLocation()));
140+
additionalLocations = locationsBuilder.ToImmutable();
141+
142+
properties = properties.Add(ExistingOverloadKey, overloadPropertyValue);
143+
return true;
144+
}
145+
}
146+
}
147+
148+
additionalLocations = ImmutableArray<Location>.Empty;
149+
return false;
150+
}
151+
}
152+
}

0 commit comments

Comments
 (0)