Initial work to create a service for returning related documents for copilot purposes. #74906
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
src/Features/Core/Portable/RelatedDocuments/AbstractRelatedDocumentsService.cs
Show resolved
Hide resolved
| /// examples might be checking to see which symbols are used at that particular location and prioritizing documents | ||
| /// those symbols are defined in. | ||
| /// </summary> | ||
| ValueTask GetRelatedDocumentIdsAsync(Document document, int position, Func<DocumentId, CancellationToken, ValueTask> callbackAsync, CancellationToken cancellationToken); |
There was a problem hiding this comment.
switched to callback-style (Vs IAsyncEnumerable). it was just much easier to model, once i had all the OOP stuff hooked up.
| [ExportLanguageService(typeof(IRelatedDocumentsService), LanguageNames.CSharp), Shared] | ||
| [method: ImportingConstructor] | ||
| [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
| internal sealed class CSharpRelatedDocumentsService() : AbstractRelatedDocumentsService< |
There was a problem hiding this comment.
language specific logic. for now, the lang specific logic is just iterating and finding which nodes to bind. we bind Name nodes (X, X.Y, X<Y>, etc.), and member access nodes with a left dotter name (like the X.Y in X.Y.Z()).
|
|
||
| namespace Microsoft.CodeAnalysis.RelatedDocuments; | ||
|
|
||
| internal interface IRemoteRelatedDocumentsService |
There was a problem hiding this comment.
remote side of the API. we want to defer to OOP to do things so we're not churning through tons of memory on the VS side, causing impactful GCs.
| [ExportRemoteServiceCallbackDispatcher(typeof(IRemoteRelatedDocumentsService)), Shared] | ||
| [method: ImportingConstructor] | ||
| [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
| internal sealed class RelatedDocumentsServiceServerCallbackDispatcher() : RemoteServiceCallbackDispatcher, IRemoteRelatedDocumentsService.ICallback |
There was a problem hiding this comment.
next two types are just OOP goo to allow an oop service to report bvack results as they are found.
| RemoteServiceCallbackId callbackId, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| return RunServiceAsync(solutionChecksum, async solution => |
There was a problem hiding this comment.
oops service is trivial, it just receives the inputs, gets the service on the oop side, and calls into it.
|
NOte: as this is green, i'd like to roll any non-critical changes into: #74918 |
| { | ||
| protected override async ValueTask GetRelatedDocumentIdsInCurrentProcessAsync( | ||
| Document document, | ||
| int position, |
There was a problem hiding this comment.
it is. you can just pass 0.
There was a problem hiding this comment.
Consider using -1 for this purpose. I think 0 is a valid position in file, so potentially we could return a different list of related files for "position 0" compare to "the entire file"
| if (IsPossibleTypeName(memberAccess.Expression, out var nameToken)) | ||
| { | ||
| // Something like `X.Y.Z` where `X.Y` is a type name. Bind X.Y | ||
| yield return (memberAccess.Expression, nameToken); |
There was a problem hiding this comment.
For X.Y.Z, memberAccess.Expression is X.Y, but the nameToken returned would be X, right? Shouldn't it be Y?
There was a problem hiding this comment.
not exactly. we have X.Y.Z. But we pass X.Y Into IsPossibleTypeName, and if it succeeds, we return Y.
| // results to whatever client is calling into us. | ||
| await ProducerConsumer<DocumentId>.RunParallelAsync( | ||
| // Order the nodes by the distance from the requested position. | ||
| IteratePotentialTypeNodes(root).OrderBy(t => t.expression.SpanStart - position), |
There was a problem hiding this comment.
yup. That would make more sense!
There was a problem hiding this comment.
This is a solid starting point. I have a couple of question about the code, other than those LGTM. Feel free to address in the follow up PR
One concern is the perf/behavior on really large files. We might consider a different grouping strategy like
- (if a position is provided) find types referenced within enclosing member
- types referenced in other members' signatures
- types referenced inside other members
We could always try to return everything from first group in the first callback, so consumer of the API would always get the most relevant ones and have an opportunity to decide if they want to wait for more. Thoughts?
Totally. Though note that the design of this (including with LSP) is to be entirely streaming based. The service just keeps calling the callback as it finds results, and teh client just pulls as long as it wants. This works in LSP as well, where we take the results we are informed about, and we send them along as 'reports' to the client. |
Yup. All those ideas are great. This was meant more as a starting position. The code that iterates the nodes can be greatly tweaked as we go forward. |
No description provided.