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
36 changes: 28 additions & 8 deletions src/Meziantou.Analyzer/Rules/UseStringComparerAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,16 +181,10 @@ public void AnalyzeInvocation(OperationAnalysisContext ctx, OverloadLookupCache
// Most ISet implementation already configured the IEqualityComparer in this constructor,
// so it should be ok to skip method calls on those types.
// A concrete use-case is HashSet<string>.Contains which has an extension method IEnumerable.Contains(value, comparer)
// The instance of the call is checked by IsCalledOnSetInstance, only before a report.
foreach (var type in (ReadOnlySpan<ITypeSymbol?>)[ISetType, IReadOnlySetType, IImmutableSetType])
{

if (type is null)
continue;

if (method.ContainingType.IsOrImplements(type))
return;

if (operation.Instance is not null && operation.Instance.GetActualType(ctx.CancellationToken)?.IsOrImplements(type) is true)
if (type is not null && method.ContainingType.IsOrImplements(type))
return;
}

Expand All @@ -203,6 +197,9 @@ public void AnalyzeInvocation(OperationAnalysisContext ctx, OverloadLookupCache

if (HasOverloadWithComparer(ctx, operation, lookupCache, out var namespaceToImport))
{
if (IsCalledOnSetInstance(operation, ctx.CancellationToken))
return;

if (IsInvocationReportSuppressedByOrdinalOption(ctx, operation, method))
return;

Expand Down Expand Up @@ -252,6 +249,29 @@ public void AnalyzeInvocation(OperationAnalysisContext ctx, OverloadLookupCache
}
}

/// <summary>
/// Indicates whether the instance of the call is a set of strings, whatever the declared type of the expression.
/// Finding the actual type runs a data flow analysis of the local the instance comes from, so it is done once, and
/// only for a call that would be reported.
/// </summary>
private bool IsCalledOnSetInstance(IInvocationOperation operation, CancellationToken cancellationToken)
{
if (operation.Instance is null)
return false;

var instanceType = operation.Instance.GetActualType(cancellationToken);
if (instanceType is null)
return false;

foreach (var type in (ReadOnlySpan<ITypeSymbol?>)[ISetType, IReadOnlySetType, IImmutableSetType])
{
if (type is not null && instanceType.IsOrImplements(type))
return true;
}

return false;
}

/// <summary>
/// Indicates whether the invoked method has an overload with an <c>IEqualityComparer&lt;string&gt;</c> or an <c>IComparer&lt;string&gt;</c>
/// parameter. An overload that does not require a new using directive is preferred. <paramref name="namespaceToImport"/> is set when the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2024,4 +2024,83 @@ public static class SampleExtensions

return test.RunAsync();
}

[Fact]
public Task MethodOnLocalHoldingSetDeclaredAsInterface_ShouldNotReportDiagnostic()
{
var test = CreateTest();
test.TestCode = """
class TypeName
{
public void Test()
{
IContainer values = new CustomSet();
_ = values.Contains("a");
}
}

interface IContainer
{
bool Contains(string value);
bool Contains(string value, System.Collections.Generic.IEqualityComparer<string> comparer);
}

sealed class CustomSet : System.Collections.Generic.HashSet<string>, IContainer
{
public bool Contains(string value, System.Collections.Generic.IEqualityComparer<string> comparer) => throw null;
}
""";

return test.RunAsync();
}

[Fact]
public Task MethodOnLocalHoldingListDeclaredAsInterface_ShouldReportDiagnostic()
{
var test = CreateTest();
test.TestCode = """
class TypeName
{
public void Test()
{
IContainer values = new CustomList();
_ = values.{|MA0002:Contains("a")|};
}
}

interface IContainer
{
bool Contains(string value);
bool Contains(string value, System.Collections.Generic.IEqualityComparer<string> comparer);
}

sealed class CustomList : System.Collections.Generic.List<string>, IContainer
{
public bool Contains(string value, System.Collections.Generic.IEqualityComparer<string> comparer) => throw null;
}
""";
test.FixedCode = """
class TypeName
{
public void Test()
{
IContainer values = new CustomList();
_ = values.Contains("a", System.StringComparer.Ordinal);
}
}

interface IContainer
{
bool Contains(string value);
bool Contains(string value, System.Collections.Generic.IEqualityComparer<string> comparer);
}

sealed class CustomList : System.Collections.Generic.List<string>, IContainer
{
public bool Contains(string value, System.Collections.Generic.IEqualityComparer<string> comparer) => throw null;
}
""";

return test.RunAsync();
}
}
Loading