Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for implementation edges in our LSIF indexer #64263

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion src/Features/Lsif/Generator/Generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,45 @@ SymbolKind.RangeVariable or
{
var definitionResultsId = symbolResultsTracker.GetResultIdForSymbol(declaredSymbol, Methods.TextDocumentDefinitionName, () => new DefinitionResult(idFactory));
lsifJsonWriter.Write(new Item(definitionResultsId.As<DefinitionResult, Vertex>(), lazyRangeVertex.Value.GetId(), documentVertex.GetId(), idFactory));

// If this declared symbol also implements an interface member, we count this as a definition of the interface member as well.
// Note in C# there are estoeric cases where a method can implement an interface member even though the containing type does not
// implement the interface, for example in this case:
//
// interface I { void M(); }
// class Base { public void M() { } }
// class Derived : Base, I { }
//
// We don't worry about supporting these cases here.
var implementedMembers = declaredSymbol.ExplicitOrImplicitInterfaceImplementations();

foreach (var implementedMember in implementedMembers)
jasonmalinowski marked this conversation as resolved.
Show resolved Hide resolved
MarkImplementationOfSymbol(implementedMember);

// If this overrides a method, we'll also mark it the same way. We want to chase to the base virtual method, skipping over intermediate
// methods so that way all overrides of the same method point to the same virtual method
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting - I would have thought we would have pointed to intermediate members which themselves point back to the base in a chain. This loses the intermediate member information, but I have no idea if that is OK or not (not sure how this is used).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this and decided to go with this way for now. We saw two potential downsides of having the full chain:

  1. It's not clear what happens if somebody removes their method of an override in the middle of a chain, since then the ends of the chain are disconnected.
  2. It means a find references in the cloud has to connect all the chains together, as opposed to doing a faster lookup.

So we're going this way for now.

if (declaredSymbol.IsOverride)
{
var overridenMember = declaredSymbol.GetOverriddenMember();

while (overridenMember?.GetOverriddenMember() != null)
overridenMember = overridenMember.GetOverriddenMember();

if (overridenMember != null)
MarkImplementationOfSymbol(overridenMember);
}

void MarkImplementationOfSymbol(ISymbol baseMember)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only way this will make sense is if you look at the picture in #59617.

{
// First we create a definition link for the reference results for the base member
var referenceResultsId = symbolResultsTracker.GetResultSetReferenceResultId(baseMember.OriginalDefinition, idFactory);
lsifJsonWriter.Write(new Item(referenceResultsId.As<ReferenceResult, Vertex>(), lazyRangeVertex.Value.GetId(), documentVertex.GetId(), idFactory, property: "definitions"));

// Then also link the result set for the method to the moniker that it implements
referenceResultsId = symbolResultsTracker.GetResultSetReferenceResultId(declaredSymbol.OriginalDefinition, idFactory);
var implementedMemberMoniker = symbolResultsTracker.GetResultIdForSymbol<Moniker>(baseMember.OriginalDefinition, "moniker", () => throw new Exception("When we produced the resultSet, we should have already created a moniker for it."));
lsifJsonWriter.Write(new Item(referenceResultsId.As<ReferenceResult, Vertex>(), implementedMemberMoniker, documentVertex.GetId(), idFactory, property: "referenceLinks"));
}
}

if (referencedSymbol != null)
Expand All @@ -259,7 +298,7 @@ SymbolKind.RangeVariable or
// symbol but the range can point a different symbol's resultSet. This can happen if the token is
// both a definition of a symbol (where we will point to the definition) but also a reference to some
// other symbol.
var referenceResultsId = symbolResultsTracker.GetResultIdForSymbol(referencedSymbol.GetOriginalUnreducedDefinition(), Methods.TextDocumentReferencesName, () => new ReferenceResult(idFactory));
var referenceResultsId = symbolResultsTracker.GetResultSetReferenceResultId(referencedSymbol.GetOriginalUnreducedDefinition(), idFactory);
lsifJsonWriter.Write(new Item(referenceResultsId.As<ReferenceResult, Vertex>(), lazyRangeVertex.Value.GetId(), documentVertex.GetId(), idFactory, property: "references"));
}

