Skip to content

Commit e28d386

Browse files
committed
Enhance RenameHandler to handle null capabilities gracefully and add tests for registration options
1 parent d2112c2 commit e28d386

2 files changed

Lines changed: 62 additions & 13 deletions

File tree

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ internal class PrepareRenameHandler
2020
RenameService renameService
2121
) : IPrepareRenameHandler
2222
{
23-
public RenameRegistrationOptions GetRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => capability.PrepareSupport ? new() { PrepareProvider = true } : new();
23+
public RenameRegistrationOptions GetRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => capability?.PrepareSupport == true ? new() { PrepareProvider = true } : new();
2424

2525
public async Task<RangeOrPlaceholderRange?> Handle(PrepareRenameParams request, CancellationToken cancellationToken)
2626
=> await renameService.PrepareRenameSymbol(request, cancellationToken).ConfigureAwait(false);
@@ -34,7 +34,9 @@ RenameService renameService
3434
) : IRenameHandler
3535
{
3636
// RenameOptions may only be specified if the client states that it supports prepareSupport in its initial initialize request.
37-
public RenameRegistrationOptions GetRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => capability.PrepareSupport ? new() { PrepareProvider = true } : new();
37+
// Passes a null capability when the client omits textDocument.rename from its advertised capabilities (e.g. a completion-only client).
38+
// Use the null-conditional operator so we don't throw NullReferenceException during initialize.
39+
public RenameRegistrationOptions GetRegistrationOptions(RenameCapability capability, ClientCapabilities clientCapabilities) => capability?.PrepareSupport == true ? new() { PrepareProvider = true } : new();
3840

3941
public async Task<WorkspaceEdit?> Handle(RenameParams request, CancellationToken cancellationToken)
4042
=> await renameService.RenameSymbol(request, cancellationToken).ConfigureAwait(false);

test/PowerShellEditorServices.Test/Refactoring/RenameHandlerTests.cs

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
88
using Microsoft.PowerShell.EditorServices.Test.Shared;
99
using OmniSharp.Extensions.LanguageServer.Protocol;
10+
using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
1011
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
1112
using static PowerShellEditorServices.Test.Refactoring.RefactorUtilities;
1213
using System.Linq;
@@ -24,25 +25,25 @@ public class RenameHandlerTests
2425
private readonly WorkspaceService workspace = new(NullLoggerFactory.Instance);
2526

2627
private readonly RenameHandler testHandler;
28+
private readonly PrepareRenameHandler testPrepareHandler;
2729
public RenameHandlerTests()
2830
{
2931
workspace.WorkspaceFolders.Add(new WorkspaceFolder
3032
{
3133
Uri = DocumentUri.FromFileSystemPath(TestUtilities.GetSharedPath("Refactoring"))
3234
});
3335

34-
testHandler = new
36+
RenameService renameService = new
3537
(
36-
new RenameService
37-
(
38-
workspace,
39-
new FakeLspSendMessageRequestFacade("I Accept"),
40-
new EmptyConfiguration()
41-
)
42-
{
43-
DisclaimerAcceptedForSession = true //Disables UI prompts
44-
}
45-
);
38+
workspace,
39+
new FakeLspSendMessageRequestFacade("I Accept"),
40+
new EmptyConfiguration()
41+
)
42+
{
43+
DisclaimerAcceptedForSession = true //Disables UI prompts
44+
};
45+
testHandler = new(renameService);
46+
testPrepareHandler = new(renameService);
4647
}
4748

4849
// Decided to keep this DAMP instead of DRY due to memberdata boundaries, duplicates with PrepareRenameHandler
@@ -125,4 +126,50 @@ public async Task RenamedVariable(RenameTestTarget s)
125126

126127
Assert.Equal(expected, actual);
127128
}
129+
130+
[Fact]
131+
public void GetRegistrationOptionsDoesNotThrowWhenCapabilityIsNull()
132+
{
133+
// Acts: framework hands us null when client omits the capability.
134+
RenameRegistrationOptions opts = testHandler.GetRegistrationOptions(
135+
capability: null,
136+
clientCapabilities: new ClientCapabilities());
137+
138+
Assert.NotNull(opts);
139+
// Without PrepareSupport advertised, PrepareProvider should be false.
140+
Assert.False(opts.PrepareProvider);
141+
}
142+
143+
[Fact]
144+
public void GetRegistrationOptionsHonorsPrepareSupportWhenCapabilityProvided()
145+
{
146+
RenameRegistrationOptions opts = testHandler.GetRegistrationOptions(
147+
capability: new RenameCapability { PrepareSupport = true },
148+
clientCapabilities: new ClientCapabilities());
149+
150+
Assert.NotNull(opts);
151+
Assert.True(opts.PrepareProvider);
152+
}
153+
154+
[Fact]
155+
public void PrepareGetRegistrationOptionsDoesNotThrowWhenCapabilityIsNull()
156+
{
157+
RenameRegistrationOptions opts = testPrepareHandler.GetRegistrationOptions(
158+
capability: null,
159+
clientCapabilities: new ClientCapabilities());
160+
161+
Assert.NotNull(opts);
162+
Assert.False(opts.PrepareProvider);
163+
}
164+
165+
[Fact]
166+
public void PrepareGetRegistrationOptionsHonorsPrepareSupportWhenCapabilityProvided()
167+
{
168+
RenameRegistrationOptions opts = testPrepareHandler.GetRegistrationOptions(
169+
capability: new RenameCapability { PrepareSupport = true },
170+
clientCapabilities: new ClientCapabilities());
171+
172+
Assert.NotNull(opts);
173+
Assert.True(opts.PrepareProvider);
174+
}
128175
}

0 commit comments

Comments
 (0)