-
Notifications
You must be signed in to change notification settings - Fork 468
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Analyzer and fixer that recommend case insensitive string comparison (#…
…6662) * Resources and documentation * Analyzer * Fixer * Base test class * C# tests * VisualBasic tests * Address suggestion: Inline localizable strings that are used only once. * Address suggestion: Prefer GetSpecialType for string. * Use explicit. * Do not pass argument to diagnostic that does not need it. * Address suggestion: Use Length instead of Count() * Address suggestion: Improve name of caseChangingMethodName * Address suggestion: Remove token cancellation handling code * Address suggestion: Remove unused cancellation token argument. * Address suggestion: Add missing fixer attribute, seal class * Analyzer: Remove unused using. * Address suggestion: Use better test pattern. Simplify tests. * More tests * Address suggestion: Await tests. * Fix spacing warning * Handle all 3 IndexOf overloads that can be converted to the StringComparison overload. * Address suggestion: Walk up parenthesized operations * Add "No diagnostic" tests * Do not fix CompareTo, only suggest. * Remove unused CompareTo fixer code and fix some warnings. * Add missing entry for second variant of diagnostic rule message in "RulesMissingDocumentation.md". * Revert md and sarif changes to fix CI * Address suggestion: Use WalkUpParentheses * Add short string for the codefix CodeAction title * Consume codefix CodeAction short string in title * Fix CI * Remove some unnecessary fixer code * Use better equivalenceKey * Diagnose inverted pattern * Tests for named arguments * Change resources to be able to address named parameters * src: Address named parameters * tests: Adjust tests to verify named parameters * Move ExportCodeFixProvider and Shared attributes to derived types. * public sealed fixers * Improved resource description mentioning allocation. Use single quotes, minor spacing. * Remove commented code * Update xlfs * Remove commented code * Rename to GetNewArguments * Fix build
- Loading branch information
1 parent
ebd3126
commit 126e31e
Showing
27 changed files
with
11,612 additions
and
9,526 deletions.
There are no files selected for viewing
101 changes: 101 additions & 0 deletions
101
...soft.NetCore.Analyzers/Performance/CSharpRecommendCaseInsensitiveStringComparisonFixer.cs
This file contains 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,101 @@ | ||
| // 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 System.Composition; | ||
| using System.Diagnostics; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.CodeFixes; | ||
| using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
| using Microsoft.CodeAnalysis.Editing; | ||
| using Microsoft.CodeAnalysis.Operations; | ||
| using Microsoft.NetCore.Analyzers.Performance; | ||
|
|
||
| namespace Microsoft.NetCore.CSharp.Analyzers.Performance | ||
| { | ||
| using RCISCAnalyzer = RecommendCaseInsensitiveStringComparisonAnalyzer; | ||
|
|
||
| [ExportCodeFixProvider(LanguageNames.CSharp), Shared] | ||
| public sealed class CSharpRecommendCaseInsensitiveStringComparisonFixer : RecommendCaseInsensitiveStringComparisonFixer | ||
| { | ||
| protected override List<SyntaxNode> GetNewArguments(SyntaxGenerator generator, IInvocationOperation mainInvocationOperation, | ||
| INamedTypeSymbol stringComparisonType, out SyntaxNode? mainInvocationInstance) | ||
| { | ||
| List<SyntaxNode> arguments = new(); | ||
| bool isAnyArgumentNamed = false; | ||
|
|
||
| InvocationExpressionSyntax invocationExpression = (InvocationExpressionSyntax)mainInvocationOperation.Syntax; | ||
|
|
||
| string? caseChangingApproachName = null; | ||
| bool isChangingCaseInArgument = false; | ||
|
|
||
| mainInvocationInstance = null; | ||
|
|
||
| if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression) | ||
| { | ||
| var internalExpression = memberAccessExpression.Expression is ParenthesizedExpressionSyntax parenthesizedExpression ? | ||
| parenthesizedExpression.Expression : | ||
| memberAccessExpression.Expression; | ||
|
|
||
| if (internalExpression is InvocationExpressionSyntax internalInvocationExpression && | ||
| internalInvocationExpression.Expression is MemberAccessExpressionSyntax internalMemberAccessExpression) | ||
| { | ||
| mainInvocationInstance = internalMemberAccessExpression.Expression; | ||
| caseChangingApproachName = GetCaseChangingApproach(internalMemberAccessExpression.Name.Identifier.ValueText); | ||
| } | ||
| else | ||
| { | ||
| mainInvocationInstance = memberAccessExpression.Expression; | ||
| isChangingCaseInArgument = true; | ||
| } | ||
| } | ||
|
|
||
| foreach (ArgumentSyntax node in invocationExpression.ArgumentList.Arguments) | ||
| { | ||
| string? argumentName = node.NameColon?.Name.Identifier.ValueText; | ||
| isAnyArgumentNamed |= argumentName != null; | ||
|
|
||
| ExpressionSyntax argumentExpression = node.Expression is ParenthesizedExpressionSyntax argumentParenthesizedExpression ? | ||
| argumentParenthesizedExpression.Expression : | ||
| node.Expression; | ||
|
|
||
| MemberAccessExpressionSyntax? argumentMemberAccessExpression = null; | ||
| if (argumentExpression is InvocationExpressionSyntax argumentInvocationExpression && | ||
| (argumentMemberAccessExpression = argumentInvocationExpression.Expression as MemberAccessExpressionSyntax) != null) | ||
| { | ||
| caseChangingApproachName = GetCaseChangingApproach(argumentMemberAccessExpression.Name.Identifier.ValueText); | ||
| } | ||
|
|
||
| SyntaxNode newArgumentNode; | ||
| if (isChangingCaseInArgument) | ||
| { | ||
| if (argumentMemberAccessExpression != null) | ||
| { | ||
| newArgumentNode = argumentName == RCISCAnalyzer.StringParameterName ? | ||
| generator.Argument(RCISCAnalyzer.StringParameterName, RefKind.None, argumentMemberAccessExpression.Expression) : | ||
| generator.Argument(argumentMemberAccessExpression.Expression); | ||
| } | ||
| else | ||
| { | ||
| newArgumentNode = node; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| newArgumentNode = node; | ||
| } | ||
|
|
||
| arguments.Add(newArgumentNode); | ||
| } | ||
|
|
||
| Debug.Assert(caseChangingApproachName != null); | ||
| Debug.Assert(mainInvocationInstance != null); | ||
|
|
||
| SyntaxNode stringComparisonArgument = GetNewStringComparisonArgument(generator, | ||
| stringComparisonType, caseChangingApproachName!, isAnyArgumentNamed); | ||
|
|
||
| arguments.Add(stringComparisonArgument); | ||
|
|
||
| return arguments; | ||
| } | ||
| } | ||
| } |
This file contains 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 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
238 changes: 238 additions & 0 deletions
238
...rosoft.NetCore.Analyzers/Performance/RecommendCaseInsensitiveStringComparison.Analyzer.cs
This file contains 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,238 @@ | ||
| // 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 System.Collections.Immutable; | ||
| 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; | ||
|
|
||
| /// <summary> | ||
| /// CA1862: Prefer the StringComparison method overloads to perform case-insensitive string comparisons. | ||
| /// </summary> | ||
| [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] | ||
| public sealed class RecommendCaseInsensitiveStringComparisonAnalyzer : DiagnosticAnalyzer | ||
| { | ||
| internal const string RuleId = "CA1862"; | ||
|
|
||
| internal const string StringComparisonInvariantCultureIgnoreCaseName = "InvariantCultureIgnoreCase"; | ||
| internal const string StringComparisonCurrentCultureIgnoreCaseName = "CurrentCultureIgnoreCase"; | ||
| internal const string StringToLowerMethodName = "ToLower"; | ||
| internal const string StringToUpperMethodName = "ToUpper"; | ||
| internal const string StringToLowerInvariantMethodName = "ToLowerInvariant"; | ||
| internal const string StringToUpperInvariantMethodName = "ToUpperInvariant"; | ||
| internal const string StringContainsMethodName = "Contains"; | ||
| internal const string StringIndexOfMethodName = "IndexOf"; | ||
| internal const string StringStartsWithMethodName = "StartsWith"; | ||
| internal const string StringCompareToMethodName = "CompareTo"; | ||
| internal const string StringComparerCompareMethodName = "Compare"; | ||
| internal const string StringParameterName = "value"; | ||
| internal const string StringComparisonParameterName = "comparisonType"; | ||
|
|
||
| internal static readonly DiagnosticDescriptor RecommendCaseInsensitiveStringComparisonRule = DiagnosticDescriptorHelper.Create( | ||
| RuleId, | ||
| CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparisonTitle)), | ||
| CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparisonMessage)), | ||
| DiagnosticCategory.Performance, | ||
| RuleLevel.IdeSuggestion, | ||
| CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparisonDescription)), | ||
| isPortedFxCopRule: false, | ||
| isDataflowRule: false); | ||
|
|
||
| internal static readonly DiagnosticDescriptor RecommendCaseInsensitiveStringComparerRule = DiagnosticDescriptorHelper.Create( | ||
| RuleId, | ||
| CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparerTitle)), | ||
| CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparerMessage)), | ||
| DiagnosticCategory.Performance, | ||
| RuleLevel.IdeSuggestion, | ||
| CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparerDescription)), | ||
| isPortedFxCopRule: false, | ||
| isDataflowRule: false); | ||
|
|
||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create( | ||
| RecommendCaseInsensitiveStringComparisonRule, RecommendCaseInsensitiveStringComparerRule); | ||
|
|
||
| public override void Initialize(AnalysisContext context) | ||
| { | ||
| context.EnableConcurrentExecution(); | ||
| context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); | ||
| context.RegisterCompilationStartAction(AnalyzeCompilationStart); | ||
| } | ||
|
|
||
| private void AnalyzeCompilationStart(CompilationStartAnalysisContext context) | ||
| { | ||
| // Retrieve the essential types: string, StringComparison, StringComparer | ||
|
|
||
| INamedTypeSymbol stringType = context.Compilation.GetSpecialType(SpecialType.System_String); | ||
| INamedTypeSymbol int32Type = context.Compilation.GetSpecialType(SpecialType.System_Int32); | ||
|
|
||
| if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringComparison, out INamedTypeSymbol? stringComparisonType)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringComparer, out INamedTypeSymbol? stringComparerType)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Retrieve the offending parameterless methods: ToLower, ToLowerInvariant, ToUpper, ToUpperInvariant | ||
|
|
||
| IMethodSymbol? toLowerParameterlessMethod = stringType.GetMembers(StringToLowerMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(); | ||
| if (toLowerParameterlessMethod == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| IMethodSymbol? toLowerInvariantParameterlessMethod = stringType.GetMembers(StringToLowerInvariantMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(); | ||
| if (toLowerInvariantParameterlessMethod == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| IMethodSymbol? toUpperParameterlessMethod = stringType.GetMembers(StringToUpperMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(); | ||
| if (toUpperParameterlessMethod == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| IMethodSymbol? toUpperInvariantParameterlessMethod = stringType.GetMembers(StringToUpperInvariantMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(); | ||
| if (toUpperInvariantParameterlessMethod == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Create the different expected parameter combinations | ||
|
|
||
| ParameterInfo[] stringParameter = new[] | ||
| { | ||
| ParameterInfo.GetParameterInfo(stringType) | ||
| }; | ||
|
|
||
| ParameterInfo[] stringInt32Parameters = new[] | ||
| { | ||
| ParameterInfo.GetParameterInfo(stringType), | ||
| ParameterInfo.GetParameterInfo(int32Type) | ||
| }; | ||
|
|
||
| ParameterInfo[] stringInt32Int32Parameters = new[] | ||
| { | ||
| ParameterInfo.GetParameterInfo(stringType), | ||
| ParameterInfo.GetParameterInfo(int32Type), | ||
| ParameterInfo.GetParameterInfo(int32Type) | ||
| }; | ||
|
|
||
| // Retrieve the diagnosable string overload methods: Contains, IndexOf (3 overloads), StartsWith, CompareTo | ||
|
|
||
| // Contains(string) | ||
| IMethodSymbol? containsStringMethod = stringType.GetMembers(StringContainsMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(stringParameter); | ||
| if (containsStringMethod == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // StartsWith(string) | ||
| IMethodSymbol? startsWithStringMethod = stringType.GetMembers(StringStartsWithMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(stringParameter); | ||
| if (startsWithStringMethod == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| IEnumerable<IMethodSymbol> indexOfMethods = stringType.GetMembers(StringIndexOfMethodName).OfType<IMethodSymbol>(); | ||
|
|
||
| // IndexOf(string) | ||
| IMethodSymbol? indexOfStringMethod = indexOfMethods.GetFirstOrDefaultMemberWithParameterInfos(stringParameter); | ||
| if (indexOfStringMethod == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // IndexOf(string, int startIndex) | ||
| IMethodSymbol? indexOfStringInt32Method = indexOfMethods.GetFirstOrDefaultMemberWithParameterInfos(stringInt32Parameters); | ||
| if (indexOfStringInt32Method == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // IndexOf(string, int startIndex, int count) | ||
| IMethodSymbol? indexOfStringInt32Int32Method = indexOfMethods.GetFirstOrDefaultMemberWithParameterInfos(stringInt32Int32Parameters); | ||
| if (indexOfStringInt32Int32Method == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // CompareTo(string) | ||
| IMethodSymbol? compareToStringMethod = stringType.GetMembers(StringCompareToMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(stringParameter); | ||
| if (compareToStringMethod == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Retrieve the StringComparer properties that need to be flagged: CurrentCultureIgnoreCase, InvariantCultureIgnoreCase | ||
|
|
||
| IEnumerable<IPropertySymbol> ccicPropertyGroup = stringComparerType.GetMembers(StringComparisonCurrentCultureIgnoreCaseName).OfType<IPropertySymbol>(); | ||
| if (!ccicPropertyGroup.Any()) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| IEnumerable<IPropertySymbol> icicPropertyGroup = stringComparerType.GetMembers(StringComparisonInvariantCultureIgnoreCaseName).OfType<IPropertySymbol>(); | ||
| if (!icicPropertyGroup.Any()) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| context.RegisterOperationAction(context => | ||
| { | ||
| IInvocationOperation caseChangingInvocation = (IInvocationOperation)context.Operation; | ||
| IMethodSymbol caseChangingMethod = caseChangingInvocation.TargetMethod; | ||
|
|
||
| if (!caseChangingMethod.Equals(toLowerParameterlessMethod) && | ||
| !caseChangingMethod.Equals(toLowerInvariantParameterlessMethod) && | ||
| !caseChangingMethod.Equals(toUpperParameterlessMethod) && | ||
| !caseChangingMethod.Equals(toUpperInvariantParameterlessMethod)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Ignore parenthesized operations | ||
| IOperation? ancestor = caseChangingInvocation.WalkUpParentheses().WalkUpConversion().Parent; | ||
|
|
||
| IInvocationOperation diagnosableInvocation; | ||
| if (ancestor is IInvocationOperation invocationAncestor) | ||
| { | ||
| diagnosableInvocation = invocationAncestor; | ||
| } | ||
| else if (ancestor is IArgumentOperation argumentAncestor && argumentAncestor.Parent is IInvocationOperation argumentInvocationAncestor) | ||
| { | ||
| diagnosableInvocation = argumentInvocationAncestor; | ||
| } | ||
| else | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| IMethodSymbol diagnosableMethod = diagnosableInvocation.TargetMethod; | ||
|
|
||
| if (diagnosableMethod.Equals(containsStringMethod) || | ||
| diagnosableMethod.Equals(startsWithStringMethod) || | ||
| diagnosableMethod.Equals(indexOfStringMethod) || | ||
| diagnosableMethod.Equals(indexOfStringInt32Method) || | ||
| diagnosableMethod.Equals(indexOfStringInt32Int32Method)) | ||
| { | ||
| context.ReportDiagnostic(diagnosableInvocation.CreateDiagnostic(RecommendCaseInsensitiveStringComparisonRule, diagnosableMethod.Name)); | ||
| } | ||
| else if (diagnosableMethod.Equals(compareToStringMethod)) | ||
| { | ||
| context.ReportDiagnostic(diagnosableInvocation.CreateDiagnostic(RecommendCaseInsensitiveStringComparerRule)); | ||
| } | ||
| }, OperationKind.Invocation); | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.