From f3020155a0ae51ed9315258ef97496a1520fc332 Mon Sep 17 00:00:00 2001 From: filipw Date: Fri, 26 May 2017 23:59:18 +0200 Subject: [PATCH 1/4] added support for go to definition for metadata files --- src/OmniSharp.Abstractions/Models/Request.cs | 8 +- .../Helpers/LocationExtensions.cs | 1 + .../Navigation/GotoDefinitionService.cs | 13 ++- .../Services/Navigation/MetadataService.cs | 8 +- .../Extensions/SymbolExtensions.cs | 13 +++ .../MetadataDocumentResult.cs | 17 ++++ src/OmniSharp.Roslyn/MetadataHelper.cs | 86 ++++++++++++------- src/OmniSharp.Roslyn/OmniSharpWorkspace.cs | 2 + .../FindReferencesFacts.cs | 58 +++++++++++++ .../GoToDefinitionFacts.cs | 78 ++++++++++++++++- 10 files changed, 240 insertions(+), 44 deletions(-) create mode 100644 src/OmniSharp.Roslyn/MetadataDocumentResult.cs diff --git a/src/OmniSharp.Abstractions/Models/Request.cs b/src/OmniSharp.Abstractions/Models/Request.cs index b51a9dd982..a18752864f 100644 --- a/src/OmniSharp.Abstractions/Models/Request.cs +++ b/src/OmniSharp.Abstractions/Models/Request.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.IO; using Newtonsoft.Json; @@ -25,5 +26,10 @@ public string FileName _fileName = value; } } + + public bool IsMetadataFile() + { + return FileName != null && FileName.StartsWith("$metadata$", StringComparison.OrdinalIgnoreCase); + } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs index 668a718c3f..c844c2c6c3 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs @@ -16,6 +16,7 @@ public static QuickFix GetQuickFix(this Location location, OmniSharpWorkspace wo var lineSpan = location.GetLineSpan(); var path = lineSpan.Path; var documents = workspace.GetDocuments(path); + var line = lineSpan.StartLinePosition.Line; var text = location.SourceTree.GetText().Lines[line].ToString(); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs index 3942962de0..14632cb81a 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs @@ -31,8 +31,12 @@ public async Task Handle(GotoDefinitionRequest request) { var quickFixes = new List(); - var document = _workspace.GetDocument(request.FileName); + var document = request.IsMetadataFile() ? + _metadataHelper.FindDocumentInMetadataCache(request.FileName) : + _workspace.GetDocument(request.FileName); + var response = new GotoDefinitionResponse(); + if (document != null) { var semanticModel = await document.GetSemanticModelAsync(); @@ -64,11 +68,12 @@ public async Task Handle(GotoDefinitionRequest request) else if (location.IsInMetadata && request.WantMetadata) { var cancellationSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(request.Timeout)); - var metadataDocument = await _metadataHelper.GetDocumentFromMetadata(document.Project, symbol, cancellationSource.Token); - if (metadataDocument != null) + var metadataDocumentResult = await _metadataHelper.GetAndAddDocumentFromMetadata(document.Project, symbol, cancellationSource.Token); + if (metadataDocumentResult.Document != null) { cancellationSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(request.Timeout)); - var metadataLocation = await _metadataHelper.GetSymbolLocationFromMetadata(symbol, metadataDocument, cancellationSource.Token); + + var metadataLocation = await _metadataHelper.GetSymbolLocationFromMetadata(symbol, metadataDocumentResult.Document, cancellationSource.Token); var lineSpan = metadataLocation.GetMappedLineSpan(); response = new GotoDefinitionResponse diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/MetadataService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/MetadataService.cs index 46d6c0bb13..45d7866498 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/MetadataService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/MetadataService.cs @@ -31,12 +31,12 @@ public async Task Handle(MetadataRequest request) if (symbol != null && symbol.ContainingAssembly.Name == request.AssemblyName) { var cancellationSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(request.Timeout)); - var document = await _metadataHelper.GetDocumentFromMetadata(project, symbol, cancellationSource.Token); - if (document != null) + var metadataDocumentResult = await _metadataHelper.GetAndAddDocumentFromMetadata(project, symbol, cancellationSource.Token); + if (metadataDocumentResult.Document != null) { - var source = await document.GetTextAsync(); - response.SourceName = _metadataHelper.GetFilePathForSymbol(project, symbol); + var source = await metadataDocumentResult.Document.GetTextAsync(); response.Source = source.ToString(); + response.SourceName = metadataDocumentResult.DocumentPath; return response; } diff --git a/src/OmniSharp.Roslyn/Extensions/SymbolExtensions.cs b/src/OmniSharp.Roslyn/Extensions/SymbolExtensions.cs index 9f4281c681..2e569ada1b 100644 --- a/src/OmniSharp.Roslyn/Extensions/SymbolExtensions.cs +++ b/src/OmniSharp.Roslyn/Extensions/SymbolExtensions.cs @@ -26,5 +26,18 @@ public static string GetKind(this ISymbol symbol) return Enum.GetName(symbol.Kind.GetType(), symbol.Kind); } + + internal static INamedTypeSymbol GetTopLevelContainingNamedType(this ISymbol symbol) + { + // Traverse up until we find a named type that is parented by the namespace + var topLevelNamedType = symbol; + while (topLevelNamedType.ContainingSymbol != symbol.ContainingNamespace || + topLevelNamedType.Kind != SymbolKind.NamedType) + { + topLevelNamedType = topLevelNamedType.ContainingSymbol; + } + + return (INamedTypeSymbol)topLevelNamedType; + } } } diff --git a/src/OmniSharp.Roslyn/MetadataDocumentResult.cs b/src/OmniSharp.Roslyn/MetadataDocumentResult.cs new file mode 100644 index 0000000000..e015642d76 --- /dev/null +++ b/src/OmniSharp.Roslyn/MetadataDocumentResult.cs @@ -0,0 +1,17 @@ +using Microsoft.CodeAnalysis; + +namespace OmniSharp.Roslyn +{ + public class MetadataDocumentResult + { + public MetadataDocumentResult(Document document, string documentPath) + { + Document = document; + DocumentPath = documentPath; + } + + public Document Document { get; } + + public string DocumentPath { get; } + } +} diff --git a/src/OmniSharp.Roslyn/MetadataHelper.cs b/src/OmniSharp.Roslyn/MetadataHelper.cs index 02a5979be0..41ba5f81a7 100644 --- a/src/OmniSharp.Roslyn/MetadataHelper.cs +++ b/src/OmniSharp.Roslyn/MetadataHelper.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Reflection; @@ -6,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; +using OmniSharp.Extensions; using OmniSharp.Services; using OmniSharp.Utilities; @@ -21,6 +23,7 @@ public class MetadataHelper private readonly Lazy _symbolKey; private readonly Lazy _metadataAsSourceHelper; private readonly Lazy _getLocationInGeneratedSourceAsync; + private Dictionary _metadataDocumentCache = new Dictionary(); private const string CSharpMetadataAsSourceService = "Microsoft.CodeAnalysis.CSharp.MetadataAsSource.CSharpMetadataAsSourceService"; private const string SymbolKey = "Microsoft.CodeAnalysis.SymbolKey"; @@ -28,6 +31,7 @@ public class MetadataHelper private const string GetLocationInGeneratedSourceAsync = "GetLocationInGeneratedSourceAsync"; private const string AddSourceToAsync = "AddSourceToAsync"; private const string Create = "Create"; + private const string MetadataKey = "$Metadata$"; public MetadataHelper(IAssemblyLoader loader) { @@ -43,36 +47,62 @@ public MetadataHelper(IAssemblyLoader loader) _getLocationInGeneratedSourceAsync = _metadataAsSourceHelper.LazyGetMethod(GetLocationInGeneratedSourceAsync); } - public string GetSymbolName(ISymbol symbol) + public Document FindDocumentInMetadataCache(string fileName) { - var topLevelSymbol = GetTopLevelContainingNamedType(symbol); - return GetTypeDisplayString(topLevelSymbol); + if (_metadataDocumentCache.TryGetValue(fileName, out var metadataDocument)) + { + return metadataDocument; + } + + return null; } - public string GetFilePathForSymbol(Project project, ISymbol symbol) + public string GetSymbolName(ISymbol symbol) { - var topLevelSymbol = GetTopLevelContainingNamedType(symbol); - return $"metadata/Project/{Folderize(project.Name)}/Assembly/{Folderize(topLevelSymbol.ContainingAssembly.Name)}/Symbol/{Folderize(GetTypeDisplayString(topLevelSymbol))}.cs".Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); + var topLevelSymbol = symbol.GetTopLevelContainingNamedType(); + return GetTypeDisplayString(topLevelSymbol); } - public Task GetDocumentFromMetadata(Project project, ISymbol symbol, CancellationToken cancellationToken = new CancellationToken()) + public async Task GetAndAddDocumentFromMetadata(Project project, ISymbol symbol, CancellationToken cancellationToken = new CancellationToken()) { - var filePath = GetFilePathForSymbol(project, symbol); - var topLevelSymbol = GetTopLevelContainingNamedType(symbol); + var fileName = GetFilePathForSymbol(project, symbol); + + Project metadataProject = null; // since submission projects cannot have new documents added to it - // we will use temporary project to hold metadata documents - var metadataProject = project.IsSubmission - ? project.Solution.AddProject("metadataTemp", "metadataTemp.dll", LanguageNames.CSharp) + // we will use a separate project to hold metadata documents + if (project.IsSubmission) + { + metadataProject = project.Solution.Projects.FirstOrDefault(x => x.Name == MetadataKey); + if (metadataProject == null) + { + metadataProject = project.Solution.AddProject(MetadataKey, $"{MetadataKey}.dll", LanguageNames.CSharp) .WithCompilationOptions(project.CompilationOptions) - .WithMetadataReferences(project.MetadataReferences) - : project; + .WithMetadataReferences(project.MetadataReferences); + } + } + else + { + // for regular projects we will use current project to store metadata + metadataProject = project; + } + + Document metadataDocument = null; + if (!_metadataDocumentCache.TryGetValue(fileName, out metadataDocument)) + { + var topLevelSymbol = symbol.GetTopLevelContainingNamedType(); + + var temporaryDocument = metadataProject.AddDocument(fileName, string.Empty); + var service = _csharpMetadataAsSourceService.CreateInstance(temporaryDocument.Project.LanguageServices); + var method = _csharpMetadataAsSourceService.GetMethod(AddSourceToAsync); + + var documentTask = method.Invoke>(service, new object[] { temporaryDocument, topLevelSymbol, default(CancellationToken) }); + metadataDocument = await documentTask; - var temporaryDocument = metadataProject.AddDocument(filePath, string.Empty); - var service = _csharpMetadataAsSourceService.CreateInstance(temporaryDocument.Project.LanguageServices); - var method = _csharpMetadataAsSourceService.GetMethod(AddSourceToAsync); + _metadataDocumentCache[fileName] = metadataDocument; + } - return method.Invoke>(service, new object[] { temporaryDocument, topLevelSymbol, cancellationToken }); + return new MetadataDocumentResult(metadataDocument, fileName); } public async Task GetSymbolLocationFromMetadata(ISymbol symbol, Document metadataDocument, CancellationToken cancellationToken = new CancellationToken()) @@ -83,7 +113,7 @@ public string GetFilePathForSymbol(Project project, ISymbol symbol) return await _getLocationInGeneratedSourceAsync.InvokeStatic>(new object[] { symboldId, metadataDocument, cancellationToken }); } - private string GetTypeDisplayString(INamedTypeSymbol symbol) + private static string GetTypeDisplayString(INamedTypeSymbol symbol) { if (symbol.SpecialType != SpecialType.None) { @@ -116,22 +146,12 @@ private string GetTypeDisplayString(INamedTypeSymbol symbol) return symbol.ToDisplayString(); } - private string Folderize(string path) + private static string GetFilePathForSymbol(Project project, ISymbol symbol) { - return string.Join("/", path.Split('.')); + var topLevelSymbol = symbol.GetTopLevelContainingNamedType(); + return $"$metadata$/Project/{Folderize(project.Name)}/Assembly/{Folderize(topLevelSymbol.ContainingAssembly.Name)}/Symbol/{Folderize(GetTypeDisplayString(topLevelSymbol))}.cs".Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); } - private INamedTypeSymbol GetTopLevelContainingNamedType(ISymbol symbol) - { - // Traverse up until we find a named type that is parented by the namespace - var topLevelNamedType = symbol; - while (topLevelNamedType.ContainingSymbol != symbol.ContainingNamespace || - topLevelNamedType.Kind != SymbolKind.NamedType) - { - topLevelNamedType = topLevelNamedType.ContainingSymbol; - } - - return (INamedTypeSymbol)topLevelNamedType; - } + private static string Folderize(string path) => string.Join("/", path.Split('.')); } } diff --git a/src/OmniSharp.Roslyn/OmniSharpWorkspace.cs b/src/OmniSharp.Roslyn/OmniSharpWorkspace.cs index e8140892d0..66d3b9987c 100644 --- a/src/OmniSharp.Roslyn/OmniSharpWorkspace.cs +++ b/src/OmniSharp.Roslyn/OmniSharpWorkspace.cs @@ -127,6 +127,8 @@ public IEnumerable GetDocuments(string filePath) public Document GetDocument(string filePath) { + if (filePath == null) return null; + var documentId = GetDocumentId(filePath); if (documentId == null) { diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index 689960c4ec..03531699a7 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -6,6 +6,8 @@ using TestUtility; using Xunit; using Xunit.Abstractions; +using OmniSharp.Models.GotoDefinition; +using OmniSharp.Models.Metadata; namespace OmniSharp.Roslyn.CSharp.Tests { @@ -228,6 +230,62 @@ public Foo Clone() { Assert.Equal("a.cs", usages.QuickFixes.ElementAt(0).FileName); } + [Fact] + public async Task CanFindReferencesFromMetadata() + { + const string code = @" + using System; + public class Foo + { + public Gu$$id Id {get;set;} + } + "; + + var file = new TestFile("a.cs", code); + + using (var host = CreateOmniSharpHost(file)) + { + var point = file.Content.GetPointFromPosition(); + + var goToDefinionRequest = new GotoDefinitionRequest + { + FileName = file.FileName, + Line = point.Line, + Column = point.Offset, + Timeout = 60000, + WantMetadata = true + }; + + var goToDefinitionHandler = host.GetRequestHandler(OmniSharpEndpoints.GotoDefinition); + var goToDefinionResponse = await goToDefinitionHandler.Handle(goToDefinionRequest); + + var metadataRequestHandler = host.GetRequestHandler(OmniSharpEndpoints.Metadata); + var metadataResponse = await metadataRequestHandler.Handle(new MetadataRequest + { + AssemblyName = goToDefinionResponse.MetadataSource.AssemblyName, + TypeName = goToDefinionResponse.MetadataSource.TypeName, + ProjectName = goToDefinionResponse.MetadataSource.ProjectName, + Language = goToDefinionResponse.MetadataSource.Language + }); + + var requestHandler = GetRequestHandler(host); + + var request = new FindUsagesRequest + { + Line = goToDefinionResponse.Line, + Column = goToDefinionResponse.Column, + FileName = metadataResponse.SourceName, + OnlyThisFile = false, + ExcludeDefinition = true + }; + + var usages = await requestHandler.Handle(request); + + Assert.Equal(1, usages.QuickFixes.Count()); + Assert.Equal("a.cs", usages.QuickFixes.ElementAt(0).FileName); + } + } + private Task FindUsagesAsync(string code, bool excludeDefinition = false) { return FindUsagesAsync(new[] { new TestFile("dummy.cs", code) }, false, excludeDefinition); diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs index 56ff8db7b7..95ce6ba47a 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs @@ -1,10 +1,13 @@ using System.Linq; using System.Threading.Tasks; -using OmniSharp.Models.GotoDefinition; -using OmniSharp.Roslyn.CSharp.Services.Navigation; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; using TestUtility; using Xunit; using Xunit.Abstractions; +using OmniSharp.Models.GotoDefinition; +using OmniSharp.Roslyn.CSharp.Services.Navigation; +using OmniSharp.Models.Metadata; namespace OmniSharp.Roslyn.CSharp.Tests { @@ -159,6 +162,77 @@ await TestGoToMetadataAsync(testFile, expectedTypeName: "System.String"); } + [Theory] + [InlineData("bar.cs")] + [InlineData("bar.csx")] + public async Task ReturnsDefinitionInMetadata_FromMetadata_WhenSymbolIsType(string filename) + { + 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 request = new GotoDefinitionRequest + { + FileName = testFile.FileName, + Line = point.Line, + Column = point.Offset, + Timeout = 60000, + WantMetadata = true + }; + var requestHandler = GetRequestHandler(host); + var response = await requestHandler.Handle(request); + + // 2. now, based on the response information + // go to the metadata endpoint, and ask for "int" specific metadata + var metadataRequestHandler = host.GetRequestHandler(OmniSharpEndpoints.Metadata); + var metadataResponse = await metadataRequestHandler.Handle(new MetadataRequest + { + AssemblyName = response.MetadataSource.AssemblyName, + TypeName = response.MetadataSource.TypeName, + ProjectName = response.MetadataSource.ProjectName, + Language = response.MetadataSource.Language + }); + + // 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(). + First().BaseList.Types.FirstOrDefault(x => x.Type.ToString() == "IComparable"); + + + // 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 relevantLineSpan = iComparable.GetLocation().GetLineSpan(); + var metadataNavigationRequest = new GotoDefinitionRequest + { + FileName = metadataResponse.SourceName, + Line = relevantLineSpan.StartLinePosition.Line, + Column = relevantLineSpan.StartLinePosition.Character, + Timeout = 60000, + WantMetadata = true + }; + + var metadataNavigationResponse = await requestHandler.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); From 2a8be676404039b22708e22b855f47325b55e7c5 Mon Sep 17 00:00:00 2001 From: filipw Date: Tue, 6 Jun 2017 15:54:43 +0200 Subject: [PATCH 2/4] addressed PR review feedback --- src/OmniSharp.Abstractions/Models/Request.cs | 5 -- .../Navigation/GotoDefinitionService.cs | 9 ++- .../Services/Navigation/MetadataService.cs | 8 +-- .../MetadataDocumentResult.cs | 17 ------ src/OmniSharp.Roslyn/MetadataHelper.cs | 13 ++--- src/OmniSharp.Roslyn/OmniSharpWorkspace.cs | 2 +- .../FindReferencesFacts.cs | 58 ------------------- .../GoToDefinitionFacts.cs | 17 +++--- 8 files changed, 23 insertions(+), 106 deletions(-) delete mode 100644 src/OmniSharp.Roslyn/MetadataDocumentResult.cs diff --git a/src/OmniSharp.Abstractions/Models/Request.cs b/src/OmniSharp.Abstractions/Models/Request.cs index a18752864f..6ed7975cb2 100644 --- a/src/OmniSharp.Abstractions/Models/Request.cs +++ b/src/OmniSharp.Abstractions/Models/Request.cs @@ -26,10 +26,5 @@ public string FileName _fileName = value; } } - - public bool IsMetadataFile() - { - return FileName != null && FileName.StartsWith("$metadata$", StringComparison.OrdinalIgnoreCase); - } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs index 14632cb81a..f5020fa393 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/GotoDefinitionService.cs @@ -31,8 +31,7 @@ public async Task Handle(GotoDefinitionRequest request) { var quickFixes = new List(); - var document = request.IsMetadataFile() ? - _metadataHelper.FindDocumentInMetadataCache(request.FileName) : + var document = _metadataHelper.FindDocumentInMetadataCache(request.FileName) ?? _workspace.GetDocument(request.FileName); var response = new GotoDefinitionResponse(); @@ -68,12 +67,12 @@ public async Task Handle(GotoDefinitionRequest request) else if (location.IsInMetadata && request.WantMetadata) { var cancellationSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(request.Timeout)); - var metadataDocumentResult = await _metadataHelper.GetAndAddDocumentFromMetadata(document.Project, symbol, cancellationSource.Token); - if (metadataDocumentResult.Document != null) + var (metadataDocument, _) = await _metadataHelper.GetAndAddDocumentFromMetadata(document.Project, symbol, cancellationSource.Token); + if (metadataDocument != null) { cancellationSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(request.Timeout)); - var metadataLocation = await _metadataHelper.GetSymbolLocationFromMetadata(symbol, metadataDocumentResult.Document, cancellationSource.Token); + var metadataLocation = await _metadataHelper.GetSymbolLocationFromMetadata(symbol, metadataDocument, cancellationSource.Token); var lineSpan = metadataLocation.GetMappedLineSpan(); response = new GotoDefinitionResponse diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/MetadataService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/MetadataService.cs index 45d7866498..3fe0338978 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/MetadataService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/MetadataService.cs @@ -31,12 +31,12 @@ public async Task Handle(MetadataRequest request) if (symbol != null && symbol.ContainingAssembly.Name == request.AssemblyName) { var cancellationSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(request.Timeout)); - var metadataDocumentResult = await _metadataHelper.GetAndAddDocumentFromMetadata(project, symbol, cancellationSource.Token); - if (metadataDocumentResult.Document != null) + var (metadataDocument, documentPath) = await _metadataHelper.GetAndAddDocumentFromMetadata(project, symbol, cancellationSource.Token); + if (metadataDocument != null) { - var source = await metadataDocumentResult.Document.GetTextAsync(); + var source = await metadataDocument.GetTextAsync(); response.Source = source.ToString(); - response.SourceName = metadataDocumentResult.DocumentPath; + response.SourceName = documentPath; return response; } diff --git a/src/OmniSharp.Roslyn/MetadataDocumentResult.cs b/src/OmniSharp.Roslyn/MetadataDocumentResult.cs deleted file mode 100644 index e015642d76..0000000000 --- a/src/OmniSharp.Roslyn/MetadataDocumentResult.cs +++ /dev/null @@ -1,17 +0,0 @@ -using Microsoft.CodeAnalysis; - -namespace OmniSharp.Roslyn -{ - public class MetadataDocumentResult - { - public MetadataDocumentResult(Document document, string documentPath) - { - Document = document; - DocumentPath = documentPath; - } - - public Document Document { get; } - - public string DocumentPath { get; } - } -} diff --git a/src/OmniSharp.Roslyn/MetadataHelper.cs b/src/OmniSharp.Roslyn/MetadataHelper.cs index 41ba5f81a7..c0143826d1 100644 --- a/src/OmniSharp.Roslyn/MetadataHelper.cs +++ b/src/OmniSharp.Roslyn/MetadataHelper.cs @@ -63,11 +63,11 @@ public string GetSymbolName(ISymbol symbol) return GetTypeDisplayString(topLevelSymbol); } - public async Task GetAndAddDocumentFromMetadata(Project project, ISymbol symbol, CancellationToken cancellationToken = new CancellationToken()) + public async Task<(Document metadataDocument, string documentPath)> GetAndAddDocumentFromMetadata(Project project, ISymbol symbol, CancellationToken cancellationToken = new CancellationToken()) { var fileName = GetFilePathForSymbol(project, symbol); - Project metadataProject = null; + Project metadataProject; // since submission projects cannot have new documents added to it // we will use a separate project to hold metadata documents @@ -77,8 +77,8 @@ public string GetSymbolName(ISymbol symbol) if (metadataProject == null) { metadataProject = project.Solution.AddProject(MetadataKey, $"{MetadataKey}.dll", LanguageNames.CSharp) - .WithCompilationOptions(project.CompilationOptions) - .WithMetadataReferences(project.MetadataReferences); + .WithCompilationOptions(project.CompilationOptions) + .WithMetadataReferences(project.MetadataReferences); } } else @@ -87,8 +87,7 @@ public string GetSymbolName(ISymbol symbol) metadataProject = project; } - Document metadataDocument = null; - if (!_metadataDocumentCache.TryGetValue(fileName, out metadataDocument)) + if (!_metadataDocumentCache.TryGetValue(fileName, out var metadataDocument)) { var topLevelSymbol = symbol.GetTopLevelContainingNamedType(); @@ -102,7 +101,7 @@ public string GetSymbolName(ISymbol symbol) _metadataDocumentCache[fileName] = metadataDocument; } - return new MetadataDocumentResult(metadataDocument, fileName); + return (metadataDocument, fileName); } public async Task GetSymbolLocationFromMetadata(ISymbol symbol, Document metadataDocument, CancellationToken cancellationToken = new CancellationToken()) diff --git a/src/OmniSharp.Roslyn/OmniSharpWorkspace.cs b/src/OmniSharp.Roslyn/OmniSharpWorkspace.cs index 66d3b9987c..89ca4a75c6 100644 --- a/src/OmniSharp.Roslyn/OmniSharpWorkspace.cs +++ b/src/OmniSharp.Roslyn/OmniSharpWorkspace.cs @@ -127,7 +127,7 @@ public IEnumerable GetDocuments(string filePath) public Document GetDocument(string filePath) { - if (filePath == null) return null; + if (string.IsNullOrWhiteSpace(filePath)) return null; var documentId = GetDocumentId(filePath); if (documentId == null) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index 03531699a7..689960c4ec 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -6,8 +6,6 @@ using TestUtility; using Xunit; using Xunit.Abstractions; -using OmniSharp.Models.GotoDefinition; -using OmniSharp.Models.Metadata; namespace OmniSharp.Roslyn.CSharp.Tests { @@ -230,62 +228,6 @@ public Foo Clone() { Assert.Equal("a.cs", usages.QuickFixes.ElementAt(0).FileName); } - [Fact] - public async Task CanFindReferencesFromMetadata() - { - const string code = @" - using System; - public class Foo - { - public Gu$$id Id {get;set;} - } - "; - - var file = new TestFile("a.cs", code); - - using (var host = CreateOmniSharpHost(file)) - { - var point = file.Content.GetPointFromPosition(); - - var goToDefinionRequest = new GotoDefinitionRequest - { - FileName = file.FileName, - Line = point.Line, - Column = point.Offset, - Timeout = 60000, - WantMetadata = true - }; - - var goToDefinitionHandler = host.GetRequestHandler(OmniSharpEndpoints.GotoDefinition); - var goToDefinionResponse = await goToDefinitionHandler.Handle(goToDefinionRequest); - - var metadataRequestHandler = host.GetRequestHandler(OmniSharpEndpoints.Metadata); - var metadataResponse = await metadataRequestHandler.Handle(new MetadataRequest - { - AssemblyName = goToDefinionResponse.MetadataSource.AssemblyName, - TypeName = goToDefinionResponse.MetadataSource.TypeName, - ProjectName = goToDefinionResponse.MetadataSource.ProjectName, - Language = goToDefinionResponse.MetadataSource.Language - }); - - var requestHandler = GetRequestHandler(host); - - var request = new FindUsagesRequest - { - Line = goToDefinionResponse.Line, - Column = goToDefinionResponse.Column, - FileName = metadataResponse.SourceName, - OnlyThisFile = false, - ExcludeDefinition = true - }; - - var usages = await requestHandler.Handle(request); - - Assert.Equal(1, usages.QuickFixes.Count()); - Assert.Equal("a.cs", usages.QuickFixes.ElementAt(0).FileName); - } - } - private Task FindUsagesAsync(string code, bool excludeDefinition = false) { return FindUsagesAsync(new[] { new TestFile("dummy.cs", code) }, false, excludeDefinition); diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs index 95ce6ba47a..58b3a41d35 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs @@ -2,12 +2,12 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; -using TestUtility; -using Xunit; -using Xunit.Abstractions; using OmniSharp.Models.GotoDefinition; using OmniSharp.Roslyn.CSharp.Services.Navigation; using OmniSharp.Models.Metadata; +using TestUtility; +using Xunit; +using Xunit.Abstractions; namespace OmniSharp.Roslyn.CSharp.Tests { @@ -193,25 +193,25 @@ public void Baz() { // 2. now, based on the response information // go to the metadata endpoint, and ask for "int" specific metadata - var metadataRequestHandler = host.GetRequestHandler(OmniSharpEndpoints.Metadata); - var metadataResponse = await metadataRequestHandler.Handle(new MetadataRequest + var metadataRequest = new MetadataRequest { AssemblyName = response.MetadataSource.AssemblyName, TypeName = response.MetadataSource.TypeName, ProjectName = response.MetadataSource.ProjectName, Language = response.MetadataSource.Language - }); + }; + var metadataRequestHandler = host.GetRequestHandler(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(). 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 relevantLineSpan = iComparable.GetLocation().GetLineSpan(); var metadataNavigationRequest = new GotoDefinitionRequest { FileName = metadataResponse.SourceName, @@ -220,7 +220,6 @@ public void Baz() { Timeout = 60000, WantMetadata = true }; - var metadataNavigationResponse = await requestHandler.Handle(metadataNavigationRequest); // 5. validate the response to be matching the expected IComparable meta info From f57c98c40050d2f1388122f7cefa32c6205c8bc2 Mon Sep 17 00:00:00 2001 From: Filip W Date: Tue, 6 Jun 2017 15:57:41 +0200 Subject: [PATCH 3/4] removed unnecessary using --- src/OmniSharp.Abstractions/Models/Request.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OmniSharp.Abstractions/Models/Request.cs b/src/OmniSharp.Abstractions/Models/Request.cs index 6ed7975cb2..96663c5626 100644 --- a/src/OmniSharp.Abstractions/Models/Request.cs +++ b/src/OmniSharp.Abstractions/Models/Request.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; using System.IO; using Newtonsoft.Json; From c317826995161c4f65c015e30b19992c0bb66007 Mon Sep 17 00:00:00 2001 From: filipw Date: Tue, 6 Jun 2017 16:00:01 +0200 Subject: [PATCH 4/4] better naming and formatting in tests --- .../GoToDefinitionFacts.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs index 58b3a41d35..24847a8e15 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/GoToDefinitionFacts.cs @@ -180,25 +180,24 @@ public void Baz() { var point = testFile.Content.GetPointFromPosition(); // 1. start by asking for definition of "int" - var request = new GotoDefinitionRequest + var gotoDefinitionRequest = new GotoDefinitionRequest { FileName = testFile.FileName, Line = point.Line, Column = point.Offset, - Timeout = 60000, WantMetadata = true }; - var requestHandler = GetRequestHandler(host); - var response = await requestHandler.Handle(request); + 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 = response.MetadataSource.AssemblyName, - TypeName = response.MetadataSource.TypeName, - ProjectName = response.MetadataSource.ProjectName, - Language = response.MetadataSource.Language + AssemblyName = gotoDefinitionResponse.MetadataSource.AssemblyName, + TypeName = gotoDefinitionResponse.MetadataSource.TypeName, + ProjectName = gotoDefinitionResponse.MetadataSource.ProjectName, + Language = gotoDefinitionResponse.MetadataSource.Language }; var metadataRequestHandler = host.GetRequestHandler(OmniSharpEndpoints.Metadata); var metadataResponse = await metadataRequestHandler.Handle(metadataRequest); @@ -206,8 +205,10 @@ public void Baz() { // 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(). - First().BaseList.Types.FirstOrDefault(x => x.Type.ToString() == "IComparable"); + var iComparable = metadataTree.GetCompilationUnitRoot(). + DescendantNodesAndSelf(). + OfType().First(). + BaseList.Types.FirstOrDefault(x => x.Type.ToString() == "IComparable"); var relevantLineSpan = iComparable.GetLocation().GetLineSpan(); // 4. now ask for the definition of "IComparable" @@ -217,10 +218,9 @@ public void Baz() { FileName = metadataResponse.SourceName, Line = relevantLineSpan.StartLinePosition.Line, Column = relevantLineSpan.StartLinePosition.Character, - Timeout = 60000, WantMetadata = true }; - var metadataNavigationResponse = await requestHandler.Handle(metadataNavigationRequest); + var metadataNavigationResponse = await gotoDefinitionRequestHandler.Handle(metadataNavigationRequest); // 5. validate the response to be matching the expected IComparable meta info Assert.NotNull(metadataNavigationResponse.MetadataSource);