Skip to content

Fix MA0002 code fix inserting StringComparer at wrong argument position #1252

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.

Sign up for GitHub

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

Merged
meziantou merged 1 commit into from
Jul 29, 2026
+228 −16
Merged

Fix MA0002 code fix inserting StringComparer at wrong argument position #1252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
166 changes: 150 additions & 16 deletions src/Meziantou.Analyzer.CodeFixers/Rules/UseStringComparerFixer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.Simplification;

namespace Meziantou.Analyzer.Rules;
Expand All @@ -23,13 +24,11 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
// In case the target expression is wrapped in another node with the same span,
// get the innermost node for ties.
var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true);
var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true)
?.FirstAncestorOrSelf<SyntaxNode>(CanFix);
if (nodeToFix is null)
return;

if (!CanFix(nodeToFix))
return;

var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
if (semanticModel is null)
return;
Expand All @@ -38,6 +37,45 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
if (stringComparerSymbol is null)
return;

var equalityComparerOpenType = semanticModel.Compilation.GetBestTypeByMetadataName("System.Collections.Generic.IEqualityComparer`1");
var comparerOpenType = semanticModel.Compilation.GetBestTypeByMetadataName("System.Collections.Generic.IComparer`1");

var insertionIndex = -1;
var parameterName = string.Empty;

var operation = semanticModel.GetOperation(nodeToFix, context.CancellationToken);
IMethodSymbol? currentMethod = operation switch
{
IInvocationOperation invocation => invocation.TargetMethod,
IObjectCreationOperation creation => creation.Constructor,
_ => null,
};

if (currentMethod is not null && (equalityComparerOpenType is not null || comparerOpenType is not null))
{
var overloadFinder = new OverloadFinder(semanticModel.Compilation);
var equalityComparerStringType = GetIEqualityComparerString(semanticModel.Compilation);
var comparerStringType = GetIComparerString(semanticModel.Compilation);

IMethodSymbol? targetOverload = null;
if (equalityComparerStringType is not null)
{
targetOverload = overloadFinder.FindOverloadWithAdditionalParameterOfType(
currentMethod, new OverloadOptions { SyntaxNode = nodeToFix }, [equalityComparerStringType]);
}

if (targetOverload is null && comparerStringType is not null)
{
targetOverload = overloadFinder.FindOverloadWithAdditionalParameterOfType(
currentMethod, new OverloadOptions { SyntaxNode = nodeToFix }, [comparerStringType]);
}

if (targetOverload is not null)
{
TryGetComparerParameterInfo(currentMethod, targetOverload, equalityComparerOpenType, comparerOpenType, out insertionIndex, out parameterName);
}
}

RegisterCodeFix(nameof(StringComparer.Ordinal));
RegisterCodeFix(nameof(StringComparer.OrdinalIgnoreCase));

Expand All @@ -46,36 +84,123 @@ void RegisterCodeFix(string comparerName)
var title = "Add StringComparer." + comparerName;
var codeAction = CodeAction.Create(
title,
ct => AddStringComparer(context.Document, nodeToFix, comparerName, stringComparerSymbol, ct),
ct => AddStringComparer(context.Document, nodeToFix, comparerName, stringComparerSymbol, insertionIndex, parameterName, ct),
equivalenceKey: title);

context.RegisterCodeFix(codeAction, context.Diagnostics);
}
}

private static async Task<Document> AddStringComparer(Document document, SyntaxNode nodeToFix, string comparerName, INamedTypeSymbol stringComparer, CancellationToken cancellationToken)
private static bool TryGetComparerParameterInfo(IMethodSymbol method, IMethodSymbol overload, INamedTypeSymbol? equalityComparerOpenType, INamedTypeSymbol? comparerOpenType, out int insertionIndex, out string parameterName)
{
// Use comparable parameters to correctly handle extension methods:
// strip the implicit 'this' parameter so indices align with the argument list.
var methodParams = GetComparableParameters(method);
var overloadParams = GetComparableParameters(overload);

for (var i = 0; i < overloadParams.Length; i++)
{
var parameter = overloadParams[i];
var originalDef = parameter.Type.OriginalDefinition;
if ((equalityComparerOpenType is null || !originalDef.IsEqualTo(equalityComparerOpenType)) &&
(comparerOpenType is null || !originalDef.IsEqualTo(comparerOpenType)))
{
continue;
}

if (i >= methodParams.Length || !IsComparerType(methodParams[i].Type, equalityComparerOpenType, comparerOpenType))
{
insertionIndex = i;
parameterName = parameter.Name;
return true;
}
}

insertionIndex = -1;
parameterName = string.Empty;
return false;
}

