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

Analyzer and fixer that recommend case insensitive string comparison #6662

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
c9b4479
Resources and documentation
carlossanlop May 31, 2023
d85c6a8
Analyzer
carlossanlop May 31, 2023
ab3965f
Fixer
carlossanlop May 31, 2023
9516781
Base test class
carlossanlop May 31, 2023
1a407d4
C# tests
carlossanlop May 31, 2023
5d202a2
VisualBasic tests
carlossanlop May 31, 2023
27acb1c
Address suggestion: Inline localizable strings that are used only once.
carlossanlop May 31, 2023
4298b5c
Address suggestion: Prefer GetSpecialType for string.
carlossanlop May 31, 2023
d6a193c
Use explicit.
carlossanlop May 31, 2023
2ca2441
Do not pass argument to diagnostic that does not need it.
carlossanlop May 31, 2023
c441153
Address suggestion: Use Length instead of Count()
carlossanlop May 31, 2023
8de0cd9
Address suggestion: Improve name of caseChangingMethodName
carlossanlop May 31, 2023
4da0528
Address suggestion: Remove token cancellation handling code
carlossanlop May 31, 2023
0011a6a
Address suggestion: Remove unused cancellation token argument.
carlossanlop May 31, 2023
0bfbc0b
Address suggestion: Add missing fixer attribute, seal class
carlossanlop May 31, 2023
5c16a10
Analyzer: Remove unused using.
carlossanlop Jun 1, 2023
492275d
Address suggestion: Use better test pattern. Simplify tests.
carlossanlop Jun 1, 2023
1098548
More tests
carlossanlop Jun 1, 2023
1a2f5ec
Address suggestion: Await tests.
carlossanlop Jun 1, 2023
1617027
Fix spacing warning
carlossanlop Jun 1, 2023
75109c0
Handle all 3 IndexOf overloads that can be converted to the StringCom…
carlossanlop Jun 1, 2023
0de635c
Address suggestion: Walk up parenthesized operations
carlossanlop Jun 1, 2023
a5e4f97
Add "No diagnostic" tests
carlossanlop Jun 2, 2023
f8d5688
Do not fix CompareTo, only suggest.
carlossanlop Jun 2, 2023
5961fab
Remove unused CompareTo fixer code and fix some warnings.
carlossanlop Jun 2, 2023
fee0f45
Add missing entry for second variant of diagnostic rule message in "R…
carlossanlop Jun 2, 2023
fb4f324
Revert md and sarif changes to fix CI
carlossanlop Jun 2, 2023
673d04c
Address suggestion: Use WalkUpParentheses
carlossanlop Jun 2, 2023
349a06c
Add short string for the codefix CodeAction title
carlossanlop Jun 2, 2023
dd47ee2
Consume codefix CodeAction short string in title
carlossanlop Jun 2, 2023
59c0115
Fix CI
carlossanlop Jun 4, 2023
21cfaaf
Remove some unnecessary fixer code
carlossanlop Jun 4, 2023
cb83e9e
Use better equivalenceKey
carlossanlop Jun 4, 2023
d73eb5c
Diagnose inverted pattern
carlossanlop Jun 4, 2023
7ede45e
Tests for named arguments
carlossanlop Jun 4, 2023
ea7d45d
Change resources to be able to address named parameters
carlossanlop Jun 6, 2023
f37c39e
src: Address named parameters
carlossanlop Jun 6, 2023
3f7a020
tests: Adjust tests to verify named parameters
carlossanlop Jun 6, 2023
801fb99
Move ExportCodeFixProvider and Shared attributes to derived types.
carlossanlop Jun 15, 2023
84745f0
public sealed fixers
carlossanlop Jun 15, 2023
2d3cf5b
Improved resource description mentioning allocation. Use single quote…
carlossanlop Jun 15, 2023
986973c
Remove commented code
carlossanlop Jun 15, 2023
0a2182e
Update xlfs
carlossanlop Jun 15, 2023
4778992
Remove commented code
carlossanlop Jun 15, 2023
e2b4b95
Rename to GetNewArguments
carlossanlop Jun 16, 2023
e570710
Merge remote-tracking branch 'dotnet/main' into RecommendCaseInsensit…
carlossanlop Jun 20, 2023
efabebc
Fix build
carlossanlop Jun 20, 2023
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 @@ -14,6 +14,7 @@ CA1858 | Performance | Info | UseStartsWithInsteadOfIndexOfComparisonWithZero, [
CA1859 | Performance | Info | UseConcreteTypeAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1859)
CA1860 | Performance | Info | PreferLengthCountIsEmptyOverAnyAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1860)
CA1861 | Performance | Info | AvoidConstArrays, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1861)
CA1862 | Performance | Info | RecommendCaseInsensitiveStringComparison, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1862)
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 @@ -2054,4 +2054,22 @@ Widening and user defined conversions are not supported with generic types.</val
<data name="PreferIsEmptyOverAnyMessage" xml:space="preserve">
<value>Prefer an 'IsEmpty' check rather than using 'Any()', both for clarity and for performance</value>
</data>
<data name="RecommendCaseInsensitiveStringComparerTitle" xml:space="preserve">
<value>Prefer using 'StringComparer' to perform case-insensitive string comparisons</value>
</data>
<data name="RecommendCaseInsensitiveStringComparerDescription" xml:space="preserve">
<value>Avoid calling 'ToLower', 'ToUpper', 'ToLowerInvariant' and 'ToUpperInvariant' to perform case-insensitive string comparisons when using 'CompareTo'. Instead, use 'StringComparer' to perform case-insensitive comparisons.</value>
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="RecommendCaseInsensitiveStringComparerMessage" xml:space="preserve">
<value>Prefer using 'StringComparer' to perform a case-insensitive comparison</value>
</data>
<data name="RecommendCaseInsensitiveStringComparisonTitle" xml:space="preserve">
<value>Prefer the 'StringComparison' method overloads to perform case-insensitive string comparisons</value>
</data>
<data name="RecommendCaseInsensitiveStringComparisonDescription" xml:space="preserve">
<value>Avoid calling 'ToLower', 'ToUpper', 'ToLowerInvariant' and 'ToUpperInvariant' to perform case-insensitive string comparisons. Instead, prefer calling the method overloads of 'Contains', 'IndexOf' and 'StartsWith' that take a 'StringComparison' enum value to perform case-insensitive comparisons.</value>
</data>
<data name="RecommendCaseInsensitiveStringComparisonMessage" xml:space="preserve">
<value>Prefer the string comparison method overload of '{0}' that takes a 'StringComparison' enum value to perform a case-insensitive comparison</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
// 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";

