Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
|Id|Category|Description|Severity|Is enabled|Code fix|Configurable|
|--|--------|-----------|:------:|:--------:|:------:|:----------:|
|[MA0001](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0001.md)|Usage|StringComparison is missing|<span title='Info'>ℹ️</span>|✔️|✔️|❌|
|[MA0002](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0002.md)|Usage|IEqualityComparer\<string\> or IComparer\<string\> is missing|<span title='Warning'>⚠️</span>|✔️|✔️|<span title='MA0002.exclude_query_operator_syntaxes&#xA;MA0002.report_collection_expressions'>✔️</span>|
|[MA0002](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0002.md)|Usage|IEqualityComparer\<string\> or IComparer\<string\> is missing|<span title='Warning'>⚠️</span>|✔️|✔️|<span title='MA0002.exclude_query_operator_syntaxes&#xA;MA0002.report_collection_expressions&#xA;MA0002.report_only_non_ordinal'>✔️</span>|
|[MA0003](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0003.md)|Style|Add parameter name to improve readability|<span title='Info'>ℹ️</span>|✔️|✔️|<span title='MA0003.excluded_methods&#xA;MA0003.excluded_methods_regex&#xA;MA0003.expression_kinds&#xA;MA0003.minimum_method_parameters'>✔️</span>|
|[MA0004](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0004.md)|Usage|Use Task.ConfigureAwait|<span title='Warning'>⚠️</span>|✔️|✔️|<span title='MA0004.report'>✔️</span>|
|[MA0005](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0005.md)|Performance|Use Array.Empty\<T\>()|<span title='Warning'>⚠️</span>|✔️|✔️|❌|
Expand Down
9 changes: 7 additions & 2 deletions docs/Rules/MA0002.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ list.Distinct(StringComparer.Ordinal);

`IQueryable` query-operator calls are excluded from this recommendation because comparer overloads are often not translatable by query providers.

Calls to `Meziantou.Framework.Assertions.Assert` are excluded because assertion methods intentionally choose the comparison semantics.

# Configuration

The rule can be configured using an `.editorconfig` file:
Expand All @@ -57,6 +55,13 @@ MA0002.exclude_query_operator_syntaxes = false
# Report collection expressions ([]) for C#14 and lower
# C# preview reports those diagnostics by default
MA0002.report_collection_expressions = false

