-
Notifications
You must be signed in to change notification settings - Fork 418
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
Support go to definition from metadata source #883
Changes from 4 commits
f302015
2a8be67
f57c98c
c317826
645f28f
a83e33f
65ecfe7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
using System.Collections.Generic; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using Newtonsoft.Json; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
using System.Linq; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.CSharp; | ||
using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
using OmniSharp.Models.GotoDefinition; | ||
using OmniSharp.Roslyn.CSharp.Services.Navigation; | ||
using OmniSharp.Models.Metadata; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort usings? |
||
using TestUtility; | ||
using Xunit; | ||
using Xunit.Abstractions; | ||
|
@@ -159,6 +162,76 @@ await TestGoToMetadataAsync(testFile, | |
expectedTypeName: "System.String"); | ||
} | ||
|
||
[Theory] | ||
[InlineData("bar.cs")] | ||
[InlineData("bar.csx")] | ||
public async Task ReturnsDefinitionInMetadata_FromMetadata_WhenSymbolIsType(string filename) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, unifying the pattern for calling each handler would be helpful for a minor readability improvement. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the comments though! Very helpful! |
||
{ | ||
var testFile = new TestFile(filename, @" | ||
using System; | ||
class Bar { | ||
public void Baz() { | ||
var number = in$$t.MaxValue; | ||
} | ||
}"); | ||
|
||
using (var host = CreateOmniSharpHost(testFile)) | ||
{ | ||
var point = testFile.Content.GetPointFromPosition(); | ||
|
||
// 1. start by asking for definition of "int" | ||
var gotoDefinitionRequest = new GotoDefinitionRequest | ||
{ | ||
FileName = testFile.FileName, | ||
Line = point.Line, | ||
Column = point.Offset, | ||
WantMetadata = true | ||
}; | ||
var gotoDefinitionRequestHandler = GetRequestHandler(host); | ||
var gotoDefinitionResponse = await gotoDefinitionRequestHandler.Handle(gotoDefinitionRequest); | ||
|
||
// 2. now, based on the response information | ||
// go to the metadata endpoint, and ask for "int" specific metadata | ||
var metadataRequest = new MetadataRequest | ||
{ | ||
AssemblyName = gotoDefinitionResponse.MetadataSource.AssemblyName, | ||
TypeName = gotoDefinitionResponse.MetadataSource.TypeName, | ||
ProjectName = gotoDefinitionResponse.MetadataSource.ProjectName, | ||
Language = gotoDefinitionResponse.MetadataSource.Language | ||
}; | ||
var metadataRequestHandler = host.GetRequestHandler<MetadataService>(OmniSharpEndpoints.Metadata); | ||
var metadataResponse = await metadataRequestHandler.Handle(metadataRequest); | ||
|
||
// 3. the metadata response contains SourceName (metadata "file") and SourceText (syntax tree) | ||
// use the source to locate "IComparable" which is an interface implemented by Int32 struct | ||
var metadataTree = CSharpSyntaxTree.ParseText(metadataResponse.Source); | ||
var iComparable = metadataTree.GetCompilationUnitRoot(). | ||
DescendantNodesAndSelf(). | ||
OfType<BaseTypeDeclarationSyntax>().First(). | ||
BaseList.Types.FirstOrDefault(x => x.Type.ToString() == "IComparable"); | ||
var relevantLineSpan = iComparable.GetLocation().GetLineSpan(); | ||
|
||
// 4. now ask for the definition of "IComparable" | ||
// pass in the SourceName (metadata "file") as FileName - since it's not a regular file in our workspace | ||
var metadataNavigationRequest = new GotoDefinitionRequest | ||
{ | ||
FileName = metadataResponse.SourceName, | ||
Line = relevantLineSpan.StartLinePosition.Line, | ||
Column = relevantLineSpan.StartLinePosition.Character, | ||
WantMetadata = true | ||
}; | ||
var metadataNavigationResponse = await gotoDefinitionRequestHandler.Handle(metadataNavigationRequest); | ||
|
||
// 5. validate the response to be matching the expected IComparable meta info | ||
Assert.NotNull(metadataNavigationResponse.MetadataSource); | ||
Assert.Equal(AssemblyHelpers.CorLibName, metadataNavigationResponse.MetadataSource.AssemblyName); | ||
Assert.Equal("System.IComparable", metadataNavigationResponse.MetadataSource.TypeName); | ||
|
||
Assert.NotEqual(0, metadataNavigationResponse.Line); | ||
Assert.NotEqual(0, metadataNavigationResponse.Column); | ||
} | ||
} | ||
|
||
private async Task TestGoToSourceAsync(params TestFile[] testFiles) | ||
{ | ||
var response = await GetResponseAsync(testFiles, wantMetadata: false); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could technically work for other endpoints as well right? Thinking of
/findusages
,/typelookup
and maybe even/highlight
.Would be great if we could simplify this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably just make it
_metadataHelper.FindDocumentInMetadataCache(request.FileName) ?? _workspace.GetDocument(request.FileName);
to avoid the first if there.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes this approach should work for other endpoints too