/// <summary>
/// Returns the parameters that correspond to actual arguments in the invocation/creation argument list,
/// i.e. excluding the implicit 'this' receiver for extension methods.
/// </summary>
private static ImmutableArray<IParameterSymbol> GetComparableParameters(IMethodSymbol method)
{
// Reduced extension method: Parameters already exclude 'this'.
if (method.MethodKind is MethodKind.ReducedExtension)
return method.Parameters;

// Non-reduced extension method: the first parameter is 'this' and is not an argument.
if (method.IsExtensionMethod && method.Parameters.Length > 0)
return method.Parameters.RemoveAt(0);

return method.Parameters;
}

private static bool IsComparerType(ITypeSymbol type, INamedTypeSymbol? equalityComparerOpenType, INamedTypeSymbol? comparerOpenType)
{
var originalDef = type.OriginalDefinition;
return (equalityComparerOpenType is not null && originalDef.IsEqualTo(equalityComparerOpenType)) ||
(comparerOpenType is not null && originalDef.IsEqualTo(comparerOpenType));
}

private static INamedTypeSymbol? GetIEqualityComparerString(Compilation compilation)
{
var openType = compilation.GetBestTypeByMetadataName("System.Collections.Generic.IEqualityComparer`1");
if (openType is null)
return null;

return openType.Construct(compilation.GetSpecialType(SpecialType.System_String));
}

private static INamedTypeSymbol? GetIComparerString(Compilation compilation)
{
var openType = compilation.GetBestTypeByMetadataName("System.Collections.Generic.IComparer`1");
if (openType is null)
return null;

return openType.Construct(compilation.GetSpecialType(SpecialType.System_String));
}

private static async Task<Document> AddStringComparer(Document document, SyntaxNode nodeToFix, string comparerName, INamedTypeSymbol stringComparer, int insertionIndex, string parameterName, CancellationToken cancellationToken)
{
var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
var generator = editor.Generator;

var newArgument = (ArgumentSyntax)generator.Argument(
generator.MemberAccessExpression(
generator.TypeExpression(stringComparer, addImport: true),
comparerName));
var comparerExpression = generator.MemberAccessExpression(
generator.TypeExpression(stringComparer, addImport: true),
comparerName);

switch (nodeToFix)
{
case ObjectCreationExpressionSyntax creationExpression:
editor.ReplaceNode(creationExpression, AddArgument(creationExpression, newArgument));
editor.ReplaceNode(creationExpression, AddArgument(creationExpression, comparerExpression, insertionIndex, parameterName, generator));
break;

case ImplicitObjectCreationExpressionSyntax implicitCreationExpression:
editor.ReplaceNode(implicitCreationExpression, implicitCreationExpression.AddArgumentListArguments(newArgument));
{
var args = implicitCreationExpression.ArgumentList.Arguments;
var newArguments = insertionIndex >= 0 && insertionIndex < args.Count
? args.Insert(insertionIndex, (ArgumentSyntax)generator.Argument(comparerExpression))
: args.Add(insertionIndex > args.Count
? (ArgumentSyntax)generator.Argument(parameterName, RefKind.None, comparerExpression)
: (ArgumentSyntax)generator.Argument(comparerExpression));
editor.ReplaceNode(implicitCreationExpression, implicitCreationExpression.WithArgumentList(implicitCreationExpression.ArgumentList.WithArguments(newArguments)));
break;
}

case InvocationExpressionSyntax invocationExpression:
editor.ReplaceNode(invocationExpression, invocationExpression.AddArgumentListArguments(newArgument));
{
var args = invocationExpression.ArgumentList.Arguments;
var newArguments = insertionIndex >= 0 && insertionIndex < args.Count
? args.Insert(insertionIndex, (ArgumentSyntax)generator.Argument(comparerExpression))
: args.Add(insertionIndex > args.Count
? (ArgumentSyntax)generator.Argument(parameterName, RefKind.None, comparerExpression)
: (ArgumentSyntax)generator.Argument(comparerExpression));
editor.ReplaceNode(invocationExpression, invocationExpression.WithArgumentList(invocationExpression.ArgumentList.WithArguments(newArguments)));
break;
}

#if CSHARP15_OR_GREATER
case CollectionExpressionSyntax collectionExpression:
Expand All @@ -90,15 +215,24 @@ private static async Task<Document> AddStringComparer(Document document, SyntaxN
return editor.GetChangedDocument();
}

private static ObjectCreationExpressionSyntax AddArgument(ObjectCreationExpressionSyntax creationExpression, ArgumentSyntax argument)
private static ObjectCreationExpressionSyntax AddArgument(ObjectCreationExpressionSyntax creationExpression, SyntaxNode comparerExpression, int insertionIndex, string parameterName, SyntaxGenerator generator)
{
if (creationExpression.ArgumentList is not null)
return creationExpression.AddArgumentListArguments(argument);
{
var args = creationExpression.ArgumentList.Arguments;
var newArguments = insertionIndex >= 0 && insertionIndex < args.Count
? args.Insert(insertionIndex, (ArgumentSyntax)generator.Argument(comparerExpression))
: args.Add(insertionIndex > args.Count
? (ArgumentSyntax)generator.Argument(parameterName, RefKind.None, comparerExpression)
: (ArgumentSyntax)generator.Argument(comparerExpression));
return creationExpression.WithArgumentList(creationExpression.ArgumentList.WithArguments(newArguments));
}

var trailingTrivia = creationExpression.Type.GetTrailingTrivia();
var newArgument = (ArgumentSyntax)generator.Argument(comparerExpression);
return creationExpression
.WithType(creationExpression.Type.WithoutTrailingTrivia())
.WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList(argument)).WithTrailingTrivia(trailingTrivia));
.WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList(newArgument)).WithTrailingTrivia(trailingTrivia));
}