private static readonly LocalizableString s_localizableStringComparisonTitle = CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparisonTitle));
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
private static readonly LocalizableString s_localizableStringComparisonMessage = CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparisonMessage));
private static readonly LocalizableString s_localizableStringComparisonDescription = CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparisonDescription));

private static readonly LocalizableString s_localizableStringComparerTitle = CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparerTitle));
private static readonly LocalizableString s_localizableStringComparerMessage = CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparerMessage));
private static readonly LocalizableString s_localizableStringComparerDescription = CreateLocalizableResourceString(nameof(RecommendCaseInsensitiveStringComparerDescription));

internal static readonly DiagnosticDescriptor RecommendCaseInsensitiveStringComparisonRule = DiagnosticDescriptorHelper.Create(
RuleId,
s_localizableStringComparisonTitle,
s_localizableStringComparisonMessage,
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
s_localizableStringComparisonDescription,
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor RecommendCaseInsensitiveStringComparerRule = DiagnosticDescriptorHelper.Create(
RuleId,
s_localizableStringComparerTitle,
s_localizableStringComparerMessage,
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
s_localizableStringComparerDescription,
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

if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemString, out INamedTypeSymbol? stringType))
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}

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;
}

// Retrieve the diagnosable string overload methods: Contains, IndexOf, StartsWith, CompareTo

ParameterInfo[] stringParameter = new[]
{
ParameterInfo.GetParameterInfo(stringType)
};

IMethodSymbol? containsStringMethod = stringType.GetMembers(StringContainsMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(stringParameter);
if (containsStringMethod == null)
{
return;
}

// TODO: There are more overloads that take StringComparison, diagnose only the simple one for now
IMethodSymbol? indexOfStringMethod = stringType.GetMembers(StringIndexOfMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(stringParameter);
if (indexOfStringMethod == null)
{
return;
}

IMethodSymbol? startsWithStringMethod = stringType.GetMembers(StringStartsWithMethodName).OfType<IMethodSymbol>().GetFirstOrDefaultMemberWithParameterInfos(stringParameter);
if (startsWithStringMethod == null)
{
return;
}

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;
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
IMethodSymbol caseChangingMethod = caseChangingInvocation.TargetMethod;

if (!caseChangingMethod.Equals(toLowerParameterlessMethod) &&
!caseChangingMethod.Equals(toLowerInvariantParameterlessMethod) &&
!caseChangingMethod.Equals(toUpperParameterlessMethod) &&
!caseChangingMethod.Equals(toUpperInvariantParameterlessMethod))
{
return;
}

if (caseChangingInvocation.Parent is not IInvocationOperation diagnosableInvocation)
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}

var diagnosableMethod = diagnosableInvocation.TargetMethod;

DiagnosticDescriptor rule;
if (diagnosableMethod.Equals(containsStringMethod) ||
diagnosableMethod.Equals(indexOfStringMethod) ||
diagnosableMethod.Equals(startsWithStringMethod))
{
rule = RecommendCaseInsensitiveStringComparisonRule;
}
else if (diagnosableMethod.Equals(compareToStringMethod))
{
rule = RecommendCaseInsensitiveStringComparerRule;
}
else
{
return;
}

context.ReportDiagnostic(diagnosableInvocation.CreateDiagnostic(rule, diagnosableMethod.Name));
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved

}, OperationKind.Invocation);
}
}
}
Loading