Validate fix conditions in RegisterCodeFixesAsync before registering code fixes

.github/copilot-instructions.md

@@ -48,6 +48,62 @@ The analyzer must use `IOperation` or `ISymbol` to analyze the content. Only fal
 Code snippets in tests must use raw string literals (`"""`) and must be minimized to only include the necessary code to reproduce the issue. Avoid including unnecessary code that does not contribute to the test case.
 When reporting a diagnostic, the snippet must use the `[|code|]` syntax or `{|id:code|}` syntax. Do not explicitly indicates lines or columns.
 
+### Code fixer best practice: validate before registering
+
+In `RegisterCodeFixesAsync`, validate **all** conditions that could prevent the fix from being applied **before** calling `context.RegisterCodeFix`. Do not register a code fix whose action would return the document unchanged.
+
+**Wrong** — registers the fix without validating whether it can be applied:
+```csharp
+public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+{
+    var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+    var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true);
+    if (nodeToFix is null)
+        return;
+
+    context.RegisterCodeFix(CodeAction.Create(title, ct => FixAsync(context.Document, nodeToFix, ct), equivalenceKey: title), context.Diagnostics);
+}
+
+private static async Task<Document> FixAsync(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken)
+{
+    if (nodeToFix is not BinaryExpressionSyntax binaryExpression)
+        return document; // Fix not applied — but it was already shown to the user!
+
+    var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+    var mySymbol = semanticModel!.Compilation.GetBestTypeByMetadataName("System.SomeType");
+    if (mySymbol is null)
+        return document; // Fix not applied — but it was already shown to the user!
+    // ...
+}
+```
+
+**Correct** — validates all conditions first, then registers the fix:
+```csharp
+public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+{
+    var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+    var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true);
+    if (nodeToFix is not BinaryExpressionSyntax binaryExpression)
+        return;
+
+    var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
+    if (semanticModel is null)
+        return;
+
+    if (semanticModel.Compilation.GetBestTypeByMetadataName("System.SomeType") is null)
+        return;
+
+    context.RegisterCodeFix(CodeAction.Create(title, ct => FixAsync(context.Document, binaryExpression, ct), equivalenceKey: title), context.Diagnostics);
+}
+
+private static async Task<Document> FixAsync(Document document, BinaryExpressionSyntax binaryExpression, CancellationToken cancellationToken)
+{
+    var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
+    // ... fix logic — all preconditions are guaranteed to hold
+    return editor.GetChangedDocument();
+}
+```
+
 ## Testing with different Roslyn versions
 
 This project supports multiple versions of Roslyn to ensure compatibility with different versions of Visual Studio and the .NET SDK. The supported Roslyn versions are configured in `Directory.Build.targets`:

src/Meziantou.Analyzer.CodeFixers/Rules/AbstractTypesShouldNotHaveConstructorsFixer.cs

@@ -20,27 +20,22 @@ public sealed class AbstractTypesShouldNotHaveConstructorsFixer : CodeFixProvide
     public override async Task RegisterCodeFixesAsync(CodeFixContext context)
     {
         var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
-        var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true);
-        if (nodeToFix is null)
+        if (root?.FindNode(context.Span, getInnermostNodeForTie: true) is not ConstructorDeclarationSyntax ctorSyntax)
             return;
 
         var title = "Make constructor protected";
         var codeAction = CodeAction.Create(
             title,
-            ct => MakeConstructorProtected(context.Document, nodeToFix, ct),
+            ct => MakeConstructorProtected(context.Document, ctorSyntax, ct),
             equivalenceKey: title);
 
         context.RegisterCodeFix(codeAction, context.Diagnostics);
     }
 