private static bool CanFix(SyntaxNode nodeToFix)
Expand Down
78 changes: 78 additions & 0 deletions tests/Meziantou.Analyzer.Test/Rules/UseStringComparerAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1360,4 +1360,82 @@ await CreateProjectBuilder()
.WithSourceCode(SourceCode)
.ValidateAsync();
}

[Fact]
public async Task CodeFix_InsertComparerBeforeMessage_Issue1249()
{
const string SourceCode = """
using System.Collections.Generic;
class Sample
{
void Test()
{
AreEqual[|("a", "b", "message")|];
}

static void AreEqual(string expected, string actual, string message) { }
static void AreEqual(string expected, string actual, IComparer<string> comparer, string message) { }
}
""";
const string CodeFix = """
using System.Collections.Generic;
class Sample
{
void Test()
{
AreEqual("a", "b", System.StringComparer.Ordinal, "message");
}

static void AreEqual(string expected, string actual, string message) { }
static void AreEqual(string expected, string actual, IComparer<string> comparer, string message) { }
}
""";
await CreateProjectBuilder()
.WithSourceCode(SourceCode)
.ShouldFixCodeWith(CodeFix)
.ValidateAsync();
}

[Fact]
public async Task CodeFix_InsertComparerBeforeCancellationToken_Issue1250()
{
const string SourceCode = """
using System.Collections.Generic;
using System.Threading;
class Sample
{
void Test(CancellationToken ct)
{
var list = new string[0];
list.[|ToDictionaryCustom(s => s, ct)|];
}
}
static class Extensions
{
public static Dictionary<TKey, T> ToDictionaryCustom<T, TKey>(this IEnumerable<T> source, System.Func<T, TKey> keySelector, CancellationToken ct) => throw null;
public static Dictionary<TKey, T> ToDictionaryCustom<T, TKey>(this IEnumerable<T> source, System.Func<T, TKey> keySelector, IEqualityComparer<TKey> comparer, CancellationToken ct) => throw null;
}
""";
const string CodeFix = """
using System.Collections.Generic;
using System.Threading;
class Sample
{
void Test(CancellationToken ct)
{
var list = new string[0];
list.ToDictionaryCustom(s => s, System.StringComparer.Ordinal, ct);
}
}
static class Extensions
{
public static Dictionary<TKey, T> ToDictionaryCustom<T, TKey>(this IEnumerable<T> source, System.Func<T, TKey> keySelector, CancellationToken ct) => throw null;
public static Dictionary<TKey, T> ToDictionaryCustom<T, TKey>(this IEnumerable<T> source, System.Func<T, TKey> keySelector, IEqualityComparer<TKey> comparer, CancellationToken ct) => throw null;
}
""";
await CreateProjectBuilder()
.WithSourceCode(SourceCode)
.ShouldFixCodeWith(CodeFix)
.ValidateAsync();
}
}