Skip to content

Commit 617e1fa

Browse files
davidwengierdavidwengier
authored
Correctly set the Html document uri for diagnostics requests (#12061)
Found while validating a life saving comment from Todd left on #12060
2 parents 4e5b86d + a2e1d6d commit 617e1fa

File tree

5 files changed

+164
-29
lines changed

5 files changed

+164
-29
lines changed

‎src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/HtmlRequestInvoker.cs

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System;
55
using System.ComponentModel.Composition;
6-
using System.Diagnostics;
76
using System.Threading;
87
using System.Threading.Tasks;
98
using Microsoft.AspNetCore.Razor;
@@ -60,13 +59,7 @@ internal sealed class HtmlRequestInvoker(
6059

6160
// If the request is for a text document, we need to update the Uri to point to the Html document,
6261
// and most importantly set it back again before leaving the method in case a caller uses it.
63-
DocumentUri? originalUri = null;
64-
var textDocumentRequest = request as ITextDocumentParams;
65-
if (textDocumentRequest is not null)
66-
{
67-
originalUri = textDocumentRequest.TextDocument.DocumentUri;
68-
textDocumentRequest.TextDocument.DocumentUri = new(htmlDocument.Uri);
69-
}
62+
UpdateTextDocumentUri(request, new(htmlDocument.Uri), out var originalUri);
7063

7164
try
7265
{
@@ -91,15 +84,27 @@ internal sealed class HtmlRequestInvoker(
9184
}
9285
finally
9386
{
94-
Debug.Assert(
95-
(textDocumentRequest is null && originalUri is null) ||
96-
(textDocumentRequest is not null && originalUri is not null), "textDocumentRequest and originalUri should either both be null or both be non-null");
97-
98-
// Reset the Uri if we changed it.
99-
if (textDocumentRequest is not null && originalUri is not null)
87+
if (originalUri is not null)
10088
{
101-
textDocumentRequest.TextDocument.DocumentUri = originalUri;
89+
UpdateTextDocumentUri(request, originalUri, out _);
10290
}
10391
}
10492
}
93+
94+
private void UpdateTextDocumentUri<TRequest>(TRequest request, DocumentUri uri, out DocumentUri? originalUri) where TRequest : notnull
95+
{
96+
var textDocument = request switch
97+
{
98+
ITextDocumentParams textDocumentParams => textDocumentParams.TextDocument,
99+
// VSInternalDiagnosticParams doesn't implement the interface because the TextDocument property is nullable
100+
VSInternalDiagnosticParams vsInternalDiagnosticParams => vsInternalDiagnosticParams.TextDocument,
101+
// We don't implement the endpoint that uses this, but it's the only other thing, at time of writing, in the LSP
102+
// protocol library that isn't handled by the above two cases.
103+
VSInternalRelatedDocumentParams vsInternalRelatedDocumentParams => vsInternalRelatedDocumentParams.TextDocument,
104+
_ => null
105+
};
106+
107+
originalUri = textDocument?.DocumentUri;
108+
textDocument?.DocumentUri = uri;
109+
}
105110
}

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/HtmlDocumentSynchronizerTest.cs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5-
using System.Collections.Generic;
65
using System.Runtime.CompilerServices;
76
using System.Threading;
87
using System.Threading.Tasks;
@@ -350,17 +349,4 @@ private class RemoteServiceInvoker(TextDocument document, Func<Task>? generateTa
350349
return (TResult)(object)(await solution.GetAdditionalDocument(_documentId).AssumeNotNull().GetTextAsync(cancellationToken)).ToString();
351350
}
352351
}
353-
354-
private class TestHtmlDocumentPublisher : IHtmlDocumentPublisher
355-
{
356-
private readonly List<(TextDocument, string, ChecksumWrapper)> _publishes = [];
357-
358-
public List<(TextDocument Document, string Text, ChecksumWrapper Checksum)> Publishes => _publishes;
359-
360-
public Task PublishAsync(TextDocument document, SynchronizationResult synchronizationResult, string htmlText, CancellationToken cancellationToken)
361-
{
362-
_publishes.Add((document, htmlText, synchronizationResult.Checksum));
363-
return Task.CompletedTask;
364-
}
365-
}
366352
}

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/HtmlRequestInvokerTest.cs

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Runtime.CompilerServices;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Microsoft.AspNetCore.Razor;
9+
using Microsoft.AspNetCore.Razor.Test.Common.Editor;
10+
using Microsoft.AspNetCore.Razor.Test.Common.VisualStudio;
11+
using Microsoft.CodeAnalysis;
12+
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
13+
using Microsoft.CodeAnalysis.Razor;
14+
using Microsoft.CodeAnalysis.Razor.Remote;
15+
using Microsoft.CodeAnalysis.Razor.Telemetry;
16+
using Microsoft.CodeAnalysis.Text;
17+
using Xunit;
18+
using Xunit.Abstractions;
19+
20+
namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;
21+
22+
public class HtmlRequestInvokerTest(ITestOutputHelper testOutput) : VisualStudioWorkspaceTestBase(testOutput)
23+
{
24+
private DocumentId? _documentId;
25+
26+
protected override void ConfigureWorkspace(AdhocWorkspace workspace)
27+
{
28+
var project = workspace.CurrentSolution.AddProject("Project", "Project.dll", LanguageNames.CSharp);
29+
var document = project.AddAdditionalDocument("File.razor", SourceText.From("<div></div>"), filePath: "file://File.razor");
30+
_documentId = document.Id;
31+
32+
Assert.True(workspace.TryApplyChanges(document.Project.Solution));
33+
}
34+
35+
[Fact]
36+
public async Task DiagnosticsRequest_UpdatesUri()
37+
{
38+
var document = Workspace.CurrentSolution.GetAdditionalDocument(_documentId).AssumeNotNull();
39+
40+
var htmlDocumentUri = new Uri("file://File.razor.html", UriKind.Absolute);
41+
var requestValidator = (object request) =>
42+
{
43+
var diagnosticParams = Assert.IsType<VSInternalDiagnosticParams>(request);
44+
Assert.Equal(htmlDocumentUri, diagnosticParams.TextDocument!.DocumentUri.GetRequiredParsedUri());
45+
};
46+
47+
var diagnosticRequest = new VSInternalDiagnosticParams
48+
{
49+
TextDocument = new TextDocumentIdentifier { DocumentUri = document.CreateDocumentUri() }
50+
};
51+
52+
await MakeHtmlRequestAsync(document, htmlDocumentUri, requestValidator, VSInternalMethods.DocumentPullDiagnosticName, diagnosticRequest);
53+
54+
Assert.Equal(document.CreateDocumentUri(), diagnosticRequest.TextDocument!.DocumentUri);
55+
}
56+
57+
[Fact]
58+
public async Task ITextDocumentParamsRequest_UpdatesUri()
59+
{
60+
var document = Workspace.CurrentSolution.GetAdditionalDocument(_documentId).AssumeNotNull();
61+
62+
var htmlDocumentUri = new Uri("file://File.razor.html", UriKind.Absolute);
63+
var requestValidator = (object request) =>
64+
{
65+
var hoverParams = Assert.IsAssignableFrom<ITextDocumentParams>(request);
66+
Assert.Equal(htmlDocumentUri, hoverParams.TextDocument!.DocumentUri.GetRequiredParsedUri());
67+
};
68+
69+
var hoverRequest = new HoverParams
70+
{
71+
TextDocument = new TextDocumentIdentifier { DocumentUri = document.CreateDocumentUri() }
72+
};
73+
74+
await MakeHtmlRequestAsync(document, htmlDocumentUri, requestValidator, Methods.TextDocumentHoverName, hoverRequest);
75+
76+
Assert.Equal(document.CreateDocumentUri(), hoverRequest.TextDocument!.DocumentUri);
77+
}
78+
79+
private async Task MakeHtmlRequestAsync<TRequest>(TextDocument document, Uri htmlDocumentUri, Action<object> requestValidator, string method, TRequest request)
80+
where TRequest : notnull
81+
{
82+
var htmlTextSnapshot = new StringTextSnapshot("");
83+
var htmlTextBuffer = new TestTextBuffer(htmlTextSnapshot);
84+
var checksum = await document.GetChecksumAsync(DisposalToken);
85+
var requestInvoker = new TestLSPRequestInvoker((method, null));
86+
var lspDocumentManager = new TestDocumentManager();
87+
var htmlVirtualDocument = new HtmlVirtualDocumentSnapshot(htmlDocumentUri, htmlTextBuffer.CurrentSnapshot, hostDocumentSyncVersion: 1, state: checksum);
88+
var documentSnapshot = new TestLSPDocumentSnapshot(document.CreateUri(), version: (int)(htmlVirtualDocument.HostDocumentSyncVersion!.Value + 1), htmlVirtualDocument);
89+
lspDocumentManager.AddDocument(documentSnapshot.Uri, documentSnapshot);
90+
91+
var publisher = new TestHtmlDocumentPublisher();
92+
var remoteServiceInvoker = new RemoteServiceInvoker();
93+
var htmlDocumentSynchronizer = new HtmlDocumentSynchronizer(remoteServiceInvoker, publisher, LoggerFactory);
94+
var invoker = new HtmlRequestInvoker(requestInvoker, lspDocumentManager, htmlDocumentSynchronizer, NoOpTelemetryReporter.Instance, LoggerFactory);
95+
96+
var validated = false;
97+
requestInvoker.RequestAction = r =>
98+
{
99+
validated = true;
100+
requestValidator(r);
101+
};
102+
103+
_ = await invoker.MakeHtmlLspRequestAsync<TRequest, object>(
104+
document,
105+
method,
106+
request,
107+
DisposalToken);
108+
109+
Assert.True(validated);
110+
}
111+
112+
private class RemoteServiceInvoker : IRemoteServiceInvoker
113+
{
114+
public ValueTask<TResult?> TryInvokeAsync<TService, TResult>(Solution solution, Func<TService, RazorPinnedSolutionInfoWrapper, CancellationToken, ValueTask<TResult>> invocation, CancellationToken cancellationToken, [CallerFilePath] string? callerFilePath = null, [CallerMemberName] string? callerMemberName = null) where TService : class
115+
{
116+
return new((TResult?)(object)"");
117+
}
118+
}
119+
}

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/TestHtmlDocumentPublisher.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Collections.Generic;
5+
using System.Threading;
6+
using System.Threading.Tasks;
7+
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
9+
10+
namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;
11+
12+
internal sealed class TestHtmlDocumentPublisher : IHtmlDocumentPublisher
13+
{
14+
public List<(TextDocument Document, string Text, ChecksumWrapper Checksum)> Publishes { get; } = [];
15+
16+
public Task PublishAsync(TextDocument document, SynchronizationResult synchronizationResult, string htmlText, CancellationToken cancellationToken)
17+
{
18+
Publishes.Add((document, htmlText, synchronizationResult.Checksum));
19+
return Task.CompletedTask;
20+
}
21+
}

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/LanguageClient/TestLSPRequestInvoker.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ internal sealed class TestLSPRequestInvoker(params IEnumerable<(string method, o
1818
{
1919
private readonly Dictionary<string, object?> _responses = responses.ToDictionary(kvp => kvp.method, kvp => kvp.response);
2020

21+
public Action<object>? RequestAction { get; set; }
22+
2123
[Obsolete]
2224
public override Task<IEnumerable<ReinvokeResponse<TOut>>> ReinvokeRequestOnMultipleServersAsync<TIn, TOut>(
2325
string method,
@@ -86,6 +88,8 @@ public override Task<ReinvokeResponse<TOut>> ReinvokeRequestOnServerAsync<TIn, T
8688
TIn parameters,
8789
CancellationToken cancellationToken)
8890
{
91+
RequestAction?.Invoke(parameters);
92+
8993
Assert.True(_responses.TryGetValue(method, out var response), $"'{method}' was not defined with a response.");
9094

9195
return Task.FromResult(new ReinvocationResponse<TOut>(languageClientName: "html", (TOut?)response)).AsNullable();

0 commit comments

Comments
 (0)