Skip to content

Commit

Permalink
Filter symbol first before creating InheritanceMargin target (#61911)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@Cosifne
Cosifne authored Jun 21, 2022
1 parent 449873d commit 69bdfb2
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 31 deletions.
17 changes: 17 additions & 0 deletions src/EditorFeatures/Test/InheritanceMargin/InheritanceMarginTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
}
63 changes: 36 additions & 27 deletions src/Features/Core/Portable/InheritanceMargin/AbstractInheritanceMarginService_Helpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -382,7 +383,7 @@ private static async ValueTask AddInheritanceMemberItemsForMembersAsync(
}
}

private static async ValueTask<InheritanceMarginItem> CreateInheritanceMemberItemForInterfaceAsync(
private static async ValueTask<InheritanceMarginItem?> CreateInheritanceMemberItemForInterfaceAsync(
Solution solution,
INamedTypeSymbol interfaceSymbol,
int lineNumber,
Expand All @@ -392,7 +393,6 @@ private static async ValueTask<InheritanceMarginItem> CreateInheritanceMemberIte
{
var baseSymbolItems = await baseSymbols
.SelectAsArray(symbol => symbol.OriginalDefinition)
.WhereAsArray(IsNavigableSymbol)
.Distinct()
.SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync(
solution,
Expand All @@ -403,23 +403,25 @@ private static async ValueTask<InheritanceMarginItem> CreateInheritanceMemberIte

var derivedTypeItems = await derivedTypesSymbols
.SelectAsArray(symbol => symbol.OriginalDefinition)
.WhereAsArray(IsNavigableSymbol)
.Distinct()
.SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync(solution,
symbol,
InheritanceRelationship.ImplementingType,
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<InheritanceMarginItem> CreateInheritanceMemberItemForInterfaceMemberAsync(
private static async ValueTask<InheritanceMarginItem?> CreateInheritanceMemberItemForInterfaceMemberAsync(
Solution solution,
ISymbol memberSymbol,
int lineNumber,
Expand All @@ -428,23 +430,23 @@ private static async ValueTask<InheritanceMarginItem> CreateInheritanceMemberIte
{
var implementedMemberItems = await implementingMembers
.SelectAsArray(symbol => symbol.OriginalDefinition)
.WhereAsArray(IsNavigableSymbol)
.Distinct()
.SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync(
solution,
symbol,
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<InheritanceMarginItem> CreateInheritanceItemForClassAndStructureAsync(
private static async ValueTask<InheritanceMarginItem?> CreateInheritanceItemForClassAndStructureAsync(
Solution solution,
INamedTypeSymbol memberSymbol,
int lineNumber,
Expand All @@ -456,7 +458,6 @@ private static async ValueTask<InheritanceMarginItem> 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,
Expand All @@ -466,23 +467,25 @@ private static async ValueTask<InheritanceMarginItem> CreateInheritanceItemForCl

var derivedTypeItems = await derivedTypesSymbols
.SelectAsArray(symbol => symbol.OriginalDefinition)
.WhereAsArray(IsNavigableSymbol)
.Distinct()
.SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync(solution,
symbol,
InheritanceRelationship.DerivedType,
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<InheritanceMarginItem> CreateInheritanceMemberItemForClassOrStructMemberAsync(
private static async ValueTask<InheritanceMarginItem?> CreateInheritanceMemberItemForClassOrStructMemberAsync(
Solution solution,
ISymbol memberSymbol,
int lineNumber,
Expand All @@ -493,17 +496,15 @@ private static async ValueTask<InheritanceMarginItem> CreateInheritanceMemberIte
{
var implementedMemberItems = await implementedMembers
.SelectAsArray(symbol => symbol.OriginalDefinition)
.WhereAsArray(IsNavigableSymbol)
.Distinct()
.SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync(
solution,
symbol,
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,
Expand All @@ -513,23 +514,26 @@ private static async ValueTask<InheritanceMarginItem> CreateInheritanceMemberIte

var overridingMemberItems = await overridingMembers
.SelectAsArray(symbol => symbol.OriginalDefinition)
.WhereAsArray(IsNavigableSymbol)
.Distinct()
.SelectAsArrayAsync((symbol, _) => CreateInheritanceItemAsync(
solution,
symbol,
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<InheritanceTargetItem> CreateInheritanceItemAsync(
private static async ValueTask<InheritanceTargetItem?> CreateInheritanceItemAsync(
Solution solution,
ISymbol targetSymbol,
InheritanceRelationship inheritanceRelationship,
Expand All @@ -540,6 +544,10 @@ private static async ValueTask<InheritanceTargetItem> 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);

Expand Down Expand Up @@ -689,9 +697,8 @@ private static async Task<ImmutableArray<INamedTypeSymbol>> 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.
/// </summary>
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)
{
Expand Down Expand Up @@ -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<InheritanceTargetItem> GetNonNullTargetItems(ImmutableArray<InheritanceTargetItem?> inheritanceTargetItems)
{
var locations = symbol.Locations;
if (locations.Length == 1)
using var _ = ArrayBuilder<InheritanceTargetItem>.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();
}
}
}
6 changes: 2 additions & 4 deletions src/Features/Core/Portable/InheritanceMargin/InheritanceMarginItem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,13 @@ public InheritanceMarginItem(
TargetItems = targetItems;
}

public static InheritanceMarginItem CreateOrdered(
public static InheritanceMarginItem? CreateOrdered(
int lineNumber,
string? topLevelDisplayText,
ImmutableArray<TaggedText> displayTexts,
Glyph glyph,
ImmutableArray<InheritanceTargetItem> targetItems)
{
return new(lineNumber, topLevelDisplayText, displayTexts, glyph, Order(targetItems));
}
=> targetItems.IsEmpty ? null : new(lineNumber, topLevelDisplayText, displayTexts, glyph, Order(targetItems));

public static ImmutableArray<InheritanceTargetItem> Order(ImmutableArray<InheritanceTargetItem> targetItems)
=> targetItems.OrderBy(t => t.DisplayName).ThenByDescending(t => t.LanguageGlyph).ThenBy(t => t.ProjectName ?? "").ToImmutableArray();
Expand Down

0 comments on commit 69bdfb2

Please sign in to comment.