-    private static async Task<Document> MakeConstructorProtected(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken)
+    private static async Task<Document> MakeConstructorProtected(Document document, ConstructorDeclarationSyntax ctorSyntax, CancellationToken cancellationToken)
     {
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
 
-        var ctorSyntax = (ConstructorDeclarationSyntax)nodeToFix;
-        if (ctorSyntax is null)
-            return document;
-
         var modifiers = ctorSyntax.Modifiers;
         foreach (var modifier in modifiers.Where(m => m.IsKind(SyntaxKind.PublicKeyword) || m.IsKind(SyntaxKind.InternalKeyword)))
         {

src/Meziantou.Analyzer.CodeFixers/Rules/AvoidComparisonWithBoolConstantFixer.cs

@@ -24,7 +24,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
     {
         var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
         var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true);
-        if (nodeToFix is null)
+        if (nodeToFix is not BinaryExpressionSyntax)
             return;
 
         var diagnostic = context.Diagnostics[0];
@@ -40,12 +40,6 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
     private static async Task<Document> RemoveComparisonWithBoolConstant(Document document, Diagnostic diagnostic, SyntaxNode nodeToFix, CancellationToken cancellationToken)
     {
-        if (nodeToFix is not BinaryExpressionSyntax binaryExpressionSyntax)
-            return document;
-
-        if (binaryExpressionSyntax.Left is null || binaryExpressionSyntax.Right is null)
-            return document;
-
         var nodeToKeepSpanStart = int.Parse(diagnostic.Properties["NodeToKeepSpanStart"]!, NumberStyles.Integer, CultureInfo.InvariantCulture);
         var nodeToKeepSpanLength = int.Parse(diagnostic.Properties["NodeToKeepSpanLength"]!, NumberStyles.Integer, CultureInfo.InvariantCulture);
         var logicalNotOperatorNeeded = bool.Parse(diagnostic.Properties["LogicalNotOperatorNeeded"]!);

src/Meziantou.Analyzer.CodeFixers/Rules/AvoidUnusedInternalTypesFixer.cs

@@ -33,13 +33,19 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
         // Code fix 1: Add DynamicallyAccessedMembers attribute
         {
-            var title = "Add DynamicallyAccessedMembers attribute";
-            var codeAction = CodeAction.Create(
-                title,
-                ct => AddDynamicallyAccessedMembersAttribute(context.Document, typeDeclarationSyntax, ct),
-                equivalenceKey: title);
+            var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
+            var dynamicallyAccessedMembersAttribute = semanticModel?.Compilation.GetBestTypeByMetadataName("System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute");
+            var dynamicallyAccessedMemberTypes = semanticModel?.Compilation.GetBestTypeByMetadataName("System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes");
+            if (dynamicallyAccessedMembersAttribute is not null && dynamicallyAccessedMemberTypes is not null)
+            {
+                var title = "Add DynamicallyAccessedMembers attribute";
+                var codeAction = CodeAction.Create(
+                    title,
+                    ct => AddDynamicallyAccessedMembersAttribute(context.Document, typeDeclarationSyntax, dynamicallyAccessedMembersAttribute, dynamicallyAccessedMemberTypes, ct),
+                    equivalenceKey: title);
 
-            context.RegisterCodeFix(codeAction, context.Diagnostics);
+                context.RegisterCodeFix(codeAction, context.Diagnostics);
+            }
         }
 
         // Code fix 2: Remove the type
@@ -54,18 +60,11 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
         }
     }
 
-    private static async Task<Document> AddDynamicallyAccessedMembersAttribute(Document document, TypeDeclarationSyntax typeDeclarationSyntax, CancellationToken cancellationToken)
+    private static async Task<Document> AddDynamicallyAccessedMembersAttribute(Document document, TypeDeclarationSyntax typeDeclarationSyntax, INamedTypeSymbol dynamicallyAccessedMembersAttribute, INamedTypeSymbol dynamicallyAccessedMemberTypes, CancellationToken cancellationToken)
     {
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
-        var semanticModel = editor.SemanticModel;
         var generator = editor.Generator;
 
-        var dynamicallyAccessedMembersAttribute = semanticModel.Compilation.GetBestTypeByMetadataName("System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute");
-        var dynamicallyAccessedMemberTypes = semanticModel.Compilation.GetBestTypeByMetadataName("System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes");
-
-        if (dynamicallyAccessedMembersAttribute is null || dynamicallyAccessedMemberTypes is null)
-            return document;
-
         var attribute = generator.Attribute(
             generator.TypeExpression(dynamicallyAccessedMembersAttribute, addImport: true),
             [

src/Meziantou.Analyzer.CodeFixers/Rules/DoNotRemoveOriginalExceptionFromThrowStatementFixer.cs

@@ -20,27 +20,23 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
     {
         var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
         var nodeToFix = root?.FindNode(context.Span, getInnermostNodeForTie: true);
-        if (nodeToFix is null)
+        if (nodeToFix is not ThrowStatementSyntax throwStatement)
             return;
 
         var title = "Throw original exception";
         var codeAction = CodeAction.Create(
             title,
-            ct => Fix(context.Document, nodeToFix, ct),
+            ct => Fix(context.Document, throwStatement, ct),
             equivalenceKey: title);
 
         context.RegisterCodeFix(codeAction, context.Diagnostics);
     }
 
-    private static async Task<Document> Fix(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken)
+    private static async Task<Document> Fix(Document document, ThrowStatementSyntax throwStatement, CancellationToken cancellationToken)
     {
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
 
-        var syntax = (ThrowStatementSyntax)nodeToFix;
-        if (syntax is null)
-            return document;
-
-        editor.ReplaceNode(syntax, syntax.WithExpression(null).WithAdditionalAnnotations(Formatter.Annotation));
+        editor.ReplaceNode(throwStatement, throwStatement.WithExpression(null).WithAdditionalAnnotations(Formatter.Annotation));
         return editor.GetChangedDocument();
     }
 }

src/Meziantou.Analyzer.CodeFixers/Rules/DoNotUseBlockingCallInAsyncContextFixer.cs

@@ -32,9 +32,14 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
         {
             case DoNotUseBlockingCallInAsyncContextData.Thread_Sleep:
                 {
+                    var sm = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
+                    var taskSymbol = sm?.Compilation.GetBestTypeByMetadataName("System.Threading.Tasks.Task");
+                    if (taskSymbol is null)
+                        break;
+
                     var codeAction = CodeAction.Create(
                         "Use Task.Delay",
-                        ct => UseTaskDelay(context.Document, nodeToFix, ct),
+                        ct => UseTaskDelay(context.Document, nodeToFix, taskSymbol, ct),
                         equivalenceKey: "Thread_Sleep");
 
                     context.RegisterCodeFix(codeAction, context.Diagnostics);
@@ -43,6 +48,10 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
             case DoNotUseBlockingCallInAsyncContextData.Task_Wait:
                 {
+                    if (nodeToFix is not InvocationExpressionSyntax taskWaitInvocation ||
+                        (taskWaitInvocation.Expression as MemberAccessExpressionSyntax)?.Expression is null)
+                        break;
+
                     var codeAction = CodeAction.Create(
                         "Use await",
                         ct => ReplaceTaskWaitWithAwait(context.Document, nodeToFix, ct),
@@ -140,9 +149,6 @@ private static async Task<Document> ReplaceTaskResultWithAwait(Document document
         var generator = editor.Generator;
 
         var expr = ((MemberAccessExpressionSyntax)nodeToFix).Expression;
-        if (expr is null)
-            return document;
-
         var newExpression = generator.AwaitExpression(expr).Parentheses();
         editor.ReplaceNode(nodeToFix, newExpression);
 
@@ -155,25 +161,18 @@ private static async Task<Document> ReplaceTaskWaitWithAwait(Document document,
         var generator = editor.Generator;
 
         var invocation = (InvocationExpressionSyntax)nodeToFix;
-        var expr = (invocation.Expression as MemberAccessExpressionSyntax)?.Expression;
-        if (expr is null)
-            return document;
-
+        var expr = (invocation.Expression as MemberAccessExpressionSyntax)!.Expression;
         var newExpression = generator.AwaitExpression(expr).Parentheses();
         editor.ReplaceNode(nodeToFix, newExpression);
 
         return editor.GetChangedDocument();
     }
 
-    private static async Task<Document> UseTaskDelay(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken)
+    private static async Task<Document> UseTaskDelay(Document document, SyntaxNode nodeToFix, INamedTypeSymbol taskSymbol, CancellationToken cancellationToken)
     {
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
         var generator = editor.Generator;
 
-        var taskSymbol = editor.SemanticModel.Compilation.GetBestTypeByMetadataName("System.Threading.Tasks.Task");
-        if (taskSymbol is null)
-            return document;
-
         var invocation = (InvocationExpressionSyntax)nodeToFix;
         var delay = invocation.ArgumentList.Arguments[0].Expression;
 

src/Meziantou.Analyzer.CodeFixers/Rules/DoNotUseEqualityComparerDefaultOfStringFixer.cs

@@ -23,6 +23,14 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
         if (nodeToFix is null)
             return;
 
+        var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
+        if (semanticModel is null)
+            return;
+
+        var stringComparerSymbol = semanticModel.Compilation.GetBestTypeByMetadataName("System.StringComparer");
+        if (stringComparerSymbol is null)
+            return;
+
         RegisterCodeFix(nameof(StringComparer.Ordinal));
         RegisterCodeFix(nameof(StringComparer.OrdinalIgnoreCase));
 
@@ -31,25 +39,20 @@ void RegisterCodeFix(string comparerName)
             var title = "Use StringComparer." + comparerName;
             var codeAction = CodeAction.Create(
                 title,
-                ct => MakeConstructorProtected(context.Document, nodeToFix, comparerName, ct),
+                ct => MakeConstructorProtected(context.Document, nodeToFix, comparerName, stringComparerSymbol, ct),
                 equivalenceKey: title);
 
             context.RegisterCodeFix(codeAction, context.Diagnostics);
         }
     }
 
-    private static async Task<Document> MakeConstructorProtected(Document document, SyntaxNode nodeToFix, string comparerName, CancellationToken cancellationToken)
+    private static async Task<Document> MakeConstructorProtected(Document document, SyntaxNode nodeToFix, string comparerName, INamedTypeSymbol stringComparer, CancellationToken cancellationToken)
     {
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
-        var semanticModel = editor.SemanticModel;
         var generator = editor.Generator;
 
         var syntax = (MemberAccessExpressionSyntax)nodeToFix;
 
-        var stringComparer = semanticModel.Compilation.GetBestTypeByMetadataName("System.StringComparer");
-        if (stringComparer is null)
-            return document;
-
         var newSyntax = generator.MemberAccessExpression(
             generator.TypeExpression(stringComparer),
             comparerName);

src/Meziantou.Analyzer.CodeFixers/Rules/DoNotUseEqualityOperatorsForSpanOfCharFixer.cs

@@ -26,25 +26,27 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
         if (nodeToFix is null)
             return;
 
+        var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
+        if (semanticModel is null)
+            return;
+
+        if (semanticModel.GetOperation(nodeToFix, context.CancellationToken) is not IBinaryOperation operation)
+            return;
+
         var title = "Use SequenceEquals";
         var codeAction = CodeAction.Create(
             title,
-            ct => Refactor(context.Document, nodeToFix, ct),
+            ct => Refactor(context.Document, operation, ct),
             equivalenceKey: title);
 
         context.RegisterCodeFix(codeAction, context.Diagnostics);
     }
 
-    private static async Task<Document> Refactor(Document document, SyntaxNode nodeToFix, CancellationToken cancellationToken)
+    private static async Task<Document> Refactor(Document document, IBinaryOperation operation, CancellationToken cancellationToken)
     {
         var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
-        var semanticModel = editor.SemanticModel;
         var generator = editor.Generator;
 
-        var operation = (IBinaryOperation?)semanticModel.GetOperation(nodeToFix, cancellationToken);
-        if (operation is null)
-            return document;
-
         var newExpression = generator.InvocationExpression(
             generator.MemberAccessExpression(operation.LeftOperand.Syntax, "SequenceEqual"), operation.RightOperand.Syntax);
 
@@ -53,7 +55,7 @@ private static async Task<Document> Refactor(Document document, SyntaxNode nodeT
             newExpression = generator.LogicalNotExpression(newExpression);
         }
 
-        editor.ReplaceNode(nodeToFix, newExpression.WithAdditionalAnnotations(Formatter.Annotation));
+        editor.ReplaceNode(operation.Syntax, newExpression.WithAdditionalAnnotations(Formatter.Annotation));
         return editor.GetChangedDocument();
     }
 }