Fix fixer for MA0089 (#543)

src/Meziantou.Analyzer/Rules/DoNotUseEqualityOperatorsForSpanOfCharFixer.cs

@@ -29,22 +29,16 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
         if (nodeToFix == null)
             return;
 
-        RegisterCodeFix(nameof(StringComparison.Ordinal));
-        RegisterCodeFix(nameof(StringComparison.OrdinalIgnoreCase));
+        var title = "Use SequenceEquals";
+        var codeAction = CodeAction.Create(
+            title,
+            ct => Refactor(context.Document, nodeToFix, ct),
+            equivalenceKey: title);
 
-        void RegisterCodeFix(string comparisonMode)
-        {
-            var title = "Use SequenceEquals " + comparisonMode;
-            var codeAction = CodeAction.Create(
-                title,
-                ct => Refactor(context.Document, nodeToFix, comparisonMode, ct),
-                equivalenceKey: title);
-
-            context.RegisterCodeFix(codeAction, context.Diagnostics);
-        }
+        context.RegisterCodeFix(codeAction, context.Diagnostics);
     }
 
-    private static async Task<Document> Refactor(Document document, SyntaxNode nodeToFix, string comparisonMode, CancellationToken cancellationToken)
+    private static async Task<Document> Refactor(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken)
     {
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
         var semanticModel = editor.SemanticModel;
@@ -54,14 +48,8 @@ private static async Task<Document> Refactor(Document document, SyntaxNode nodeT
         if (operation == null)
             return document;
 
-        var stringComparison = semanticModel.Compilation.GetBestTypeByMetadataName("System.StringComparison");
-        if (stringComparison is null)
-            return document;
-
         var newExpression = generator.InvocationExpression(
-            generator.MemberAccessExpression(operation.LeftOperand.Syntax, "SequenceEqual"),
-            operation.RightOperand.Syntax,
-            generator.MemberAccessExpression(generator.TypeExpression(stringComparison, addImport: true), comparisonMode));
+            generator.MemberAccessExpression(operation.LeftOperand.Syntax, "SequenceEqual"), operation.RightOperand.Syntax);
 
         if (operation.OperatorKind == BinaryOperatorKind.NotEquals)
         {

src/Meziantou.Analyzer/Rules/OptimizeLinqUsageAnalyzer.cs

@@ -6,6 +6,7 @@
 using Meziantou.Analyzer.Configurations;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Operations;
 using static System.FormattableString;
@@ -775,11 +776,16 @@ private static void UseCastInsteadOfSelect(OperationAnalysisContext context, IIn
                 NullableFlowState.MaybeNull :
                 NullableFlowState.None;
 
+            var typeSyntax = castOp.Syntax;
+            if (typeSyntax is CastExpressionSyntax castSyntax)
+            {
+                typeSyntax = castSyntax.Type;
+            }
+
             // Get the cast type's minimally qualified name, in the current context
-            var castType = castOp.Type.ToMinimalDisplayString(semanticModel, nullableFlowState, operation.Syntax.SpanStart);
-            var properties = CreateProperties(OptimizeLinqUsageData.UseCastInsteadOfSelect)
-               .Add("CastType", castType);
+            var properties = CreateProperties(OptimizeLinqUsageData.UseCastInsteadOfSelect);
 
+            var castType = castOp.Type.ToMinimalDisplayString(semanticModel, nullableFlowState, operation.Syntax.SpanStart);
             context.ReportDiagnostic(s_useCastInsteadOfSelect, properties, operation, DiagnosticReportOptions.ReportOnMethodName, castType);
 
             static bool CanReplaceByCast(IConversionOperation op)

src/Meziantou.Analyzer/Rules/OptimizeLinqUsageFixer.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Immutable;
+using System.ComponentModel.Design.Serialization;
 using System.Composition;
 using System.Globalization;
 using System.Linq;
@@ -75,15 +76,15 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
             case OptimizeLinqUsageData.UseFindMethodWithConversion:
                 context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "Find", convertPredicate: true, ct), equivalenceKey: title), context.Diagnostics);
                 break;
-                
+
             case OptimizeLinqUsageData.UseTrueForAllMethod:
                 context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "TrueForAll", convertPredicate: false, ct), equivalenceKey: title), context.Diagnostics);
                 break;
 
             case OptimizeLinqUsageData.UseTrueForAllMethodWithConversion:
                 context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "TrueForAll", convertPredicate: true, ct), equivalenceKey: title), context.Diagnostics);
                 break;