# Only report APIs whose default string comparison is not known to be ordinal.
# When enabled, equality-based APIs whose default comparer is already ordinal are not reported
# (e.g. HashSet, Dictionary, ConcurrentDictionary, ToDictionary, ToImmutableDictionary, Distinct,
# GroupBy, and Meziantou.Framework.Assertions.Assert), while culture-sensitive ordering APIs are
# still reported (e.g. SortedDictionary, SortedSet, SortedList, OrderBy, ThenBy, Order).
MA0002.report_only_non_ordinal = false
```

## Additional resources
Expand Down
110 changes: 108 additions & 2 deletions src/Meziantou.Analyzer/Rules/UseStringComparerAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,48 @@ public sealed class UseStringComparerAnalyzer : DiagnosticAnalyzer
{ "ToLookup", 1 },
};

// Methods whose default string comparison is ordinal (equality-based). Ordering methods
// (Order/OrderBy/OrderByDescending/ThenBy/ThenByDescending) are intentionally excluded because
// their default is Comparer<string>.Default (= StringComparer.CurrentCulture, culture-sensitive).
private static readonly HashSet<string> KnownOrdinalMethodNames = new(StringComparer.Ordinal)
{
// System.Linq.Enumerable / System.Linq.Queryable (methods taking IEqualityComparer<string>).
// Ordering methods (Order/OrderBy/OrderByDescending/OrderDescending/ThenBy/ThenByDescending) and
// Min/Max/MinBy/MaxBy take IComparer<string> (culture-sensitive) and are intentionally excluded.
"AggregateBy",
"Contains",
"CountBy",
"Distinct",
"DistinctBy",
"Except",
"ExceptBy",
"GroupBy",
"GroupJoin",
"Intersect",
"IntersectBy",
"Join",
"LeftJoin",
"RightJoin",
"SequenceEqual",
"ToDictionary",
"ToHashSet",
"ToLookup",
"Union",
"UnionBy",

// System.Collections.Immutable / System.Collections.Frozen factory methods. These names also
// exist on the Sorted variants (IComparer<string>, culture-sensitive), but those are excluded
// by scoping to the containers in BuildKnownOrdinalContainerTypes.
"Create",
"CreateBuilder",
"CreateRange",
"CreateRangeWithOverwrite",
"ToImmutableDictionary",
"ToImmutableHashSet",
"ToFrozenDictionary",
"ToFrozenSet",
};

private static readonly DiagnosticDescriptor Rule = new(
RuleIdentifiers.UseStringComparer,
title: "IEqualityComparer<string> or IComparer<string> is missing",
Expand All @@ -53,6 +95,7 @@ public sealed class UseStringComparerAnalyzer : DiagnosticAnalyzer
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseStringComparer));

private static readonly ConfigurationDefinition<bool> ExcludeQueryOperatorSyntaxesConfiguration = new(Rule.Id + ".exclude_query_operator_syntaxes", defaultValue: false);
private static readonly ConfigurationDefinition<bool> ReportOnlyNonOrdinalConfiguration = new(Rule.Id + ".report_only_non_ordinal", defaultValue: false);
#if ROSLYN_4_14_OR_GREATER
private static readonly ConfigurationDefinition<bool> ReportCollectionExpressionsConfiguration = new(Rule.Id + ".report_collection_expressions", defaultValue: false);
#endif
Expand Down Expand Up @@ -80,6 +123,15 @@ private sealed class AnalyzerContext(Compilation compilation)
private readonly OverloadFinder _overloadFinder = new(compilation);
private readonly OperationUtilities _operationUtilities = new(compilation);

// Types whose default string comparison is ordinal (equality-based collections). Ordering
// collections (SortedDictionary/SortedList/SortedSet) are intentionally excluded because their
// default is Comparer<string>.Default (= StringComparer.CurrentCulture, culture-sensitive).
private readonly HashSet<INamedTypeSymbol> _knownOrdinalTypes = BuildKnownOrdinalTypes(compilation);

// Static classes hosting the known-ordinal methods, used to avoid suppressing unrelated
// user-defined methods that happen to share a name with a BCL method.
private readonly HashSet<INamedTypeSymbol> _knownOrdinalContainerTypes = BuildKnownOrdinalContainerTypes(compilation);

public INamedTypeSymbol? EqualityComparerStringType { get; } = GetIEqualityComparerString(compilation);
public INamedTypeSymbol? ComparerStringType { get; } = GetIComparerString(compilation);
public INamedTypeSymbol? EnumerableType { get; } = compilation.GetBestTypeByMetadataName("System.Linq.Enumerable");
Expand All @@ -102,6 +154,9 @@ public void AnalyzeConstructor(OperationAnalysisContext ctx)
if ((EqualityComparerStringType is not null && _overloadFinder.HasOverloadWithAdditionalParameterOfType(method, options: default, [EqualityComparerStringType])) ||
(ComparerStringType is not null && _overloadFinder.HasOverloadWithAdditionalParameterOfType(method, options: default, [ComparerStringType])))
{
if (ctx.Options.GetConfigurationValue(operation, ReportOnlyNonOrdinalConfiguration) && IsKnownOrdinalType(operation.Type))
return;

ctx.ReportDiagnostic(Rule, operation);
}
}
Expand All @@ -116,8 +171,6 @@ public void AnalyzeInvocation(OperationAnalysisContext ctx)
return;

var method = operation.TargetMethod;
if (method.ContainingType.IsEqualTo(MeziantouFrameworkAssertType))
return;

// Most ISet implementation already configured the IEqualityComparer in this constructor,
// so it should be ok to skip method calls on those types.
Expand Down Expand Up @@ -145,6 +198,9 @@ public void AnalyzeInvocation(OperationAnalysisContext ctx)
if ((EqualityComparerStringType is not null && _overloadFinder.HasOverloadWithAdditionalParameterOfType(operation, options: default, [EqualityComparerStringType])) ||
(ComparerStringType is not null && _overloadFinder.HasOverloadWithAdditionalParameterOfType(operation, options: default, [ComparerStringType])))
{
if (IsInvocationReportSuppressedByOrdinalOption(ctx, operation, method))
return;

ctx.ReportDiagnostic(Rule, operation, DefaultDiagnosticInvocationReportOptions);
return;
}
Expand Down Expand Up @@ -179,6 +235,9 @@ public void AnalyzeInvocation(OperationAnalysisContext ctx)

if (!HasEqualityComparerArgument(operation.Arguments))
{
if (IsInvocationReportSuppressedByOrdinalOption(ctx, operation, method))
return;

ctx.ReportDiagnostic(Rule, operation, DefaultDiagnosticInvocationReportOptions);
}
}
Expand Down Expand Up @@ -210,6 +269,9 @@ public void AnalyzeCollectionExpression(OperationAnalysisContext ctx)
if ((EqualityComparerStringType is not null && _overloadFinder.HasOverloadWithAdditionalParameterOfType(constructMethod, options: default, [EqualityComparerStringType])) ||
(ComparerStringType is not null && _overloadFinder.HasOverloadWithAdditionalParameterOfType(constructMethod, options: default, [ComparerStringType])))
{
if (ctx.Options.GetConfigurationValue(operation, ReportOnlyNonOrdinalConfiguration) && IsKnownOrdinalType(operation.Type))
return;

ctx.ReportDiagnostic(Rule, operation);
}

Expand Down Expand Up @@ -267,6 +329,50 @@ private bool HasEqualityComparerConstructArgument(ImmutableArray<IOperation> con
#pragma warning restore RSEXPERIMENTAL006
#endif

private bool IsInvocationReportSuppressedByOrdinalOption(OperationAnalysisContext ctx, IInvocationOperation operation, IMethodSymbol method)
{
if (!ctx.Options.GetConfigurationValue(operation, ReportOnlyNonOrdinalConfiguration))
return false;

if (method.ContainingType.IsEqualTo(MeziantouFrameworkAssertType))
return true;

return KnownOrdinalMethodNames.Contains(method.Name)
&& _knownOrdinalContainerTypes.Contains(method.ContainingType.OriginalDefinition);
}

private bool IsKnownOrdinalType(ITypeSymbol? type)
{
return type is INamedTypeSymbol namedType
&& _knownOrdinalTypes.Contains(namedType.OriginalDefinition);
}

private static HashSet<INamedTypeSymbol> BuildKnownOrdinalTypes(Compilation compilation)
{
var result = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Generic.HashSet`1"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Generic.Dictionary`2"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Generic.OrderedDictionary`2"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Concurrent.ConcurrentDictionary`2"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Immutable.ImmutableDictionary`2"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Immutable.ImmutableHashSet`1"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Frozen.FrozenDictionary`2"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Frozen.FrozenSet`1"));
return result;
}

private static HashSet<INamedTypeSymbol> BuildKnownOrdinalContainerTypes(Compilation compilation)
{
var result = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Linq.Enumerable"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Linq.Queryable"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Immutable.ImmutableDictionary"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Immutable.ImmutableHashSet"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Frozen.FrozenDictionary"));
result.AddIfNotNull(compilation.GetBestTypeByMetadataName("System.Collections.Frozen.FrozenSet"));
return result;
}

private static INamedTypeSymbol? GetIEqualityComparerString(Compilation compilation)
{
var equalityComparerInterfaceType = compilation.GetBestTypeByMetadataName("System.Collections.Generic.IEqualityComparer`1");
Expand Down
Loading