Skip to content
Draft
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
73 changes: 73 additions & 0 deletions Documentation/Diagnostics/PH2164.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# PH2164: String-keyed collections should specify a StringComparer

| Property | Value |
|--|--|
| Package | [Philips.CodeAnalysis.MaintainabilityAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MaintainabilityAnalyzers) |
| Diagnostic ID | PH2164 |
| Category | [Maintainability](../Maintainability.md) |
| Analyzer | [StringDictionaryNeedsComparerAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/StringDictionaryNeedsComparerAnalyzer.cs)
| CodeFix | No |
| Severity | Error |
| Enabled By Default | Yes |

## Introduction

String-keyed collections should explicitly specify a StringComparer (or IComparer<string> for sorted collections) to avoid culture-dependent or case-sensitivity surprises. Without an explicit comparer, collections may use culture-sensitive string comparisons that can lead to unexpected behavior in different locales.

## How to solve

Explicitly specify a StringComparer when creating string-keyed collections:

- For equality-based collections: Use `StringComparer.Ordinal`, `StringComparer.OrdinalIgnoreCase`, or other appropriate StringComparer options
- For sorted collections: Use `StringComparer.Ordinal` (cast to IComparer<string>) or other appropriate comparer

## Example

Code that triggers a diagnostic:
``` cs
class BadExample
{
public void BadMethod()
{
// Missing StringComparer - uses default culture-sensitive comparison
var dict = new Dictionary<string, int>();
var set = new HashSet<string>();

// Also problematic for sorted collections
var sortedDict = new SortedDictionary<string, int>();
var sortedSet = new SortedSet<string>();
}
}
```

And the improved replacement:
``` cs
class GoodExample
{
public void GoodMethod()
{
// Explicit StringComparer for predictable behavior
var dict = new Dictionary<string, int>(StringComparer.Ordinal);
var set = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

// Explicit comparer for sorted collections
var sortedDict = new SortedDictionary<string, int>(StringComparer.Ordinal);
var sortedSet = new SortedSet<string>(StringComparer.OrdinalIgnoreCase);
}
}
```

## Supported Collections

The analyzer detects the following string-keyed collection types:

- `Dictionary<string, TValue>`
- `HashSet<string>`
- `ConcurrentDictionary<string, TValue>` (planned)
- `SortedDictionary<string, TValue>` (planned)
- `SortedSet<string>` (planned)
- `ImmutableDictionary<string, TValue>` (planned)

## Configuration

This analyzer is enabled by default. The general ways of [suppressing](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings) diagnostics apply.
1 change: 1 addition & 0 deletions Philips.CodeAnalysis.Common/DiagnosticId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,6 @@ public enum DiagnosticId
AvoidUnlicensedPackages = 2155,
AvoidPkcsPaddingWithRsaEncryption = 2158,
AvoidUnnecessaryAttributeParentheses = 2159,
StringDictionaryNeedsComparer = 2164,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using Philips.CodeAnalysis.Common;

namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Maintainability
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class StringDictionaryNeedsComparerAnalyzer : DiagnosticAnalyzerBase
{
private const string Title = @"String-keyed collections should specify a StringComparer";
public const string MessageFormat = @"{0} with a string key should explicitly specify a {1}";
private const string Description = @"String-keyed collections should explicitly specify a StringComparer (or IComparer<string> for sorted collections) to avoid culture-dependent or case-sensitivity surprises.";
private const string Category = Categories.Maintainability;

private static readonly DiagnosticDescriptor Rule = new(DiagnosticId.StringDictionaryNeedsComparer.ToId(), Title, MessageFormat, Category, DiagnosticSeverity.Error, isEnabledByDefault: true, description: Description);

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

protected override void InitializeCompilation(CompilationStartAnalysisContext context)
{
var wellKnownTypes = new WellKnownTypes(context.Compilation);

// Debug: Only proceed if we have the basic types
if (wellKnownTypes.String == null || wellKnownTypes.Dictionary_T2 == null)
{
return;
}

context.RegisterOperationAction(ctx => AnalyzeObjectCreation(ctx, wellKnownTypes), OperationKind.ObjectCreation);
context.RegisterOperationAction(ctx => AnalyzeInvocation(ctx, wellKnownTypes), OperationKind.Invocation);
}

private static void AnalyzeObjectCreation(OperationAnalysisContext context, WellKnownTypes types)
{
var creation = (IObjectCreationOperation)context.Operation;
if (creation.Type is not INamedTypeSymbol constructed)
{
return;
}

// Determine if this is one of the supported collection types with string key
if (!TryGetCollectionInfo(constructed, types, out _, out RequiredComparer requiresComparerType))
{
return;
}

// If a comparer argument is already provided, nothing to do
IMethodSymbol ctor = creation.Constructor;
if (ctor == null)
{
return;
}

if (HasRequiredComparerParameter(ctor, requiresComparerType, types))
{
return; // explicit comparer provided
}

// No comparer in the selected overload → report
var collectionDisplay = constructed.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat);
var comparerDisplay = requiresComparerType == RequiredComparer.IEqualityComparer
? "StringComparer/IEqualityComparer<string>"
: "StringComparer/IComparer<string>";

context.ReportDiagnostic(Diagnostic.Create(
Rule,
creation.Syntax.GetLocation(),
collectionDisplay,
comparerDisplay));
}

private static void AnalyzeInvocation(OperationAnalysisContext context, WellKnownTypes types)
{
var invocation = (IInvocationOperation)context.Operation;
IMethodSymbol targetMethod = invocation.TargetMethod;

// ImmutableDictionary.Create<TKey, TValue>(...) without comparer
if (types.ImmutableDictionary != null && targetMethod.ContainingType.Equals(types.ImmutableDictionary, SymbolEqualityComparer.Default) &&
targetMethod.Name is "Create" or "CreateRange")
{
ImmutableArray<ITypeSymbol> typeArgs = targetMethod.TypeArguments;
if (typeArgs.Length == 2 && SymbolEqualityComparer.Default.Equals(typeArgs[0], types.String) &&
!HasRequiredComparerParameter(targetMethod, RequiredComparer.IEqualityComparer, types))
{
var collectionDisplay = $"ImmutableDictionary<{typeArgs[0].ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)}, {typeArgs[1].ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)}>";
context.ReportDiagnostic(Diagnostic.Create(
Rule,
invocation.Syntax.GetLocation(),
collectionDisplay,
"StringComparer/IEqualityComparer<string>"));
}
}
}


private enum RequiredComparer { IEqualityComparer, IComparer }

private static bool TryGetCollectionInfo(INamedTypeSymbol constructed, WellKnownTypes types, out string kind, out RequiredComparer requires)
{
kind = default;
requires = default;

// HashSet<string>
if (types.HashSet_T != null && constructed.ConstructedFrom.Equals(types.HashSet_T, SymbolEqualityComparer.Default))
{
if (constructed.TypeArguments.Length == 1 && SymbolEqualityComparer.Default.Equals(constructed.TypeArguments[0], types.String))
{
kind = "HashSet";
requires = RequiredComparer.IEqualityComparer;
return true;
}
return false;
}

// Dictionary<string, TValue>
if (types.Dictionary_T2 != null && constructed.ConstructedFrom.Equals(types.Dictionary_T2, SymbolEqualityComparer.Default))
{
if (constructed.TypeArguments.Length == 2 && SymbolEqualityComparer.Default.Equals(constructed.TypeArguments[0], types.String))
{
kind = "Dictionary";
requires = RequiredComparer.IEqualityComparer;
return true;
}
return false;
}

// ConcurrentDictionary<string, TValue>
if (types.ConcurrentDictionary_T2 != null && constructed.ConstructedFrom.Equals(types.ConcurrentDictionary_T2, SymbolEqualityComparer.Default))
{
if (constructed.TypeArguments.Length == 2 && SymbolEqualityComparer.Default.Equals(constructed.TypeArguments[0], types.String))
{
kind = "ConcurrentDictionary";
requires = RequiredComparer.IEqualityComparer;
return true;
}
return false;
}

// SortedDictionary<string, TValue>
if (types.SortedDictionary_T2 != null && constructed.ConstructedFrom.Equals(types.SortedDictionary_T2, SymbolEqualityComparer.Default))
{
if (constructed.TypeArguments.Length == 2 && SymbolEqualityComparer.Default.Equals(constructed.TypeArguments[0], types.String))
{
kind = "SortedDictionary";
requires = RequiredComparer.IComparer;
return true;
}
return false;
}

// SortedSet<string>
if (types.SortedSet_T != null && constructed.ConstructedFrom.Equals(types.SortedSet_T, SymbolEqualityComparer.Default))
{
if (constructed.TypeArguments.Length == 1 && SymbolEqualityComparer.Default.Equals(constructed.TypeArguments[0], types.String))
{
kind = "SortedSet";
requires = RequiredComparer.IComparer;
return true;
}
return false;
}

// ImmutableDictionary<string, TValue>
if (types.ImmutableDictionary_T2 != null && constructed.ConstructedFrom.Equals(types.ImmutableDictionary_T2, SymbolEqualityComparer.Default))
{
if (constructed.TypeArguments.Length == 2 && SymbolEqualityComparer.Default.Equals(constructed.TypeArguments[0], types.String))
{
kind = "ImmutableDictionary";
requires = RequiredComparer.IEqualityComparer;
return true;
}
return false;
}

return false;
}

