-
Notifications
You must be signed in to change notification settings - Fork 480
Implement Use 'StartsWith' instead of 'IndexOf' analyzer #6295
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
4d68b55
Implement Use 'StartsWith' instead of 'IndexOf' analyzer
Youssef1313 3f41d01
Support fix all
Youssef1313 cd66a08
Write a test
Youssef1313 64c5793
address feedback, more tests
Youssef1313 1af0b0e
Remove unnecessary using
Youssef1313 4aad448
Merge branch 'main' into starts-with
Youssef1313 7df3273
Simplify
Youssef1313 01eaaf4
Refactor
Youssef1313 2ce9999
wip
Youssef1313 c81a3e3
wip
Youssef1313 451ba14
Redundant comment
Youssef1313 4c43d0f
Remove unused using directive
Youssef1313 400cde7
Handle some scenarios, fix tests, and add more tests
Youssef1313 c9817ff
Remove TODO
Youssef1313 5b5026e
Fix formatting
Youssef1313 f23bb18
Merge branch 'main' into starts-with
Youssef1313 e7b76a0
Address review comments
Youssef1313 6605ad0
Fix build
Youssef1313 ebb49f1
Fix formatting
Youssef1313 e4dc78d
Fix build
Youssef1313 e42cc08
Try with elastic marker
Youssef1313 58ce1d6
Update generated files
Youssef1313 167b826
Revert "Try with elastic marker"
Youssef1313 437db82
Update test
Youssef1313 d0a198d
Run pack
Youssef1313 b1ad36e
Revert "Revert "Try with elastic marker""
Youssef1313 c358bfb
revert
Youssef1313 d22ec17
Apply suggestions from code review
buyaa-n File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
49 changes: 49 additions & 0 deletions
49
...ft.NetCore.Analyzers/Performance/UseStartsWithInsteadOfIndexOfComparisonWithZero.Fixer.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // 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 override FixAllProvider GetFixAllProvider() | ||
| => WellKnownFixAllProviders.BatchFixer; | ||
|
|
||
| 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.
Outdated
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.
Outdated
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.
Outdated
Show resolved
Hide resolved
|
||
| }, | ||
buyaa-n marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| equivalenceKey: nameof(MicrosoftNetCoreAnalyzersResources.UseStartsWithInsteadOfIndexOfComparisonWithZeroTitle)), | ||
| context.Diagnostics); | ||
| } | ||
| } | ||
| } | ||
91 changes: 91 additions & 0 deletions
91
...icrosoft.NetCore.Analyzers/Performance/UseStartsWithInsteadOfIndexOfComparisonWithZero.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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.
Outdated
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.
Outdated
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.
Outdated
Show resolved
Hide resolved
|
||
| argumentLocation = invocation.Arguments[0].Syntax.GetLocation(); | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.