Enhance RenameHandler to handle null capabilities gracefully and add tests for registration options (#2296)

* Enhance RenameHandler to handle null capabilities gracefully and add tests for registration options

* Refactor RenameHandler to support nullable capabilities and enhance tests for registration options

* Address review: keep interface-compatible signature; cover explicit false capability

- Revert GetRegistrationOptions parameter to the non-nullable RenameCapability
  declared by IPrepareRenameHandler/IRenameHandler, and rely on the
  null-conditional check (capability?.PrepareSupport == true) for the runtime
  case where the framework passes a null capability. This matches the OmniSharp
  interface signature exactly, avoiding any nullable-annotation mismatch
  (e.g. CS8765) while remaining null-safe during initialize.
- Add explicit { false, false } theory cases for both handler kinds so the
  three distinct PrepareSupport inputs (omitted/null, true, false) are all
  covered and a regression that treats false as enabled would be caught.

Verified: src and test projects build with 0 warnings under #nullable enable;
all 6 GetRegistrationOptionsReflectsPrepareSupport cases pass on net462 and net8.0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: mgreenegit

src/PowerShellEditorServices/Services/TextDocument/Handlers/RenameHandler.cs

@@ -20,7 +20,7 @@ internal class PrepareRenameHandler
     RenameService renameService
 ) : IPrepareRenameHandler
 {
-    public RenameRegistrationOptions GetRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => capability.PrepareSupport ? new() { PrepareProvider = true } : new();
+    public RenameRegistrationOptions GetRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => capability?.PrepareSupport == true ? new() { PrepareProvider = true } : new();
 
     public async Task<RangeOrPlaceholderRange?> Handle(PrepareRenameParams request, CancellationToken cancellationToken)
         => await renameService.PrepareRenameSymbol(request, cancellationToken).ConfigureAwait(false);
@@ -34,7 +34,9 @@ RenameService renameService
 ) : IRenameHandler
 {
     // RenameOptions may only be specified if the client states that it supports prepareSupport in its initial initialize request.
-    public RenameRegistrationOptions GetRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => capability.PrepareSupport ? new() { PrepareProvider = true } : new();
+    // The framework passes a null capability when the client omits textDocument.rename from its advertised capabilities (e.g. a completion-only client).
+    // The parameter keeps the interface's non-nullable signature; the null-conditional operator avoids a NullReferenceException during initialize.
+    public RenameRegistrationOptions GetRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => capability?.PrepareSupport == true ? new() { PrepareProvider = true } : new();
 
     public async Task<WorkspaceEdit?> Handle(RenameParams request, CancellationToken cancellationToken)
         => await renameService.RenameSymbol(request, cancellationToken).ConfigureAwait(false);

test/PowerShellEditorServices.Test/Refactoring/RenameHandlerTests.cs

@@ -1,12 +1,14 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+using System;
 using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.PowerShell.EditorServices.Handlers;
 using Microsoft.PowerShell.EditorServices.Services;
 using Microsoft.PowerShell.EditorServices.Services.TextDocument;
 using Microsoft.PowerShell.EditorServices.Test.Shared;
 using OmniSharp.Extensions.LanguageServer.Protocol;
+using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
 using OmniSharp.Extensions.LanguageServer.Protocol.Models;
 using static PowerShellEditorServices.Test.Refactoring.RefactorUtilities;
 using System.Linq;
@@ -24,25 +26,25 @@ public class RenameHandlerTests
     private readonly WorkspaceService workspace = new(NullLoggerFactory.Instance);
 
     private readonly RenameHandler testHandler;
+    private readonly PrepareRenameHandler testPrepareHandler;
     public RenameHandlerTests()
     {
         workspace.WorkspaceFolders.Add(new WorkspaceFolder
         {
             Uri = DocumentUri.FromFileSystemPath(TestUtilities.GetSharedPath("Refactoring"))
         });
 
-        testHandler = new
+        RenameService renameService = new
         (
-            new RenameService
-            (
-                workspace,
-                new FakeLspSendMessageRequestFacade("I Accept"),
-                new EmptyConfiguration()
-            )
-            {
-                DisclaimerAcceptedForSession = true //Disables UI prompts
-            }
-        );
+            workspace,
+            new FakeLspSendMessageRequestFacade("I Accept"),
+            new EmptyConfiguration()
+        )
+        {
+            DisclaimerAcceptedForSession = true //Disables UI prompts
+        };
+        testHandler = new(renameService);
+        testPrepareHandler = new(renameService);
     }
 
     // Decided to keep this DAMP instead of DRY due to memberdata boundaries, duplicates with PrepareRenameHandler
@@ -125,4 +127,43 @@ public async Task RenamedVariable(RenameTestTarget s)
 
         Assert.Equal(expected, actual);
     }
+
+    public enum RegistrationHandlerKind
+    {
+        Rename,
+        PrepareRename
+    }
+
+    // prepareSupport has three distinct inputs: null = client omitted the capability entirely (framework hands us null),
+    // true = client supports prepareRename, false = client explicitly does not. Only true should enable PrepareProvider.
+    public static TheoryData<RegistrationHandlerKind, bool?, bool> RegistrationOptionsTestCases() => new()
+    {
+        { RegistrationHandlerKind.Rename, null, false },
+        { RegistrationHandlerKind.Rename, false, false },
+        { RegistrationHandlerKind.Rename, true, true },
+        { RegistrationHandlerKind.PrepareRename, null, false },
+        { RegistrationHandlerKind.PrepareRename, false, false },
+        { RegistrationHandlerKind.PrepareRename, true, true }
+    };
+
+    [Theory]
+    [MemberData(nameof(RegistrationOptionsTestCases))]
+    public void GetRegistrationOptionsReflectsPrepareSupport(RegistrationHandlerKind handlerKind, bool? prepareSupport, bool expectedPrepareProvider)
+    {
+        RenameCapability capability = prepareSupport is bool ps
+            ? new RenameCapability { PrepareSupport = ps }
+            : null;
+
+        Func<RenameCapability, ClientCapabilities, RenameRegistrationOptions> getRegistrationOptions = handlerKind switch
+        {
+            RegistrationHandlerKind.Rename => testHandler.GetRegistrationOptions,
+            RegistrationHandlerKind.PrepareRename => testPrepareHandler.GetRegistrationOptions,
+            _ => throw new ArgumentOutOfRangeException(nameof(handlerKind))
+        };
+
+        RenameRegistrationOptions opts = getRegistrationOptions(capability, new ClientCapabilities());
+
+        Assert.NotNull(opts);
+        Assert.Equal(expectedPrepareProvider, opts.PrepareProvider);
+    }
 }