Expand Down
9 changes: 8 additions & 1 deletion src/Features/Lsif/Generator/Graph/Item.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.Graph
{
/// <summary>
/// Represents a single item that points to a range from a result. See https://github.com/Microsoft/language-server-protocol/blob/master/indexFormat/specification.md#request-textdocumentreferences
/// Represents a single item that points to a range or moniker from a result. See https://github.com/Microsoft/language-server-protocol/blob/master/indexFormat/specification.md#request-textdocumentreferences
/// for an example of item edges.
/// </summary>
internal sealed class Item : Edge
Expand All @@ -19,5 +19,12 @@ public Item(Id<Vertex> outVertex, Id<Range> range, Id<LsifDocument> document, Id
Shard = document;
Property = property;
}

public Item(Id<Vertex> outVertex, Id<Moniker> moniker, Id<LsifDocument> document, IdFactory idFactory, string? property = null)
: base(label: "item", outVertex, new[] { moniker.As<Moniker, Vertex>() }, idFactory)
{
Shard = document;
Property = property;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,23 @@ namespace Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.ResultSetTr
/// </summary>
internal interface IResultSetTracker
{
/// <summary>
/// Returns the ID of the <see cref="ResultSet"/> that represents a symbol.
/// </summary>
Id<ResultSet> GetResultSetIdForSymbol(ISymbol symbol);

/// <summary>
/// Returns an ID of a vertex that is linked from a result set. For example, a <see cref="ResultSet"/> has an edge that points to a <see cref="ReferenceResult"/>, and
/// item edges from that <see cref="ReferenceResult"/> are the references for the range. This gives you the ID of the <see cref="ReferenceResult"/> in this case.
/// </summary>
Id<T> GetResultIdForSymbol<T>(ISymbol symbol, string edgeKind, Func<T> vertexCreator) where T : Vertex;

/// <summary>
/// Similar to <see cref="GetResultIdForSymbol{T}(ISymbol, string, Func{T})"/>, but instead of creating the vertex (if needed) and adding an edge, this
/// simply tracks that this method has been called, and it's up to the caller that got a true return value to create and add the vertex themselves. This is handy
/// when the actual identity of the node isn't needed by any other consumers, or the vertex creation is expensive and we don't want it running under the lock that
/// <see cref="GetResultIdForSymbol{T}(ISymbol, string, Func{T})"/> would have to take.
/// </summary>
bool ResultSetNeedsInformationalEdgeAdded(ISymbol symbol, string edgeKind);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.Graph;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.ResultSetTracking
{
internal static class IResultSetTrackerExtensions
{
/// <summary>
/// Returns the ID of the <see cref="ReferenceResult"/> for a <see cref="ResultSet"/>.
/// </summary>
public static Id<ReferenceResult> GetResultSetReferenceResultId(this IResultSetTracker tracker, ISymbol symbol, IdFactory idFactory)
=> tracker.GetResultIdForSymbol(symbol, Methods.TextDocumentReferencesName, () => new ReferenceResult(idFactory));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ private TrackedResultSet GetTrackedResultSet(ISymbol symbol)

if (monikerVertex != null)
{
_lsifJsonWriter.Write(monikerVertex);
_lsifJsonWriter.Write(Edge.Create("moniker", trackedResultSet.Id, monikerVertex.GetId(), _idFactory));
// Attach the moniker vertex for this result set
_ = GetResultIdForSymbol(symbol, "moniker", () => monikerVertex);
}
}

Expand Down Expand Up @@ -156,15 +156,15 @@ public TrackedResultSet(Id<ResultSet> id)
Id = id;
}

public Id<T> GetResultId<T>(string edgeKind, Func<T> vertexCreator, ILsifJsonWriter lsifJsonWriter, IdFactory idFactory) where T : Vertex
public Id<T> GetResultId<T>(string edgeLabel, Func<T> vertexCreator, ILsifJsonWriter lsifJsonWriter, IdFactory idFactory) where T : Vertex
{
lock (_edgeKindToVertexId)
{
if (_edgeKindToVertexId.TryGetValue(edgeKind, out var existingId))
if (_edgeKindToVertexId.TryGetValue(edgeLabel, out var existingId))
{
if (!existingId.HasValue)
{
throw new Exception($"This ResultSet already has an edge of {edgeKind} as {nameof(ResultSetNeedsInformationalEdgeAdded)} was called with this edge kind.");
throw new Exception($"This ResultSet already has an edge of {edgeLabel} as {nameof(ResultSetNeedsInformationalEdgeAdded)} was called with this edge label.");
}

// TODO: this is a violation of the type system here, really: we're assuming that all calls to this function with the same edge kind
Expand All @@ -173,10 +173,10 @@ public Id<T> GetResultId<T>(string edgeKind, Func<T> vertexCreator, ILsifJsonWri
}

var vertex = vertexCreator();
_edgeKindToVertexId.Add(edgeKind, vertex.GetId().As<T, Vertex>());
_edgeKindToVertexId.Add(edgeLabel, vertex.GetId().As<T, Vertex>());

lsifJsonWriter.Write(vertex);
lsifJsonWriter.Write(Edge.Create(edgeKind, Id, vertex.GetId(), idFactory));
lsifJsonWriter.Write(Edge.Create(edgeLabel, Id, vertex.GetId(), idFactory));

return vertex.GetId();
}
Expand Down
70 changes: 70 additions & 0 deletions src/Features/Lsif/GeneratorTest/RangeResultSetTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.Test.Utilities
Imports Microsoft.VisualStudio.LanguageServer.Protocol
Imports Roslyn.Test.Utilities

Namespace Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.UnitTests
<UseExportProvider>
Expand Down Expand Up @@ -185,5 +186,74 @@ Namespace Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.UnitTests
End If
Next
End Function

<Fact>
Public Async Function InterfaceImplementation() As Task
Dim lsif = Await TestLsifOutput.GenerateForWorkspaceAsync(
TestWorkspace.CreateWorkspace(
<Workspace>
<Project Language="C#" AssemblyName=<%= TestProjectAssemblyName %> FilePath="Z:\TestProject.csproj" CommonReferences="true">
<Document Name="A.cs" FilePath="Z:\A.cs">
interface I { void {|Base:M|}(); }
class C : I { public void {|Implementation:M|}() { } }
</Document>
</Project>
</Workspace>))

Await AssertImplementationCorrectlyLinked(lsif)
End Function

<Fact>
Public Async Function [Overrides]() As Task
Dim lsif = Await TestLsifOutput.GenerateForWorkspaceAsync(
TestWorkspace.CreateWorkspace(
<Workspace>
<Project Language="C#" AssemblyName=<%= TestProjectAssemblyName %> FilePath="Z:\TestProject.csproj" CommonReferences="true">
<Document Name="A.cs" FilePath="Z:\A.cs">
class C { public virtual void {|Base:M|}() { } }
class D : C { public override void {|Implementation:M|}() { } }
</Document>
</Project>
</Workspace>))

Await AssertImplementationCorrectlyLinked(lsif)
End Function

<Fact>
Public Async Function OverridesAlwaysPointsToInitialVirtualMethod() As Task
Dim lsif = Await TestLsifOutput.GenerateForWorkspaceAsync(
TestWorkspace.CreateWorkspace(
<Workspace>
<Project Language="C#" AssemblyName=<%= TestProjectAssemblyName %> FilePath="Z:\TestProject.csproj" CommonReferences="true">
<Document Name="A.cs" FilePath="Z:\A.cs">
class C { public virtual void {|Base:M|}() { } }
class D : C { public override void {|Implementation:M|}() { } }
class E : D { public override void {|Implementation:M|}() { } }
</Document>
</Project>
</Workspace>))

Await AssertImplementationCorrectlyLinked(lsif)
End Function

Private Shared Async Function AssertImplementationCorrectlyLinked(lsif As TestLsifOutput) As Task
Dim interfaceMethodRange = Await lsif.GetAnnotatedRangeAsync("Base")
Dim implementationRanges = Await lsif.GetAnnotatedRangesAsync("Implementation")

' The references vertex should have a definition items pointing ot the implementations
Dim resultSetVertex = lsif.GetLinkedVertices(Of Graph.ResultSet)(interfaceMethodRange, "next").Single()
Dim expectedMoniker = lsif.GetLinkedVertices(Of Graph.Moniker)(resultSetVertex, "moniker").Single()
Dim referencesVertex = lsif.GetLinkedVertices(Of Graph.ReferenceResult)(resultSetVertex, Methods.TextDocumentReferencesName).Single()
Dim definitionRangesForImplementingMethods = lsif.GetLinkedVertices(Of Graph.Range)(referencesVertex, Function(e) DirectCast(e, Graph.Item).Property = "definitions")
AssertEx.SetEqual(implementationRanges, definitionRangesForImplementingMethods)

' The result set for the implementation method should point to the moniker of the base
For Each implementationRange In implementationRanges
resultSetVertex = lsif.GetLinkedVertices(Of Graph.ResultSet)(implementationRange, "next").Single()
referencesVertex = lsif.GetLinkedVertices(Of Graph.ReferenceResult)(resultSetVertex, Methods.TextDocumentReferencesName).Single()
Dim moniker = lsif.GetLinkedVertices(Of Graph.Moniker)(referencesVertex, Function(e) DirectCast(e, Graph.Item).Property = "referenceLinks").Single()
Assert.Same(expectedMoniker, moniker)
Next
End Function
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,19 @@ Namespace Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.UnitTests.U
''' Returns all the vertices linked to the given vertex by the edge type.
''' </summary>
Public Function GetLinkedVertices(Of T As Vertex)(vertex As Graph.Vertex, edgeLabel As String) As ImmutableArray(Of T)
Return GetLinkedVertices(Of T)(vertex, Function(e) e.Label = edgeLabel)
End Function

''' <summary>
''' Returns all the vertices linked to the given vertex by the edge predicate.
''' </summary>
Public Function GetLinkedVertices(Of T As Vertex)(vertex As Graph.Vertex, predicate As Func(Of Edge, Boolean)) As ImmutableArray(Of T)
SyncLock _gate
Dim builder = ImmutableArray.CreateBuilder(Of T)

Dim edges As List(Of Edge) = Nothing
If _edgesByOutVertex.TryGetValue(vertex, edges) Then
Dim inVerticesId = edges.Where(Function(e) e.Label = edgeLabel).SelectMany(Function(e) e.InVertices)
Dim inVerticesId = edges.Where(predicate).SelectMany(Function(e) e.InVertices)

For Each inVertexId In inVerticesId
' This is an unsafe "cast" if you will converting the ID to the expected type;
Expand Down
Loading