Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Use 'StartsWith' instead of 'IndexOf' analyzer #6295

Merged
merged 28 commits into from
Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4d68b55
Implement Use 'StartsWith' instead of 'IndexOf' analyzer
Youssef1313 Dec 1, 2022
3f41d01
Support fix all
Youssef1313 Dec 1, 2022
cd66a08
Write a test
Youssef1313 Dec 1, 2022
64c5793
address feedback, more tests
Youssef1313 Dec 2, 2022
1af0b0e
Remove unnecessary using
Youssef1313 Dec 2, 2022
4aad448
Merge branch 'main' into starts-with
Youssef1313 Dec 6, 2022
7df3273
Simplify
Youssef1313 Dec 7, 2022
01eaaf4
Refactor
Youssef1313 Dec 7, 2022
2ce9999
wip
Youssef1313 Dec 7, 2022
c81a3e3
wip
Youssef1313 Dec 7, 2022
451ba14
Redundant comment
Youssef1313 Dec 7, 2022
4c43d0f
Remove unused using directive
Youssef1313 Dec 8, 2022
400cde7
Handle some scenarios, fix tests, and add more tests
Youssef1313 Dec 9, 2022
c9817ff
Remove TODO
Youssef1313 Dec 9, 2022
5b5026e
Fix formatting
Youssef1313 Dec 9, 2022
f23bb18
Merge branch 'main' into starts-with
Youssef1313 Dec 16, 2022
e7b76a0
Address review comments
Youssef1313 Dec 17, 2022
6605ad0
Fix build
Youssef1313 Dec 17, 2022
ebb49f1
Fix formatting
Youssef1313 Dec 17, 2022
e4dc78d
Fix build
Youssef1313 Dec 17, 2022
e42cc08
Try with elastic marker
Youssef1313 Dec 17, 2022
58ce1d6
Update generated files
Youssef1313 Dec 17, 2022
167b826
Revert "Try with elastic marker"
Youssef1313 Dec 17, 2022
437db82
Update test
Youssef1313 Dec 17, 2022
d0a198d
Run pack
Youssef1313 Dec 17, 2022
b1ad36e
Revert "Revert "Try with elastic marker""
Youssef1313 Dec 17, 2022
c358bfb
revert
Youssef1313 Dec 17, 2022
d22ec17
Apply suggestions from code review
buyaa-n Dec 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ Rule ID | Category | Severity | Notes
--------|----------|----------|-------
CA1856 | Performance | Error | ConstantExpectedAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1856)
CA1857 | Performance | Warning | ConstantExpectedAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1857)
CA1858 | Performance | Info | UseStartsWithInsteadOfIndexOfComparisonWithZero, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1858)
Original file line number Diff line number Diff line change
Expand Up @@ -2031,4 +2031,13 @@
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesConversionThrowsMessage" xml:space="preserve">
<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>
</data>
<data name="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription" xml:space="preserve">
<value>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</value>
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage" xml:space="preserve">
<value>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</value>
</data>
<data name="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle" xml:space="preserve">
<value>Use 'StartsWith'</value>
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Composition;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Editing;

