Skip to content

Commit

Permalink
Unskip cohosting diagnostics tests (#11541)
Browse files Browse the repository at this point in the history
Step one of The Great Unskippening!™

Part of #10693

It is no longer simply possible to use string manipulation to get a
generated C# document path from a Razor document path. The alternative
options for what is available differ depending on which process code is
running in. Yay! :)
  • Loading branch information
davidwengier authored Feb 25, 2025
2 parents 133c1fa + 6f5c1e4 commit 8054ca5
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ internal abstract class AbstractFilePathService(LanguageServerFeatureOptions lan
public string GetRazorCSharpFilePath(ProjectKey projectKey, string razorFilePath)
=> GetGeneratedFilePath(projectKey, razorFilePath, _languageServerFeatureOptions.CSharpVirtualDocumentSuffix);

public Uri GetRazorDocumentUri(Uri virtualDocumentUri)
public virtual Uri GetRazorDocumentUri(Uri virtualDocumentUri)
{
var uriPath = virtualDocumentUri.AbsoluteUri;
var razorFilePath = GetRazorFilePath(uriPath);
var uri = new Uri(razorFilePath, UriKind.Absolute);
return uri;
}

public bool IsVirtualCSharpFile(Uri uri)
public virtual bool IsVirtualCSharpFile(Uri uri)
=> CheckIfFileUriAndExtensionMatch(uri, _languageServerFeatureOptions.CSharpVirtualDocumentSuffix);

public bool IsVirtualHtmlFile(Uri uri)
Expand All @@ -37,20 +37,23 @@ private static bool CheckIfFileUriAndExtensionMatch(Uri uri, string extension)

private string GetRazorFilePath(string filePath)
{
var trimIndex = filePath.LastIndexOf(_languageServerFeatureOptions.CSharpVirtualDocumentSuffix);
if (trimIndex == -1)
{
trimIndex = filePath.LastIndexOf(_languageServerFeatureOptions.HtmlVirtualDocumentSuffix);
}
else if (_languageServerFeatureOptions.IncludeProjectKeyInGeneratedFilePath)
var trimIndex = filePath.LastIndexOf(_languageServerFeatureOptions.HtmlVirtualDocumentSuffix);

// We don't check for C# in cohosting, as it will throw, and people might call this method on any
// random path.
if (trimIndex == -1 && !_languageServerFeatureOptions.UseRazorCohostServer)
{
trimIndex = filePath.LastIndexOf(_languageServerFeatureOptions.CSharpVirtualDocumentSuffix);

// If this is a C# generated file, and we're including the project suffix, then filename will be
// <Page>.razor.<project slug><c# suffix>
// This means we can remove the project key easily, by just looking for the last '.'. The project
// slug itself cannot a '.', enforced by the assert below in GetProjectSuffix

trimIndex = filePath.LastIndexOf('.', trimIndex - 1);
Debug.Assert(trimIndex != -1, "There was no project element to the generated file name?");
if (_languageServerFeatureOptions.IncludeProjectKeyInGeneratedFilePath)
{
// We can remove the project key easily, by just looking for the last '.'. The project
// slug itself cannot a '.', enforced by the assert below in GetProjectSuffix
trimIndex = filePath.LastIndexOf('.', trimIndex - 1);
Debug.Assert(trimIndex != -1, "There was no project element to the generated file name?");
}
}

if (trimIndex != -1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor;

namespace Microsoft.CodeAnalysis;
Expand Down Expand Up @@ -34,4 +36,19 @@ public static bool TryGetCSharpDocument(this Project project, Uri csharpDocument

return document is not null;
}

/// <summary>
/// Finds source generated documents by iterating through all of them. In OOP there are better options!
/// </summary>
public static async Task<Document?> TryGetSourceGeneratedDocumentFromHintNameAsync(this Project project, string? hintName, CancellationToken cancellationToken)
{
// TODO: use this when the location is case-insensitive on windows (https://github.com/dotnet/roslyn/issues/76869)
//var generator = typeof(RazorSourceGenerator);
//var generatorAssembly = generator.Assembly;
//var generatorName = generatorAssembly.GetName();
//var generatedDocuments = await _project.GetSourceGeneratedDocumentsForGeneratorAsync(generatorName.Name!, generatorAssembly.Location, generatorName.Version!, generator.Name, cancellationToken).ConfigureAwait(false);

var generatedDocuments = await project.GetSourceGeneratedDocumentsAsync(cancellationToken).ConfigureAwait(false);
return generatedDocuments.SingleOrDefault(d => d.HintName == hintName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ internal struct RemoteClientInitializationOptions
[JsonPropertyName("usePreciseSemanticTokenRanges")]
public required bool UsePreciseSemanticTokenRanges { get; set; }

[JsonPropertyName("csharpVirtualDocumentSuffix")]
public required string CSharpVirtualDocumentSuffix { get; set; }

[JsonPropertyName("htmlVirtualDocumentSuffix")]
public required string HtmlVirtualDocumentSuffix { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public void SetOptions(RemoteClientInitializationOptions options)

public override bool SupportsFileManipulation => _options.SupportsFileManipulation;

public override string CSharpVirtualDocumentSuffix => _options.CSharpVirtualDocumentSuffix;
public override string CSharpVirtualDocumentSuffix => throw new InvalidOperationException("This property is not valid in OOP");

public override string HtmlVirtualDocumentSuffix => _options.HtmlVirtualDocumentSuffix;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ public bool TryGetDocument(string filePath, [NotNullWhen(true)] out IDocumentSna
{
var generatorResult = await GetRazorGeneratorResultAsync(cancellationToken).ConfigureAwait(false);
if (generatorResult is null)
{
return null;
}

return generatorResult.GetCodeDocument(documentSnapshot.FilePath);
}
Expand All @@ -172,33 +174,36 @@ public bool TryGetDocument(string filePath, [NotNullWhen(true)] out IDocumentSna
{
var generatorResult = await GetRazorGeneratorResultAsync(cancellationToken).ConfigureAwait(false);
if (generatorResult is null)
{
return null;
}

var hintName = generatorResult.GetHintName(documentSnapshot.FilePath);

// TODO: use this when the location is case-insensitive on windows (https://github.com/dotnet/roslyn/issues/76869)
//var generator = typeof(RazorSourceGenerator);
//var generatorAssembly = generator.Assembly;
//var generatorName = generatorAssembly.GetName();
//var generatedDocuments = await _project.GetSourceGeneratedDocumentsForGeneratorAsync(generatorName.Name!, generatorAssembly.Location, generatorName.Version!, generator.Name, cancellationToken).ConfigureAwait(false);
var generatedDocument = await _project.TryGetSourceGeneratedDocumentFromHintNameAsync(hintName, cancellationToken).ConfigureAwait(false);

var generatedDocuments = await _project.GetSourceGeneratedDocumentsAsync(cancellationToken).ConfigureAwait(false);
return generatedDocuments.Single(d => d.HintName == hintName);
return generatedDocument ?? throw new InvalidOperationException("Couldn't get the source generated document for a hint name that we got from the generator?");
}

private async Task<RazorGeneratorResult?> GetRazorGeneratorResultAsync(CancellationToken cancellationToken)
{
var result = await _project.GetSourceGeneratorRunResultAsync(cancellationToken).ConfigureAwait(false);
if (result is null)
{
return null;
}

var runResult = result.Results.SingleOrDefault(r => r.Generator.GetGeneratorType().Assembly.Location == typeof(RazorSourceGenerator).Assembly.Location);
if (runResult.Generator is null)
{
return null;
}

#pragma warning disable RSEXPERIMENTAL004 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
if (!runResult.HostOutputs.TryGetValue(nameof(RazorGeneratorResult), out var objectResult) || objectResult is not RazorGeneratorResult generatorResult)
{
return null;
}
#pragma warning restore RSEXPERIMENTAL004 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.

return generatorResult;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Composition;
using Microsoft.CodeAnalysis.Razor.Workspaces;

Expand All @@ -10,4 +11,18 @@ namespace Microsoft.CodeAnalysis.Remote.Razor;
[method: ImportingConstructor]
internal sealed class RemoteFilePathService(LanguageServerFeatureOptions options) : AbstractFilePathService(options)
{
public override Uri GetRazorDocumentUri(Uri virtualDocumentUri)
{
if (IsVirtualCSharpFile(virtualDocumentUri))
{
throw new InvalidOperationException("Can not get a Razor document from a generated document Uri in cohosting");
}

return base.GetRazorDocumentUri(virtualDocumentUri);
}

public override bool IsVirtualCSharpFile(Uri uri)
{
return uri.Scheme == "source-generated";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System.Diagnostics.CodeAnalysis;
using System.IO;
using Microsoft.CodeAnalysis;
using Microsoft.NET.Sdk.Razor.SourceGenerators;

namespace Microsoft.VisualStudio.Razor.Extensions;

internal static class TextDocumentExtensions
{
/// <summary>
/// This method tries to compute the source generated hint name for a Razor document using only string manipulation
/// </summary>
/// <remarks>
/// This should only be used in the devenv process. In OOP we can look at the actual generated run result to find this
/// information.
/// </remarks>
public static bool TryComputeHintNameFromRazorDocument(this TextDocument razorDocument, [NotNullWhen(true)] out string? hintName)
{
if (razorDocument.FilePath is null)
{
hintName = null;
return false;
}

var projectBasePath = Path.GetDirectoryName(razorDocument.Project.FilePath);
var relativeDocumentPath = razorDocument.FilePath[projectBasePath.Length..].TrimStart('/', '\\');
hintName = RazorSourceGenerator.GetIdentifierFromPath(relativeDocumentPath);

return hintName is not null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.CodeAnalysis.Razor.Protocol;
using Microsoft.CodeAnalysis.Razor.Remote;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.VisualStudio.LanguageServer.ContainedLanguage;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Microsoft.VisualStudio.Razor.Extensions;
using ExternalHandlers = Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost.Handlers;
using LspDiagnostic = Microsoft.VisualStudio.LanguageServer.Protocol.Diagnostic;
using RoslynDiagnostic = Roslyn.LanguageServer.Protocol.Diagnostic;
Expand All @@ -36,14 +35,12 @@ internal class CohostDocumentPullDiagnosticsEndpoint(
IRemoteServiceInvoker remoteServiceInvoker,
IHtmlDocumentSynchronizer htmlDocumentSynchronizer,
LSPRequestInvoker requestInvoker,
IFilePathService filePathService,
ILoggerFactory loggerFactory)
: AbstractRazorCohostDocumentRequestHandler<VSInternalDocumentDiagnosticsParams, VSInternalDiagnosticReport[]?>, IDynamicRegistrationProvider
{
private readonly IRemoteServiceInvoker _remoteServiceInvoker = remoteServiceInvoker;
private readonly IHtmlDocumentSynchronizer _htmlDocumentSynchronizer = htmlDocumentSynchronizer;
private readonly LSPRequestInvoker _requestInvoker = requestInvoker;
private readonly IFilePathService _filePathService = filePathService;
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<CohostDocumentPullDiagnosticsEndpoint>();

protected override bool MutatesSolutionState => false;
Expand Down Expand Up @@ -124,13 +121,8 @@ public ImmutableArray<Registration> GetRegistrations(VSInternalClientCapabilitie

private async Task<LspDiagnostic[]> GetCSharpDiagnosticsAsync(TextDocument razorDocument, CancellationToken cancellationToken)
{
// TODO: This code will not work when the source generator is hooked up.
// How do we get the source generated C# document without OOP? Can we reverse engineer a file path?
var projectKey = razorDocument.Project.ToProjectKey();
var csharpFilePath = _filePathService.GetRazorCSharpFilePath(projectKey, razorDocument.FilePath.AssumeNotNull());
// We put the project Id in the generated document path, so there can only be one document
if (razorDocument.Project.Solution.GetDocumentIdsWithFilePath(csharpFilePath) is not [{ } generatedDocumentId] ||
razorDocument.Project.GetDocument(generatedDocumentId) is not { } generatedDocument)
if (!razorDocument.TryComputeHintNameFromRazorDocument(out var hintName) ||
await razorDocument.Project.TryGetSourceGeneratedDocumentFromHintNameAsync(hintName, cancellationToken).ConfigureAwait(false) is not { } generatedDocument)
{
return [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ Task InitializeOOPAsync(RazorRemoteHostClient remoteClient)
{
UseRazorCohostServer = _languageServerFeatureOptions.UseRazorCohostServer,
UsePreciseSemanticTokenRanges = _languageServerFeatureOptions.UsePreciseSemanticTokenRanges,
CSharpVirtualDocumentSuffix = _languageServerFeatureOptions.CSharpVirtualDocumentSuffix,
HtmlVirtualDocumentSuffix = _languageServerFeatureOptions.HtmlVirtualDocumentSuffix,
IncludeProjectKeyInGeneratedFilePath = _languageServerFeatureOptions.IncludeProjectKeyInGeneratedFilePath,
ReturnCodeActionAndRenamePathsWithPrefixedSlash = _languageServerFeatureOptions.ReturnCodeActionAndRenamePathsWithPrefixedSlash,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;

public class CohostDocumentPullDiagnosticsTest(FuseTestContext context, ITestOutputHelper testOutputHelper) : CohostEndpointTestBase(testOutputHelper), IClassFixture<FuseTestContext>
{
[FuseFact(Skip = "Need to get generated C# doc without OOP")]
[FuseFact]
public Task CSharp()
=> VerifyDiagnosticsAsync("""
<div></div>
Expand Down Expand Up @@ -107,7 +107,7 @@ public Task FilterEscapedAtFromCss()
}]);
}

[FuseFact(Skip = "Need to get generated C# doc without OOP")]
[FuseFact]
public Task CombinedAndNestedDiagnostics()
=> VerifyDiagnosticsAsync("""
@using System.Threading.Tasks;
Expand Down Expand Up @@ -144,7 +144,7 @@ private async Task VerifyDiagnosticsAsync(TestCode input, VSInternalDiagnosticRe

var requestInvoker = new TestLSPRequestInvoker([(VSInternalMethods.DocumentPullDiagnosticName, htmlResponse)]);

var endpoint = new CohostDocumentPullDiagnosticsEndpoint(RemoteServiceInvoker, TestHtmlDocumentSynchronizer.Instance, requestInvoker, FilePathService, LoggerFactory);
var endpoint = new CohostDocumentPullDiagnosticsEndpoint(RemoteServiceInvoker, TestHtmlDocumentSynchronizer.Instance, requestInvoker, LoggerFactory);

var result = await endpoint.GetTestAccessor().HandleRequestAsync(document, DisposalToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;

public abstract class CohostEndpointTestBase(ITestOutputHelper testOutputHelper) : ToolingTestBase(testOutputHelper)
{
private const string CSharpVirtualDocumentSuffix = ".g.cs";
private ExportProvider? _exportProvider;
private TestRemoteServiceInvoker? _remoteServiceInvoker;
private RemoteClientInitializationOptions _clientInitializationOptions;
Expand Down Expand Up @@ -76,7 +75,6 @@ protected override async Task InitializeAsync()

_clientInitializationOptions = new()
{
CSharpVirtualDocumentSuffix = CSharpVirtualDocumentSuffix,
HtmlVirtualDocumentSuffix = ".g.html",
IncludeProjectKeyInGeneratedFilePath = false,
UsePreciseSemanticTokenRanges = false,
Expand Down

0 comments on commit 8054ca5

Please sign in to comment.