-                
+
             case OptimizeLinqUsageData.UseExistsMethod:
                 context.RegisterCodeFix(CodeAction.Create(title, ct => UseListMethod(context.Document, nodeToFix, "Exists", convertPredicate: false, ct), equivalenceKey: title), context.Diagnostics);
                 break;
@@ -144,7 +145,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
                 break;
 
             case OptimizeLinqUsageData.UseCastInsteadOfSelect:
-                context.RegisterCodeFix(CodeAction.Create(title, ct => UseCastInsteadOfSelect(context.Document, diagnostic, nodeToFix, ct), equivalenceKey: title), context.Diagnostics);
+                context.RegisterCodeFix(CodeAction.Create(title, ct => UseCastInsteadOfSelect(context.Document, nodeToFix, ct), equivalenceKey: title), context.Diagnostics);
                 break;
         }
     }
@@ -311,24 +312,25 @@ private static async Task<Document> UseSkipAndAny(Document document, Diagnostic
         return editor.GetChangedDocument();
     }
 
-    private static async Task<Document> UseCastInsteadOfSelect(Document document, Diagnostic diagnostic, SyntaxNode nodeToFix, CancellationToken cancellationToken)
+    private static async Task<Document> UseCastInsteadOfSelect(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken)
     {
         if (nodeToFix is not InvocationExpressionSyntax selectInvocationExpression)
             return document;
 
         if (selectInvocationExpression.Expression is not MemberAccessExpressionSyntax memberAccessExpression)
             return document;
 
-        // Build the 'Cast<CastType>' name
-        var castType = diagnostic.Properties.GetValueOrDefault("CastType");
-        var castNameSyntax = GenericName(Identifier("Cast"))
-            .WithTypeArgumentList(
-                TypeArgumentList(
-                    SingletonSeparatedList<TypeSyntax>(
-                        IdentifierName(castType!))));
-
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
         var generator = editor.Generator;
+        var operation = editor.SemanticModel.GetOperation(selectInvocationExpression, cancellationToken) as IInvocationOperation;
+        if (operation == null)
+            return document;
+
+        var type = operation.TargetMethod.TypeArguments[1];
+        var typeSyntax = (TypeSyntax)generator.TypeExpression(type);
+
+        var castNameSyntax = GenericName(Identifier("Cast"))
+            .WithTypeArgumentList(TypeArgumentList(SingletonSeparatedList(typeSyntax)));
 
         // Is the 'source' (i.e. the sequence of values 'Select' is invoked on) passed in as argument?
         //  If there is 1 argument      -> No 'source' argument, only 'selector'
@@ -419,7 +421,7 @@ private async static Task<Document> UseListMethod(Document document, SyntaxNode
         {
             var compilation = editor.SemanticModel.Compilation;
             var symbol = editor.SemanticModel.GetSymbolInfo(nodeToFix, cancellationToken: cancellationToken).Symbol as IMethodSymbol;
-            if(symbol == null || symbol.TypeArguments.Length != 1)
+            if (symbol == null || symbol.TypeArguments.Length != 1)
                 return document;
 
             var type = symbol.TypeArguments[0];

src/Meziantou.Analyzer/Rules/OptimizeStartsWithAnalyzer.cs

@@ -219,7 +219,8 @@ public void AnalyzeInvocation(OperationAnalysisContext context)
                         if (operation.Arguments[0].Value is { Type.SpecialType: SpecialType.System_String, ConstantValue: { HasValue: true, Value: string { Length: 1 } } } &&
                             operation.Arguments[1].Value is { Type.SpecialType: SpecialType.System_String, ConstantValue: { HasValue: true, Value: string { Length: 1 } } })
                         {
-                            context.ReportDiagnostic(s_rule, operation);
+                            // Improve the error message as the rule is reported on the method
+                            context.ReportDiagnostic(s_rule, ImmutableDictionary<string, string?>.Empty, operation, DiagnosticReportOptions.ReportOnMethodName);
                         }
                     }
                     else if (operation.Arguments.Length == 3)
@@ -228,7 +229,7 @@ public void AnalyzeInvocation(OperationAnalysisContext context)
                             operation.Arguments[1].Value is { Type.SpecialType: SpecialType.System_String, ConstantValue: { HasValue: true, Value: string { Length: 1 } } } &&
                             operation.Arguments[2].Value is { ConstantValue: { HasValue: true, Value: (int)StringComparison.Ordinal } })
                         {
-                            context.ReportDiagnostic(s_rule, operation);
+                            context.ReportDiagnostic(s_rule, ImmutableDictionary<string, string?>.Empty, operation, DiagnosticReportOptions.ReportOnMethodName);
                         }
                     }
                 }