private static bool HasRequiredComparerParameter(IMethodSymbol method, RequiredComparer requires, WellKnownTypes types)
{
foreach (IParameterSymbol param in method.Parameters)
{
if (requires == RequiredComparer.IEqualityComparer &&
types.IEqualityComparer_T != null &&
param.Type is INamedTypeSymbol p &&
p.OriginalDefinition.Equals(types.IEqualityComparer_T, SymbolEqualityComparer.Default) &&
p.TypeArguments.Length == 1 && SymbolEqualityComparer.Default.Equals(p.TypeArguments[0], types.String))
{
return true;
}

if (requires == RequiredComparer.IComparer &&
types.IComparer_T != null &&
param.Type is INamedTypeSymbol p2 &&
p2.OriginalDefinition.Equals(types.IComparer_T, SymbolEqualityComparer.Default) &&
p2.TypeArguments.Length == 1 && SymbolEqualityComparer.Default.Equals(p2.TypeArguments[0], types.String))
{
return true;
}
}

return false;
}

private sealed class WellKnownTypes
{
public INamedTypeSymbol String { get; }
public INamedTypeSymbol IEqualityComparer_T { get; }
public INamedTypeSymbol IComparer_T { get; }
public INamedTypeSymbol Dictionary_T2 { get; }
public INamedTypeSymbol HashSet_T { get; }
public INamedTypeSymbol ConcurrentDictionary_T2 { get; }
public INamedTypeSymbol SortedDictionary_T2 { get; }
public INamedTypeSymbol SortedSet_T { get; }
public INamedTypeSymbol ImmutableDictionary_T2 { get; }
public INamedTypeSymbol ImmutableDictionary { get; }

public WellKnownTypes(Compilation compilation)
{
String = compilation.GetSpecialType(SpecialType.System_String);
IEqualityComparer_T = compilation.GetTypeByMetadataName("System.Collections.Generic.IEqualityComparer`1");
IComparer_T = compilation.GetTypeByMetadataName("System.Collections.Generic.IComparer`1");
Dictionary_T2 = compilation.GetTypeByMetadataName("System.Collections.Generic.Dictionary`2");
HashSet_T = compilation.GetTypeByMetadataName("System.Collections.Generic.HashSet`1");
ConcurrentDictionary_T2 = compilation.GetTypeByMetadataName("System.Collections.Concurrent.ConcurrentDictionary`2");
SortedDictionary_T2 = compilation.GetTypeByMetadataName("System.Collections.Generic.SortedDictionary`2");
SortedSet_T = compilation.GetTypeByMetadataName("System.Collections.Generic.SortedSet`1");
ImmutableDictionary_T2 = compilation.GetTypeByMetadataName("System.Collections.Immutable.ImmutableDictionary`2");
ImmutableDictionary = compilation.GetTypeByMetadataName("System.Collections.Immutable.ImmutableDictionary");
}
}
}
}
Loading