Skip to content

Commit

Permalink
Merge pull request #75176 from genlu/removeResultId
Browse files Browse the repository at this point in the history 
Remove ResultId from RelatedDocumentsHandler
  • Loading branch information
@genlu
genlu authored Sep 19, 2024
2 parents 0dd4ad7 + 78a6fed commit 4d21882
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 98 deletions.
90 changes: 18 additions & 72 deletions src/LanguageServer/Protocol/Handler/RelatedDocuments/RelatedDocumentsHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.RelatedDocuments;
using Microsoft.CodeAnalysis.Serialization;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CommonLanguageServerProtocol.Framework;
Expand All @@ -34,44 +31,17 @@ internal sealed class RelatedDocumentsHandler
: ILspServiceRequestHandler<VSInternalRelatedDocumentParams, VSInternalRelatedDocumentReport[]?>,
ITextDocumentIdentifierHandler<VSInternalRelatedDocumentParams, TextDocumentIdentifier>
{
/// <summary>
/// Cache where we store the data produced by prior requests so that they can be returned if nothing of significance
/// changed. The version key is produced by combining the checksums for project options <see
/// cref="ProjectState.GetParseOptionsChecksum"/> and <see cref="DocumentStateChecksums.Text"/>
/// </summary>
private readonly VersionedPullCache<(Checksum parseOptionsChecksum, Checksum textChecksum)?> _versionedCache = new(nameof(RelatedDocumentsHandler));

public bool MutatesSolutionState => false;
public bool RequiresLSPSolution => true;

private static async Task<(Checksum parseOptionsChecksum, Checksum textChecksum)> ComputeChecksumsAsync(Document document, CancellationToken cancellationToken)
{
var project = document.Project;
var parseOptionsChecksum = project.State.GetParseOptionsChecksum();

var documentChecksumState = await document.State.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false);
var textChecksum = documentChecksumState.Text;

return (parseOptionsChecksum, textChecksum);
}

public TextDocumentIdentifier GetTextDocumentIdentifier(VSInternalRelatedDocumentParams requestParams)
=> requestParams.TextDocument;

/// <summary>
/// Retrieve the previous results we reported. Used so we can avoid resending data for unchanged files.
/// </summary>
private static ImmutableArray<PreviousPullResult>? GetPreviousResults(VSInternalRelatedDocumentParams requestParams)
=> requestParams.PreviousResultId != null && requestParams.TextDocument != null
? [new PreviousPullResult(requestParams.PreviousResultId, requestParams.TextDocument)]
// The client didn't provide us with a previous result to look for, so we can't lookup anything.
: null;

