From 69bdfb29ce4d027c5f5bf70f50be9bebcb5df1aa Mon Sep 17 00:00:00 2001 From: Shen Chen Date: Tue, 21 Jun 2022 15:01:22 -0700 Subject: [PATCH] Filter symbol first before creating InheritanceMargin target (#61911) * Move IsNavigableSymbol check into ToSlimDefinitionItem * Include hidden locations * Update the test * Rename * Update the test not to include hidden places * Use null instead * Simplify code * Fix format * Fix format * Address feedback --- .../InheritanceMarginTests.cs | 17 +++++ ...bstractInheritanceMarginService_Helpers.cs | 63 +++++++++++-------- .../InheritanceMarginItem.cs | 6 +- 3 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/EditorFeatures/Test/InheritanceMargin/InheritanceMarginTests.cs b/src/EditorFeatures/Test/InheritanceMargin/InheritanceMarginTests.cs index 24264ba621bce..47b199c74365b 100644 --- a/src/EditorFeatures/Test/InheritanceMargin/InheritanceMarginTests.cs +++ b/src/EditorFeatures/Test/InheritanceMargin/InheritanceMarginTests.cs @@ -124,6 +124,7 @@ private static async Task VerifyTestMemberInDocumentAsync( private static async Task VerifyInheritanceMemberAsync(TestWorkspace testWorkspace, TestInheritanceMemberItem expectedItem, InheritanceMarginItem actualItem) { + Assert.True(!actualItem.TargetItems.IsEmpty); Assert.Equal(expectedItem.LineNumber, actualItem.LineNumber); Assert.Equal(expectedItem.MemberName, actualItem.DisplayTexts.JoinText()); Assert.Equal(expectedItem.Targets.Length, actualItem.TargetItems.Length); @@ -2279,5 +2280,21 @@ public class {|target3:Bar|} new[] { itemForIBar, itemForBarInMarkup2 }, testHost); } + + [Theory, CombinatorialData] + public async Task TestHiddenLocationSymbol(TestHost testHost) + { + await VerifyNoItemForDocumentAsync(@" +public class {|target2:B|} : C +{ +} + +#line hidden +public class {|target1:C|} +{ +}", + LanguageNames.CSharp, + testHost); + } } } diff --git a/src/Features/Core/Portable/InheritanceMargin/AbstractInheritanceMarginService_Helpers.cs b/src/Features/Core/Portable/InheritanceMargin/AbstractInheritanceMarginService_Helpers.cs index 3a11d5fb393e9..18098d6079267 100644 --- a/src/Features/Core/Portable/InheritanceMargin/AbstractInheritanceMarginService_Helpers.cs +++ b/src/Features/Core/Portable/InheritanceMargin/AbstractInheritanceMarginService_Helpers.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using System.IO; @@ -382,7 +383,7 @@ private static async ValueTask AddInheritanceMemberItemsForMembersAsync( } } - private static async ValueTask CreateInheritanceMemberItemForInterfaceAsync( + private static async ValueTask CreateInheritanceMemberItemForInterfaceAsync( Solution solution, INamedTypeSymbol interfaceSymbol, int lineNumber, @@ -392,7 +393,6 @@ private static async ValueTask CreateInheritanceMemberIte { var baseSymbolItems = await baseSymbols .SelectAsArray(symbol => symbol.OriginalDefinition) - .WhereAsArray(IsNavigableSymbol) .Distinct() .SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync( solution, @@ -403,7 +403,6 @@ private static async ValueTask CreateInheritanceMemberIte var derivedTypeItems = await derivedTypesSymbols .SelectAsArray(symbol => symbol.OriginalDefinition) - .WhereAsArray(IsNavigableSymbol) .Distinct() .SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync(solution, symbol, @@ -411,15 +410,18 @@ private static async ValueTask CreateInheritanceMemberIte cancellationToken), cancellationToken) .ConfigureAwait(false); + var nonNullBaseSymbolItems = GetNonNullTargetItems(baseSymbolItems); + var nonNullDerivedTypeItems = GetNonNullTargetItems(derivedTypeItems); + return InheritanceMarginItem.CreateOrdered( lineNumber, topLevelDisplayText: null, FindUsagesHelpers.GetDisplayParts(interfaceSymbol), interfaceSymbol.GetGlyph(), - baseSymbolItems.Concat(derivedTypeItems)); + nonNullBaseSymbolItems.Concat(nonNullDerivedTypeItems)); } - private static async ValueTask CreateInheritanceMemberItemForInterfaceMemberAsync( + private static async ValueTask CreateInheritanceMemberItemForInterfaceMemberAsync( Solution solution, ISymbol memberSymbol, int lineNumber, @@ -428,7 +430,6 @@ private static async ValueTask CreateInheritanceMemberIte { var implementedMemberItems = await implementingMembers .SelectAsArray(symbol => symbol.OriginalDefinition) - .WhereAsArray(IsNavigableSymbol) .Distinct() .SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync( solution, @@ -436,15 +437,16 @@ private static async ValueTask CreateInheritanceMemberIte InheritanceRelationship.ImplementingMember, cancellationToken), cancellationToken).ConfigureAwait(false); + var nonNullImplementedMemberItems = GetNonNullTargetItems(implementedMemberItems); return InheritanceMarginItem.CreateOrdered( lineNumber, topLevelDisplayText: null, FindUsagesHelpers.GetDisplayParts(memberSymbol), memberSymbol.GetGlyph(), - implementedMemberItems); + nonNullImplementedMemberItems); } - private static async ValueTask CreateInheritanceItemForClassAndStructureAsync( + private static async ValueTask CreateInheritanceItemForClassAndStructureAsync( Solution solution, INamedTypeSymbol memberSymbol, int lineNumber, @@ -456,7 +458,6 @@ private static async ValueTask CreateInheritanceItemForCl // and if it is an class/struct, it whould be shown as 'Base Type' var baseSymbolItems = await baseSymbols .SelectAsArray(symbol => symbol.OriginalDefinition) - .WhereAsArray(IsNavigableSymbol) .Distinct() .SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync( solution, @@ -466,7 +467,6 @@ private static async ValueTask CreateInheritanceItemForCl var derivedTypeItems = await derivedTypesSymbols .SelectAsArray(symbol => symbol.OriginalDefinition) - .WhereAsArray(IsNavigableSymbol) .Distinct() .SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync(solution, symbol, @@ -474,15 +474,18 @@ private static async ValueTask CreateInheritanceItemForCl cancellationToken), cancellationToken) .ConfigureAwait(false); + var nonNullBaseSymbolItems = GetNonNullTargetItems(baseSymbolItems); + var nonNullDerivedTypeItems = GetNonNullTargetItems(derivedTypeItems); + return InheritanceMarginItem.CreateOrdered( lineNumber, topLevelDisplayText: null, FindUsagesHelpers.GetDisplayParts(memberSymbol), memberSymbol.GetGlyph(), - baseSymbolItems.Concat(derivedTypeItems)); + nonNullBaseSymbolItems.Concat(nonNullDerivedTypeItems)); } - private static async ValueTask CreateInheritanceMemberItemForClassOrStructMemberAsync( + private static async ValueTask CreateInheritanceMemberItemForClassOrStructMemberAsync( Solution solution, ISymbol memberSymbol, int lineNumber, @@ -493,7 +496,6 @@ private static async ValueTask CreateInheritanceMemberIte { var implementedMemberItems = await implementedMembers .SelectAsArray(symbol => symbol.OriginalDefinition) - .WhereAsArray(IsNavigableSymbol) .Distinct() .SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync( solution, @@ -501,9 +503,8 @@ private static async ValueTask CreateInheritanceMemberIte InheritanceRelationship.ImplementedMember, cancellationToken), cancellationToken).ConfigureAwait(false); - var overridenMemberItems = await overriddenMembers + var overriddenMemberItems = await overriddenMembers .SelectAsArray(symbol => symbol.OriginalDefinition) - .WhereAsArray(IsNavigableSymbol) .Distinct() .SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync( solution, @@ -513,7 +514,6 @@ private static async ValueTask CreateInheritanceMemberIte var overridingMemberItems = await overridingMembers .SelectAsArray(symbol => symbol.OriginalDefinition) - .WhereAsArray(IsNavigableSymbol) .Distinct() .SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync( solution, @@ -521,15 +521,19 @@ private static async ValueTask CreateInheritanceMemberIte InheritanceRelationship.OverridingMember, cancellationToken), cancellationToken).ConfigureAwait(false); + var nonNullImplementedMemberItems = GetNonNullTargetItems(implementedMemberItems); + var nonNullOverriddenMemberItems = GetNonNullTargetItems(overriddenMemberItems); + var nonNullOverridingMemberItems = GetNonNullTargetItems(overridingMemberItems); + return InheritanceMarginItem.CreateOrdered( lineNumber, topLevelDisplayText: null, FindUsagesHelpers.GetDisplayParts(memberSymbol), memberSymbol.GetGlyph(), - implementedMemberItems.Concat(overridenMemberItems).Concat(overridingMemberItems)); + nonNullImplementedMemberItems.Concat(nonNullOverriddenMemberItems, nonNullOverridingMemberItems)); } - private static async ValueTask CreateInheritanceItemAsync( + private static async ValueTask CreateInheritanceItemAsync( Solution solution, ISymbol targetSymbol, InheritanceRelationship inheritanceRelationship, @@ -540,6 +544,10 @@ private static async ValueTask CreateInheritanceItemAsync // Right now the targets are not shown in a classified way. var definition = ToSlimDefinitionItem(targetSymbol, solution); + if (definition == null) + { + return null; + } var displayName = targetSymbol.ToDisplayString(s_displayFormat); @@ -689,9 +697,8 @@ private static async Task> GetDerivedTypesAndIm /// Otherwise, create the full non-classified DefinitionItem. Because in such case we want to display all the locations to the user /// by reusing the FAR window. /// - private static DefinitionItem ToSlimDefinitionItem(ISymbol symbol, Solution solution) + private static DefinitionItem? ToSlimDefinitionItem(ISymbol symbol, Solution solution) { - RoslynDebug.Assert(IsNavigableSymbol(symbol)); var locations = symbol.Locations; if (locations.Length > 1) { @@ -728,19 +735,21 @@ private static DefinitionItem ToSlimDefinitionItem(ISymbol symbol, Solution solu } } - throw ExceptionUtilities.Unreachable; + return null; } - private static bool IsNavigableSymbol(ISymbol symbol) + private static ImmutableArray GetNonNullTargetItems(ImmutableArray inheritanceTargetItems) { - var locations = symbol.Locations; - if (locations.Length == 1) + using var _ = ArrayBuilder.GetInstance(out var builder); + foreach (var item in inheritanceTargetItems) { - var location = locations[0]; - return location.IsInMetadata || (location.IsInSource && location.IsVisibleSourceLocation()); + if (item.HasValue) + { + builder.Add(item.Value); + } } - return !locations.IsEmpty; + return builder.ToImmutable(); } } } diff --git a/src/Features/Core/Portable/InheritanceMargin/InheritanceMarginItem.cs b/src/Features/Core/Portable/InheritanceMargin/InheritanceMarginItem.cs index 0bc5f0b4f0561..989813a71c8bb 100644 --- a/src/Features/Core/Portable/InheritanceMargin/InheritanceMarginItem.cs +++ b/src/Features/Core/Portable/InheritanceMargin/InheritanceMarginItem.cs @@ -58,15 +58,13 @@ public InheritanceMarginItem( TargetItems = targetItems; } - public static InheritanceMarginItem CreateOrdered( + public static InheritanceMarginItem? CreateOrdered( int lineNumber, string? topLevelDisplayText, ImmutableArray displayTexts, Glyph glyph, ImmutableArray targetItems) - { - return new(lineNumber, topLevelDisplayText, displayTexts, glyph, Order(targetItems)); - } + => targetItems.IsEmpty ? null : new(lineNumber, topLevelDisplayText, displayTexts, glyph, Order(targetItems)); public static ImmutableArray Order(ImmutableArray targetItems) => targetItems.OrderBy(t => t.DisplayName).ThenByDescending(t => t.LanguageGlyph).ThenBy(t => t.ProjectName ?? "").ToImmutableArray();