src/Meziantou.Analyzer/Rules/OptimizeStartsWithFixer.cs

@@ -1,10 +1,13 @@
 using System.Collections.Immutable;
 using System.Composition;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Differencing;
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.Operations;
 
@@ -39,15 +42,48 @@ private static async Task<Document> Fix(Document document, SyntaxNode nodeToFix,
     {
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
         var operation = editor.SemanticModel.GetOperation(nodeToFix, cancellationToken);
+        if (operation == null && !nodeToFix.IsKind(Microsoft.CodeAnalysis.CSharp.SyntaxKind.InvocationExpression))
+        {
+            var invocationNode = nodeToFix.AncestorsAndSelf().OfType<InvocationExpressionSyntax>().FirstOrDefault();
+            if (invocationNode == null)
+                return document;
+
+            operation = editor.SemanticModel.GetOperation(invocationNode, cancellationToken);
+            if (operation == null)
+                return document;
+        }
+
+        var invocation = GetInvocationOperation(operation);
+
+        static IInvocationOperation? GetInvocationOperation(IOperation? operation)
+        {
+            if (operation is IInvocationOperation invocationOperation)
+                return invocationOperation;
+
+            return operation?.Ancestors().OfType<IInvocationOperation>().FirstOrDefault();
+        }
+
         if (operation is ILiteralOperation literalOperation && literalOperation.ConstantValue.Value is string { Length: 1 } literalValue)
         {
             editor.ReplaceNode(literalOperation.Syntax, editor.Generator.LiteralExpression(literalValue[0]));
+
+            if (invocation?.TargetMethod.Name is "StartsWith" or "EndsWith" or "LastIndexOf")
+            {
+                RemoveStringComparisonArgument(editor, invocation);
+            }
+            else if (invocation?.TargetMethod.Name is "IndexOf")
+            {
+                if (invocation.TargetMethod.Parameters.Length > 1 && invocation.TargetMethod.Parameters[1].Type.IsInt32())
+                {
+                    RemoveStringComparisonArgument(editor, invocation);
+                }
+            }
         }
         else if (operation is IArgumentOperation argumentOperation && argumentOperation.Value.ConstantValue.Value is string { Length: 1 } argumentValue)
         {
             editor.ReplaceNode(argumentOperation.Value.Syntax, editor.Generator.LiteralExpression(argumentValue[0]));
         }
-        else if (operation is IInvocationOperation invocation && invocation.TargetMethod.Name == "Replace" && invocation.Arguments.Length >= 2)
+        else if (invocation != null && invocation.TargetMethod.Name == "Replace" && invocation.Arguments.Length >= 2)
         {
             for (var i = 0; i < 2; i++)
             {
@@ -56,8 +92,23 @@ private static async Task<Document> Fix(Document document, SyntaxNode nodeToFix,
                     editor.ReplaceNode(invocation.Arguments[i].Value.Syntax, editor.Generator.LiteralExpression(arg[0]));
                 }
             }
+
+            RemoveStringComparisonArgument(editor, invocation);
         }
 
         return editor.GetChangedDocument();
     }
+
+    private static void RemoveStringComparisonArgument(DocumentEditor editor, IInvocationOperation invocation)
+    {
+        for (var i = invocation.Arguments.Length - 1; i >= 0; i--)
+        {
+            var argument = invocation.Arguments[i];
+            if (argument.Parameter?.Type.IsEqualTo(editor.SemanticModel.Compilation.GetBestTypeByMetadataName("System.StringComparison")) is true)
+            {
+                editor.RemoveNode(argument.Syntax);
+                return;
+            }
+        }
+    }
 }

tests/Meziantou.Analyzer.Test/Helpers/ProjectBuilder.Validation.cs

@@ -495,7 +495,7 @@ private async Task VerifyFix(IList<DiagnosticAnalyzer> analyzers, CodeFixProvide
                 var fixAllContext = new FixAllContext(document, codeFixProvider, FixAllScope.Document, action.EquivalenceKey, analyzerDiagnostics.Select(d => d.Id).Distinct(StringComparer.Ordinal), new CustomDiagnosticProvider(analyzerDiagnostics), CancellationToken.None);
                 var fixes = await codeFixProvider.GetFixAllProvider().GetFixAsync(fixAllContext).ConfigureAwait(false);
 
-                document = await ApplyFix(document, fixes).ConfigureAwait(false);
+                document = await ApplyFix(document, fixes, mustCompile: IsValidFixCode).ConfigureAwait(false);
             }
         }
         else
