diff --git a/src/Meziantou.Analyzer.CodeFixers/Rules/UseStringComparerFixer.cs b/src/Meziantou.Analyzer.CodeFixers/Rules/UseStringComparerFixer.cs index 8f208f06..f19ea062 100644 --- a/src/Meziantou.Analyzer.CodeFixers/Rules/UseStringComparerFixer.cs +++ b/src/Meziantou.Analyzer.CodeFixers/Rules/UseStringComparerFixer.cs @@ -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; @@ -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(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; @@ -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)); @@ -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 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; + } + + /// + /// Returns the parameters that correspond to actual arguments in the invocation/creation argument list, + /// i.e. excluding the implicit 'this' receiver for extension methods. + /// + private static ImmutableArray 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 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: @@ -90,15 +215,24 @@ private static async Task 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) diff --git a/tests/Meziantou.Analyzer.Test/Rules/UseStringComparerAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/UseStringComparerAnalyzerTests.cs index 198f2f37..0bedffb7 100755 --- a/tests/Meziantou.Analyzer.Test/Rules/UseStringComparerAnalyzerTests.cs +++ b/tests/Meziantou.Analyzer.Test/Rules/UseStringComparerAnalyzerTests.cs @@ -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 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 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 ToDictionaryCustom(this IEnumerable source, System.Func keySelector, CancellationToken ct) => throw null; + public static Dictionary ToDictionaryCustom(this IEnumerable source, System.Func keySelector, IEqualityComparer 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 ToDictionaryCustom(this IEnumerable source, System.Func keySelector, CancellationToken ct) => throw null; + public static Dictionary ToDictionaryCustom(this IEnumerable source, System.Func keySelector, IEqualityComparer comparer, CancellationToken ct) => throw null; + } + """; + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ShouldFixCodeWith(CodeFix) + .ValidateAsync(); + } }