public async Task<VSInternalRelatedDocumentReport[]?> HandleRequestAsync(
VSInternalRelatedDocumentParams requestParams, RequestContext context, CancellationToken cancellationToken)
{
context.TraceInformation($"{this.GetType()} started getting related documents");
context.TraceInformation($"PreviousResultId={requestParams.PreviousResultId}");

var solution = context.Solution;
var document = context.Document;
Expand All @@ -90,52 +60,28 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(VSInternalRelatedDocumen
// The progress object we will stream reports to.
using var progress = BufferedProgress.Create(requestParams.PartialResultToken);

var documentToPreviousParams = new Dictionary<Document, PreviousPullResult>();
if (requestParams.PreviousResultId != null)
documentToPreviousParams.Add(document, new PreviousPullResult(requestParams.PreviousResultId, requestParams.TextDocument));

var newResultId = await _versionedCache.GetNewResultIdAsync(
documentToPreviousParams,
document,
computeVersionAsync: async () => await ComputeChecksumsAsync(document, cancellationToken).ConfigureAwait(false),
cancellationToken).ConfigureAwait(false);
if (newResultId != null)
{
context.TraceInformation($"Version was changed for document: {document.FilePath}");

var linePosition = requestParams.Position is null
? new LinePosition(0, 0)
: ProtocolConversions.PositionToLinePosition(requestParams.Position);
var linePosition = requestParams.Position is null
? new LinePosition(0, 0)
: ProtocolConversions.PositionToLinePosition(requestParams.Position);

var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
var position = text.Lines.GetPosition(linePosition);
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
var position = text.Lines.GetPosition(linePosition);

await relatedDocumentsService.GetRelatedDocumentIdsAsync(
document,
position,
(relatedDocumentIds, cancellationToken) =>
await relatedDocumentsService.GetRelatedDocumentIdsAsync(
document,
position,
(relatedDocumentIds, cancellationToken) =>
{
// As the related docs services reports document ids to us, stream those immediately through our
// progress reporter.
progress.Report(new VSInternalRelatedDocumentReport
{
// As the related docs services reports document ids to us, stream those immediately through our
// progress reporter.
progress.Report(new VSInternalRelatedDocumentReport
{
ResultId = newResultId,
FilePaths = relatedDocumentIds.Select(id => solution.GetRequiredDocument(id).FilePath).WhereNotNull().ToArray(),
});

return ValueTaskFactory.CompletedTask;
},
cancellationToken).ConfigureAwait(false);
}
else
{
context.TraceInformation($"Version was unchanged for document: {document.FilePath}");
FilePaths = relatedDocumentIds.Select(id => solution.GetRequiredDocument(id).FilePath).WhereNotNull().ToArray(),
});

// Nothing changed between the last request and this one. Report a (null-file-paths, same-result-id)
// response to the client as that means they should just preserve the current related file paths they
// have for this file.
progress.Report(new VSInternalRelatedDocumentReport { ResultId = requestParams.PreviousResultId });
}
return ValueTaskFactory.CompletedTask;
},
cancellationToken).ConfigureAwait(false);

// If we had a progress object, then we will have been reporting to that. Otherwise, take what we've been
// collecting and return that.
Expand Down
19 changes: 8 additions & 11 deletions src/LanguageServer/Protocol/Protocol/Internal/VSInternalRelatedDocumentParams.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,15 @@ namespace Roslyn.LanguageServer.Protocol
/// <summary>
/// Parameter for copilot/_related_documents.
/// </summary>
internal sealed class VSInternalRelatedDocumentParams : VSInternalStreamingParams, IPartialResultParams<VSInternalRelatedDocumentReport[]>
internal sealed class VSInternalRelatedDocumentParams : IPartialResultParams<VSInternalRelatedDocumentReport[]>
{
/// <summary>
/// Gets or sets the document for which the feature is being requested for.
/// </summary>
[JsonPropertyName("_vs_textDocument")]
[JsonRequired]
public TextDocumentIdentifier TextDocument { get; set; }

/// <summary>
/// Gets or sets the value which indicates the position within the document.
/// </summary>
Expand All @@ -27,16 +34,6 @@ internal sealed class VSInternalRelatedDocumentParams : VSInternalStreamingParam

internal sealed class VSInternalRelatedDocumentReport
{
/// <summary>
/// Gets or sets the server-generated version number for the related documents result. This is treated as a
/// black box by the client: it is stored on the client for each textDocument and sent back to the server when
/// requesting related documents. The server can use this result ID to avoid resending results
/// that had previously been sent.
/// </summary>
[JsonPropertyName("_vs_resultId")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public string? ResultId { get; set; }

[JsonPropertyName("_vs_file_paths")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public string[]? FilePaths { get; set; }
Expand Down
21 changes: 6 additions & 15 deletions src/LanguageServer/ProtocolUnitTests/RelatedDocuments/RelatedDocumentsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public sealed class RelatedDocumentsTests(ITestOutputHelper testOutputHelper)
private static async Task<VSInternalRelatedDocumentReport[]> RunGetRelatedDocumentsAsync(
TestLspServer testLspServer,
Uri uri,
string? previousResultId = null,
bool useProgress = false)
{
BufferedProgress<VSInternalRelatedDocumentReport[]>? progress = useProgress ? BufferedProgress.Create<VSInternalRelatedDocumentReport[]>(null) : null;
Expand All @@ -30,7 +29,6 @@ private static async Task<VSInternalRelatedDocumentReport[]> RunGetRelatedDocume
new VSInternalRelatedDocumentParams
{
TextDocument = new TextDocumentIdentifier { Uri = uri },
PreviousResultId = previousResultId,
PartialResultToken = progress,
},
CancellationToken.None).ConfigureAwait(false);
Expand Down Expand Up @@ -135,7 +133,7 @@ class Z
}

[Theory, CombinatorialData]
public async Task TestResultIds(bool mutatingLspWorkspace, bool useProgress)
public async Task TestRepeatInvocations(bool mutatingLspWorkspace, bool useProgress)
{
var markup1 = """
class X
Expand All @@ -158,29 +156,22 @@ class Y
project.Documents.First().GetURI(),
useProgress: useProgress);

AssertJsonEquals(results1, new VSInternalRelatedDocumentReport[]
var expectedResult = new VSInternalRelatedDocumentReport[]
{
new()
{
ResultId = "RelatedDocumentsHandler:0",
FilePaths = [project.Documents.Last().FilePath!],
}
});
};

AssertJsonEquals(results1, expectedResult);

// Calling again, without a change, should return the old result id and no filepaths.
var results2 = await RunGetRelatedDocumentsAsync(
testLspServer,
project.Documents.First().GetURI(),
previousResultId: results1.Single().ResultId,
useProgress: useProgress);

AssertJsonEquals(results2, new VSInternalRelatedDocumentReport[]
{
new()
{
ResultId = "RelatedDocumentsHandler:0",
FilePaths = null,
}
});
AssertJsonEquals(results2, expectedResult);
}
}

0 comments on commit 4d21882

Please sign in to comment.