@@ -512,24 +512,46 @@ private async Task VerifyFix(IList<DiagnosticAnalyzer> analyzers, CodeFixProvide
 
                 if (codeFixIndex != null)
                 {
-                    document = await ApplyFix(document, actions[(int)codeFixIndex]).ConfigureAwait(false);
+                    document = await ApplyFix(document, actions[(int)codeFixIndex], mustCompile: IsValidFixCode).ConfigureAwait(false);
                     break;
                 }
 
-                document = await ApplyFix(document, actions[0]).ConfigureAwait(false);
+                document = await ApplyFix(document, actions[0], mustCompile: IsValidFixCode).ConfigureAwait(false);
                 analyzerDiagnostics = await GetSortedDiagnosticsFromDocuments(analyzers, new[] { document }, compileSolution: false).ConfigureAwait(false);
             }
         }
 
-        //after applying all of the code fixes, compare the resulting string to the inputted one
+        // after applying all of the code fixes, compare the resulting string to the inputted one
         var actual = await GetStringFromDocument(document).ConfigureAwait(false);
         Assert.Equal(newSource, actual, ignoreLineEndingDifferences: true);
     }
 
-    private static async Task<Document> ApplyFix(Document document, CodeAction codeAction)
+    private async Task<Document> ApplyFix(Document document, CodeAction codeAction, bool mustCompile)
     {
         var operations = await codeAction.GetOperationsAsync(CancellationToken.None).ConfigureAwait(false);
         var solution = operations.OfType<ApplyChangesOperation>().Single().ChangedSolution;
+
+        if (mustCompile)
+        {
+            var options = new CSharpCompilationOptions(OutputKind, allowUnsafe: true);
+            var project = solution.Projects.Single();
+            var compilation = (await project.GetCompilationAsync().ConfigureAwait(false)).WithOptions(options);
+
+            using var ms = new MemoryStream();
+            var result = compilation.Emit(ms);
+            if (!result.Success)
+            {
+                string sourceCode = null;
+                document = project.Documents.FirstOrDefault();
+                if (document != null)
+                {
+                    sourceCode = (await document.GetSyntaxRootAsync().ConfigureAwait(false)).ToFullString();
+                }
+
+                Assert.True(false, "The fixed code doesn't compile. " + string.Join(Environment.NewLine, result.Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error)) + Environment.NewLine + sourceCode);
+            }
+        }
+
         return solution.GetDocument(document.Id);
     }
 

tests/Meziantou.Analyzer.Test/Helpers/ProjectBuilder.cs

@@ -10,6 +10,7 @@
 using System.Reflection.PortableExecutable;
 using System.Text;
 using System.Threading.Tasks;
+using Meziantou.Analyzer.Test.Helpers;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.CSharp;
@@ -30,6 +31,7 @@ public sealed partial class ProjectBuilder
     public Dictionary<string, string> AnalyzerConfiguration { get; private set; }
     public Dictionary<string, string> AdditionalFiles { get; private set; }
     public bool IsValidCode { get; private set; } = true;
+    public bool IsValidFixCode { get; private set; } = true;
     public LanguageVersion LanguageVersion { get; private set; } = LanguageVersion.Latest;
     public TargetFramework TargetFramework { get; private set; } = TargetFramework.NetStandard2_0;
     public IList<MetadataReference> References { get; } = new List<MetadataReference>();
@@ -58,8 +60,7 @@ async Task<string[]> Download()
             if (!Directory.Exists(tempFolder) || !Directory.EnumerateFileSystemEntries(tempFolder).Any())
             {
                 Directory.CreateDirectory(tempFolder);
-                using var httpClient = new HttpClient();
-                using var stream = await httpClient.GetStreamAsync(new Uri($"https://www.nuget.org/api/v2/package/{packageName}/{version}")).ConfigureAwait(false);
+                using var stream = await SharedHttpClient.Instance.GetStreamAsync(new Uri($"https://www.nuget.org/api/v2/package/{packageName}/{version}")).ConfigureAwait(false);
                 using var zip = new ZipArchive(stream, ZipArchiveMode.Read);
 
                 foreach (var entry in zip.Entries.Where(file => paths.Any(path => file.FullName.StartsWith(path, StringComparison.Ordinal))))
@@ -258,6 +259,13 @@ public ProjectBuilder WithCompilation()
     public ProjectBuilder WithNoCompilation()
     {
         IsValidCode = false;
+        IsValidFixCode = false;
+        return this;
+    }
+
+    public ProjectBuilder WithNoFixCompilation()
+    {
+        IsValidFixCode = false;
         return this;
     }