namespace Microsoft.NetCore.Analyzers.Performance
{
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
public sealed class UseStartsWithInsteadOfIndexOfComparisonWithZeroCodeFix : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(UseStartsWithInsteadOfIndexOfComparisonWithZero.RuleId);

public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var document = context.Document;
var diagnostic = context.Diagnostics[0];
var root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
var node = root.FindNode(context.Span, getInnermostNodeForTie: true);
var instance = root.FindNode(diagnostic.AdditionalLocations[0].SourceSpan);
var argument = root.FindNode(diagnostic.AdditionalLocations[1].SourceSpan);

context.RegisterCodeFix(
CodeAction.Create(MicrosoftNetCoreAnalyzersResources.UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle,
jeffhandley marked this conversation as resolved.
Show resolved Hide resolved
createChangedDocument: cancellationToken =>
{
var generator = SyntaxGenerator.GetGenerator(document);
var expression = generator.InvocationExpression(generator.MemberAccessExpression(instance, "StartsWith"), argument);
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved

var shouldNegate = diagnostic.Properties.TryGetValue(UseStartsWithInsteadOfIndexOfComparisonWithZero.ShouldNegateKey, out _);
if (shouldNegate)
{
expression = generator.LogicalNotExpression(expression);
}

return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(node, expression)));
jeffhandley marked this conversation as resolved.
Show resolved Hide resolved
},
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
equivalenceKey: nameof(MicrosoftNetCoreAnalyzersResources.UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle)),
context.Diagnostics);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Performance
{
using static MicrosoftNetCoreAnalyzersResources;

[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class UseStartsWithInsteadOfIndexOfComparisonWithZero : DiagnosticAnalyzer
{
internal const string RuleId = "CA1858";
internal const string ShouldNegateKey = "ShouldNegate";

internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
id: "CA1858",
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
title: CreateLocalizableResourceString(nameof(UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle)),
messageFormat: CreateLocalizableResourceString(nameof(UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage)),
category: DiagnosticCategory.Performance,
ruleLevel: RuleLevel.IdeSuggestion,
description: CreateLocalizableResourceString(nameof(UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription)),
isPortedFxCopRule: false,
isDataflowRule: false
);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

context.RegisterCompilationStartAction(context =>
{
var stringType = context.Compilation.GetSpecialType(SpecialType.System_String);
if (stringType.GetMembers("StartsWith").FirstOrDefault() is not IMethodSymbol ||
stringType.GetMembers("IndexOf").FirstOrDefault() is not IMethodSymbol indexOfSymbol)
{
return;
}

context.RegisterOperationAction(context =>
{
var binaryOperation = (IBinaryOperation)context.Operation;
if (binaryOperation.OperatorKind is not (BinaryOperatorKind.Equals or BinaryOperatorKind.NotEquals))
{
return;
}

if (IsIndexOfComparedWithZero(binaryOperation.LeftOperand, binaryOperation.RightOperand, indexOfSymbol, out var instanceLocation, out var argumentLocation) ||
IsIndexOfComparedWithZero(binaryOperation.RightOperand, binaryOperation.LeftOperand, indexOfSymbol, out instanceLocation, out argumentLocation))
{
var properties = ImmutableDictionary<string, string?>.Empty;
if (binaryOperation.OperatorKind == BinaryOperatorKind.NotEquals)
{
properties = properties.Add(ShouldNegateKey, "");
}

context.ReportDiagnostic(binaryOperation.CreateDiagnostic(Rule, additionalLocations: ImmutableArray.Create(instanceLocation, argumentLocation), properties: properties));
}

}, OperationKind.Binary);
});
}

private static bool IsIndexOfComparedWithZero(IOperation left, IOperation right, IMethodSymbol indexOfSymbol, [NotNullWhen(true)] out Location? instanceLocation, [NotNullWhen(true)] out Location? argumentLocation)
{
if (right.ConstantValue is not { HasValue: true, Value: 0 } ||
left is not IInvocationOperation invocation ||
invocation.Arguments.Length != 1 ||
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
SymbolEqualityComparer.Default.Equals(invocation.TargetMethod, indexOfSymbol))
{
instanceLocation = null;
argumentLocation = null;
return false;
}


instanceLocation = invocation.Instance.Syntax.GetLocation();
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
argumentLocation = invocation.Arguments[0].Syntax.GetLocation();
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">Preferovat možnost Vymazat před možností Fill</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">Použít string.Equals</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">„Clear“ vor „Fill“ bevorzugen</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">"string.Equals" verwenden</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">Preferir "Clear" en lugar de "Fill"</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">Use 'string.Equals'</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">Préférer 'Effacer' à 'Remplissage'</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">Utilisez ’String. Equals</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">Preferisci 'Cancella' a 'Riempimento'</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">Usare 'string.Equals'</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">'塗りつぶし' よりも 'クリア' を優先する</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">'string.Equals' を使用します。</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">'채우기'보다 '지우기' 선호</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">'string.Equals' 사용</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">Preferuj opcję „Wyczyść” niż „Wypełnij”</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">Użyj ciągu „string. Equals”</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2987,6 +2987,21 @@
<target state="translated">Preferir 'Clear' ao invés de 'Fill'</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroDescription">
<source>It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</source>
<target state="new">It is both clearer and likely faster to use 'StartsWith' instead of comparing the result of 'IndexOf' to zero.</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroMessage">
<source>Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</source>
<target state="new">Use 'StartsWith' instead of comparing the result of 'IndexOf' to 0</target>
<note />
</trans-unit>
<trans-unit id="UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle">
<source>Use 'StartsWith'</source>
<target state="new">Use 'StartsWith'</target>
<note />
</trans-unit>
<trans-unit id="UseStringEqualsOverStringCompareCodeFixTitle">
<source>Use 'string.Equals'</source>
<target state="translated">Use 'string. Igual a'</target>
Expand Down
Loading