From 135744e7cacedf7c358639be3340bf44c86dce29 Mon Sep 17 00:00:00 2001 From: Ivan Basov Date: Mon, 21 Oct 2019 12:43:19 -0500 Subject: [PATCH] follow up for Go to base: support metadata references and bug fixes (#39334) --- .../Core/GoToBase/AbstractGoToBaseService.cs | 7 +++--- .../Test2/GoToBase/VisuaBasicGoToBaseTests.vb | 2 +- .../Test2/GoToHelpers/GoToHelpers.vb | 24 +++++++------------ .../Entries/AbstractItemEntry.cs | 6 ++--- .../Entries/DefinitionItemEntry.cs | 2 +- .../Entries/DocumentSpanEntry.cs | 10 ++++---- .../Entries/MetadataDefinitionItemEntry.cs | 4 ++-- .../FindReferences/BaseTypeFinder.cs | 8 +++++++ 8 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/EditorFeatures/Core/GoToBase/AbstractGoToBaseService.cs b/src/EditorFeatures/Core/GoToBase/AbstractGoToBaseService.cs index 24d63f9691d..2514a037ecf 100644 --- a/src/EditorFeatures/Core/GoToBase/AbstractGoToBaseService.cs +++ b/src/EditorFeatures/Core/GoToBase/AbstractGoToBaseService.cs @@ -38,14 +38,13 @@ public async Task FindBasesAsync(Document document, int position, IFindUsagesCon var found = false; // For each potential base, try to find its definition in sources. - // If found, add its' definitionItem to the context. - // If not found but the symbol is from metadata, create its' definition item from metadata and add to the context. + // If found, add it's definitionItem to the context. + // If not found but the symbol is from metadata, create it's definition item from metadata and add to the context. foreach (var baseSymbol in bases) { var sourceDefinition = await SymbolFinder.FindSourceDefinitionAsync( SymbolAndProjectId.Create(baseSymbol, projectId), solution, cancellationToken).ConfigureAwait(false); - if (sourceDefinition.Symbol != null && - sourceDefinition.Symbol.Locations.Any(l => l.IsInSource)) + if (sourceDefinition.Symbol != null) { var definitionItem = await sourceDefinition.Symbol.ToClassifiedDefinitionItemAsync( solution.GetProject(sourceDefinition.ProjectId), includeHiddenLocations: false, diff --git a/src/EditorFeatures/Test2/GoToBase/VisuaBasicGoToBaseTests.vb b/src/EditorFeatures/Test2/GoToBase/VisuaBasicGoToBaseTests.vb index ff0d8d61b72..e036322111a 100644 --- a/src/EditorFeatures/Test2/GoToBase/VisuaBasicGoToBaseTests.vb +++ b/src/EditorFeatures/Test2/GoToBase/VisuaBasicGoToBaseTests.vb @@ -420,7 +420,7 @@ End Interface") Public Async Function TestWithVirtualMethodHiddenAndInterfaceImplementedOnDerivedType() As Task ' We should not find hidden methods. - ' We should not find methods of interfaces no implemented by the method symbol. + ' We should not find methods of interfaces not implemented by the method symbol. ' In this example, ' Dim i As I = New D() ' i.M() diff --git a/src/EditorFeatures/Test2/GoToHelpers/GoToHelpers.vb b/src/EditorFeatures/Test2/GoToHelpers/GoToHelpers.vb index 5b55740298c..f0460312078 100644 --- a/src/EditorFeatures/Test2/GoToHelpers/GoToHelpers.vb +++ b/src/EditorFeatures/Test2/GoToHelpers/GoToHelpers.vb @@ -45,13 +45,13 @@ Friend Class GoToHelpers $"Expected: ({expected}) but got: ({actual})") Next - Dim actualDefintionsWithoutSpans = context.GetDefinitions(). - Where(Function(d) d.SourceSpans.IsDefaultOrEmpty). - Select(Function(di) - Return String.Format("{0}:{1}", - String.Join("", di.OriginationParts.Select(Function(t) t.Text)), - String.Join("", di.NameDisplayParts.Select(Function(t) t.Text))) - End Function).ToList() + Dim actualDefintionsWithoutSpans = context.GetDefinitions() _ + .Where(Function(d) d.SourceSpans.IsDefaultOrEmpty) _ + .Select(Function(di) + Return String.Format("{0}:{1}", + String.Join("", di.OriginationParts.Select(Function(t) t.Text)), + String.Join("", di.NameDisplayParts.Select(Function(t) t.Text))) + End Function).ToList() actualDefintionsWithoutSpans.Sort() @@ -59,15 +59,7 @@ Friend Class GoToHelpers metadataDefinitions = {} End If - Assert.Equal(actualDefintionsWithoutSpans.Count, metadataDefinitions.Count) - - For i = 0 To actualDefintionsWithoutSpans.Count - 1 - Dim actual = actualDefintionsWithoutSpans(i) - Dim expected = metadataDefinitions(i) - - Assert.True(actual.CompareTo(expected) = 0, - $"Expected: ({expected}) but got: ({actual})") - Next + Assert.Equal(actualDefintionsWithoutSpans, metadataDefinitions) End If End Using End Function diff --git a/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/AbstractItemEntry.cs b/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/AbstractItemEntry.cs index 36a5ab0df76..5cddccaa6ee 100644 --- a/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/AbstractItemEntry.cs +++ b/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/AbstractItemEntry.cs @@ -12,12 +12,12 @@ internal partial class StreamingFindUsagesPresenter { private abstract class AbstractItemEntry : Entry { - protected readonly StreamingFindUsagesPresenter _presenter; + protected readonly StreamingFindUsagesPresenter Presenter; public AbstractItemEntry(RoslynDefinitionBucket definitionBucket, StreamingFindUsagesPresenter presenter) : base(definitionBucket) { - _presenter = presenter; + Presenter = presenter; } public override bool TryCreateColumnContent(string columnName, out FrameworkElement content) @@ -25,7 +25,7 @@ public override bool TryCreateColumnContent(string columnName, out FrameworkElem if (columnName == StandardTableColumnDefinitions2.LineText) { var inlines = CreateLineTextInlines(); - var textBlock = inlines.ToTextBlock(_presenter.ClassificationFormatMap, wrap: false); + var textBlock = inlines.ToTextBlock(Presenter.ClassificationFormatMap, wrap: false); content = textBlock; return true; diff --git a/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/DefinitionItemEntry.cs b/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/DefinitionItemEntry.cs index 43cd49fd151..2e2f61a8b41 100644 --- a/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/DefinitionItemEntry.cs +++ b/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/DefinitionItemEntry.cs @@ -30,7 +30,7 @@ private class DefinitionItemEntry : AbstractDocumentSpanEntry } protected override IList CreateLineTextInlines() - => DefinitionBucket.DefinitionItem.DisplayParts.ToInlines(_presenter.ClassificationFormatMap, _presenter.TypeMap); + => DefinitionBucket.DefinitionItem.DisplayParts.ToInlines(Presenter.ClassificationFormatMap, Presenter.TypeMap); } } } diff --git a/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/DocumentSpanEntry.cs b/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/DocumentSpanEntry.cs index 12d60bcf4ff..f47b4af2f9f 100644 --- a/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/DocumentSpanEntry.cs +++ b/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/DocumentSpanEntry.cs @@ -69,7 +69,7 @@ protected override IList CreateLineTextInlines( ? WrittenReferenceHighlightTag.TagId : ReferenceHighlightTag.TagId; - var properties = _presenter.FormatMapService + var properties = Presenter.FormatMapService .GetEditorFormatMap("text") .GetProperties(propertyId); @@ -83,8 +83,8 @@ protected override IList CreateLineTextInlines( cs => new ClassifiedText(cs.ClassificationType, _excerptResult.Content.ToString(cs.TextSpan))); var inlines = classifiedTexts.ToInlines( - _presenter.ClassificationFormatMap, - _presenter.TypeMap, + Presenter.ClassificationFormatMap, + Presenter.TypeMap, runCallback: (run, classifiedText, position) => { if (properties["Background"] is Brush highlightBrush) @@ -136,7 +136,7 @@ protected override object GetValueWorker(string keyName) private DisposableToolTip CreateDisposableToolTip(Document document, TextSpan sourceSpan) { - _presenter.AssertIsForeground(); + Presenter.AssertIsForeground(); var controlService = document.Project.Solution.Workspace.Services.GetService(); var sourceText = document.GetTextSynchronously(CancellationToken.None); @@ -144,7 +144,7 @@ private DisposableToolTip CreateDisposableToolTip(Document document, TextSpan so var excerptService = document.Services.GetService(); if (excerptService != null) { - var excerpt = _presenter.ThreadingContext.JoinableTaskFactory.Run(() => excerptService.TryExcerptAsync(document, sourceSpan, ExcerptMode.Tooltip, CancellationToken.None)); + var excerpt = Presenter.ThreadingContext.JoinableTaskFactory.Run(() => excerptService.TryExcerptAsync(document, sourceSpan, ExcerptMode.Tooltip, CancellationToken.None)); if (excerpt != null) { // get tooltip from excerpt service diff --git a/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/MetadataDefinitionItemEntry.cs b/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/MetadataDefinitionItemEntry.cs index 8983624afb8..f32b5a56b28 100644 --- a/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/MetadataDefinitionItemEntry.cs +++ b/src/VisualStudio/Core/Def/Implementation/FindReferences/Entries/MetadataDefinitionItemEntry.cs @@ -31,11 +31,11 @@ protected override object GetValueWorker(string keyName) } bool ISupportsNavigation.TryNavigateTo(bool isPreview) - => DefinitionBucket.DefinitionItem.TryNavigateTo(_presenter._workspace, isPreview); + => DefinitionBucket.DefinitionItem.TryNavigateTo(Presenter._workspace, isPreview); protected override IList CreateLineTextInlines() => DefinitionBucket.DefinitionItem.DisplayParts - .ToInlines(_presenter.ClassificationFormatMap, _presenter.TypeMap); + .ToInlines(Presenter.ClassificationFormatMap, Presenter.TypeMap); } } } diff --git a/src/Workspaces/Core/Portable/FindSymbols/FindReferences/BaseTypeFinder.cs b/src/Workspaces/Core/Portable/FindSymbols/FindReferences/BaseTypeFinder.cs index b562a27ed30..7fc51f7eed0 100644 --- a/src/Workspaces/Core/Portable/FindSymbols/FindReferences/BaseTypeFinder.cs +++ b/src/Workspaces/Core/Portable/FindSymbols/FindReferences/BaseTypeFinder.cs @@ -35,16 +35,24 @@ public static ImmutableArray FindBaseTypesAndInterfaces(INamedTypeSymbo // We should add implementations only for overridden members but not for hidden ones. // In the following example: + // // interface I { void M(); } // class A : I { public void M(); } // class B : A { public new void M(); } + // // we should not find anything for B.M() because it does not implement the interface: + // // I i = new B(); i.M(); + // // will call the method from A. // However, if we change the code to + // // class B : A, I { public new void M(); } + // // then + // // I i = new B(); i.M(); + // // will call the method from B. We should find the base for B.M in this case. // And if we change 'new' to 'override' in the original code and add 'virtual' where needed, // we should find I.M as a base for B.M(). And the next line helps with this scenario. -- GitLab