Skip to content
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

Prioritize original user symbol in FAR results above cascaded symbols. #74805

Merged
merged 3 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ await _context.OnReferencesFoundAsync(
/// Forwards IFindReferencesProgress calls to an IFindUsagesContext instance.
/// </summary>
private sealed class FindReferencesProgressAdapter(
Solution solution, IFindUsagesContext context, FindReferencesSearchOptions searchOptions, OptionsProvider<ClassificationOptions> classificationOptions)
Solution solution,
ISymbol symbol,
IFindUsagesContext context,
FindReferencesSearchOptions searchOptions,
OptionsProvider<ClassificationOptions> classificationOptions)
: IStreamingFindReferencesProgress
{
/// <summary>
Expand Down Expand Up @@ -85,11 +89,14 @@ private async ValueTask<DefinitionItem> GetDefinitionItemAsync(SymbolGroup group
{
if (!_definitionToItem.TryGetValue(group, out var definitionItem))
{
// Ensure that we prioritize the symbol the user is searching for as the primary definition. This
// will ensure it shows up above the rest in the final results.
var isPrimary = group.Symbols.Contains(symbol.OriginalDefinition);
definitionItem = await group.ToClassifiedDefinitionItemAsync(
classificationOptions,
solution,
searchOptions,
isPrimary: _definitionToItem.Count == 0,
isPrimary,
includeHiddenLocations: false,
cancellationToken).ConfigureAwait(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ private static Task FindReferencesInCurrentProcessAsync(
OptionsProvider<ClassificationOptions> classificationOptions,
CancellationToken cancellationToken)
{
var progress = new FindReferencesProgressAdapter(project.Solution, context, searchOptions, classificationOptions);
var progress = new FindReferencesProgressAdapter(
project.Solution, symbol, context, searchOptions, classificationOptions);
return SymbolFinder.FindReferencesAsync(
symbol, project.Solution, progress, documents: null, searchOptions, cancellationToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ private abstract class AbstractTableDataSourceFindUsagesContext :
/// </summary>
private bool _currentlyGroupingByDefinition;

protected readonly List<Entry> EntriesWhenNotGroupingByDefinition = [];
protected readonly List<Entry> EntriesWhenGroupingByDefinition = [];
protected readonly (List<Entry> primary, List<Entry> nonPrimary) EntriesWhenNotGroupingByDefinition = ([], []);
protected readonly (List<Entry> primary, List<Entry> nonPrimary) EntriesWhenGroupingByDefinition = ([], []);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

general idea is that we have 'entries' sorted into two sections. The 'primary' entries (those associated with the original symbol teh user is finding) and non-primary (those that came about from cascading).


private TableEntriesSnapshot? _lastSnapshot;
public int CurrentVersionNumber { get; protected set; }
Expand Down Expand Up @@ -182,9 +182,17 @@ protected AbstractTableDataSourceFindUsagesContext(
protected abstract ValueTask OnDefinitionFoundWorkerAsync(DefinitionItem definition, CancellationToken cancellationToken);
protected abstract ValueTask OnReferenceFoundWorkerAsync(SourceReferenceItem reference, CancellationToken cancellationToken);

protected static void AddRange<T>(List<T> list, ArrayBuilder<T> builder)
protected static void Add<T>((List<T> primary, List<T> nonPrimary) entries, T item, bool isPrimary)
{
var list = isPrimary ? entries.primary : entries.nonPrimary;
list.Add(item);
}

protected static void AddRange<T>((List<T> primary, List<T> nonPrimary) entries, ArrayBuilder<T> builder, bool isPrimary)
{
var list = isPrimary ? entries.primary : entries.nonPrimary;
list.Capacity = list.Count + builder.Count;

foreach (var item in builder)
list.Add(item);
}
Expand Down Expand Up @@ -589,8 +597,8 @@ public ITableEntriesSnapshot GetCurrentSnapshot()
var entries = _cleared
? []
: _currentlyGroupingByDefinition
? EntriesWhenGroupingByDefinition.ToImmutableArray()
: EntriesWhenNotGroupingByDefinition.ToImmutableArray();
? ToImmutableArray(EntriesWhenGroupingByDefinition)
: ToImmutableArray(EntriesWhenNotGroupingByDefinition);

_lastSnapshot = new TableEntriesSnapshot(entries, CurrentVersionNumber);
}
Expand All @@ -599,6 +607,18 @@ public ITableEntriesSnapshot GetCurrentSnapshot()
}
}

private static ImmutableArray<Entry> ToImmutableArray((List<Entry> primary, List<Entry> nonPrimary) entries)
{
var result = new FixedSizeArrayBuilder<Entry>(entries.nonPrimary.Count + entries.primary.Count);
foreach (var entry in entries.primary)
result.Add(entry);

foreach (var entry in entries.nonPrimary)
result.Add(entry);

return result.MoveToImmutable();
}

public ITableEntriesSnapshot? GetSnapshot(int versionNumber)
{
lock (Gate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,23 @@ internal partial class StreamingFindUsagesPresenter
/// This context supports showing reference items, and will display appropriate messages
/// about no-references being found for a definition at the end of the search.
/// </summary>
private sealed class WithReferencesFindUsagesContext : AbstractTableDataSourceFindUsagesContext
private sealed class WithReferencesFindUsagesContext(
StreamingFindUsagesPresenter presenter,
IFindAllReferencesWindow findReferencesWindow,
ImmutableArray<ITableColumnDefinition> customColumns,
IGlobalOptionService globalOptions,
bool includeContainingTypeAndMemberColumns,
bool includeKindColumn,
IThreadingContext threadingContext)
: AbstractTableDataSourceFindUsagesContext(
presenter,
findReferencesWindow,
customColumns,
globalOptions,
includeContainingTypeAndMemberColumns,
includeKindColumn,
threadingContext)
{
public WithReferencesFindUsagesContext(
StreamingFindUsagesPresenter presenter,
IFindAllReferencesWindow findReferencesWindow,
ImmutableArray<ITableColumnDefinition> customColumns,
IGlobalOptionService globalOptions,
bool includeContainingTypeAndMemberColumns,
bool includeKindColumn,
IThreadingContext threadingContext)
: base(presenter, findReferencesWindow, customColumns, globalOptions, includeContainingTypeAndMemberColumns, includeKindColumn, threadingContext)
{
}

protected override async ValueTask OnDefinitionFoundWorkerAsync(DefinitionItem definition, CancellationToken cancellationToken)
{
// If this is a definition we always want to show, then create entries for all the declaration locations
Expand Down Expand Up @@ -75,14 +78,15 @@ private async Task AddDeclarationEntriesAsync(DefinitionItem definition, bool ex
await AddDocumentSpanEntriesAsync(entries, definitionBucket, definition, cancellationToken).ConfigureAwait(false);

var changed = false;
var isPrimary = IsPrimary(definition);
lock (Gate)
{
// Do one final check to ensure that no other thread beat us here.
if (!HasDeclarationEntries(definition))
{
// We only include declaration entries in the entries we show when
// not grouping by definition.
AddRange(EntriesWhenNotGroupingByDefinition, entries);
AddRange(EntriesWhenNotGroupingByDefinition, entries, isPrimary);
CurrentVersionNumber++;
changed = true;
}
Expand All @@ -99,7 +103,13 @@ private bool HasDeclarationEntries(DefinitionItem definition)
{
lock (Gate)
{
foreach (var entry in EntriesWhenNotGroupingByDefinition)
foreach (var entry in EntriesWhenNotGroupingByDefinition.primary)
{
if (entry.DefinitionBucket.DefinitionItem == definition)
return true;
}

foreach (var entry in EntriesWhenNotGroupingByDefinition.nonPrimary)
{
if (entry.DefinitionBucket.DefinitionItem == definition)
return true;
Expand Down Expand Up @@ -151,16 +161,17 @@ private async ValueTask OnEntryFoundAsync(
// Proceed, even if we didn't create an entry. It's possible that we augmented
// an existing entry and we want the UI to refresh to show the results of that.

var isPrimary = IsPrimary(definition);
lock (Gate)
{
if (entry != null)
{
// Once we can make the new entry, add it to the appropriate list.
if (addToEntriesWhenGroupingByDefinition)
EntriesWhenGroupingByDefinition.Add(entry);
Add(EntriesWhenGroupingByDefinition, entry, isPrimary);

if (addToEntriesWhenNotGroupingByDefinition)
EntriesWhenNotGroupingByDefinition.Add(entry);
Add(EntriesWhenNotGroupingByDefinition, entry, isPrimary);
}

CurrentVersionNumber++;
Expand Down Expand Up @@ -228,15 +239,17 @@ private ImmutableArray<DefinitionItem> GetDefinitionsToCreateMissingReferenceIte
{
lock (Gate)
{
var entries = whenGroupingByDefinition
var (primary, nonPrimary) = whenGroupingByDefinition
? EntriesWhenGroupingByDefinition
: EntriesWhenNotGroupingByDefinition;

// Find any definitions that we didn't have any references to. But only show
// them if they want to be displayed without any references. This will
// ensure that we still see things like overrides and whatnot, but we
// won't show property-accessors.
var seenDefinitions = entries.Select(r => r.DefinitionBucket.DefinitionItem).ToSet();
var seenDefinitions = primary.Concat(nonPrimary)
.Select(r => r.DefinitionBucket.DefinitionItem)
.ToSet();
var q = from definition in Definitions
where !seenDefinitions.Contains(definition) &&
definition.DisplayIfNoReferences
Expand All @@ -245,19 +258,15 @@ private ImmutableArray<DefinitionItem> GetDefinitionsToCreateMissingReferenceIte
// If we find at least one of these types of definitions, then just return those.
var result = ImmutableArray.CreateRange(q);
if (result.Length > 0)
{
return result;
}

// We found no definitions that *want* to be displayed. However, we still
// want to show something. So, if necessary, show at lest the first definition
// even if we found no references and even if it would prefer to not be seen.
if (entries.Count == 0 && Definitions.Count > 0)
{
// We found no definitions that *want* to be displayed. However, we still want to show something. So,
// if necessary, show at lest the first definition even if we found no references and even if it would
// prefer to not be seen.
if (primary.Count == 0 && nonPrimary.Count == 0 && Definitions.Count > 0)
return [Definitions.First()];
}

return ImmutableArray<DefinitionItem>.Empty;
return [];
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,23 @@ internal partial class StreamingFindUsagesPresenter
/// This context will not group entries by definition, and will instead just create
/// entries for the definitions themselves.
/// </summary>
private sealed class WithoutReferencesFindUsagesContext : AbstractTableDataSourceFindUsagesContext
private sealed class WithoutReferencesFindUsagesContext(
StreamingFindUsagesPresenter presenter,
IFindAllReferencesWindow findReferencesWindow,
ImmutableArray<ITableColumnDefinition> customColumns,
IGlobalOptionService globalOptions,
bool includeContainingTypeAndMemberColumns,
bool includeKindColumn,
IThreadingContext threadingContext)
: AbstractTableDataSourceFindUsagesContext(
presenter,
findReferencesWindow,
customColumns,
globalOptions,
includeContainingTypeAndMemberColumns,
includeKindColumn,
threadingContext)
{
public WithoutReferencesFindUsagesContext(
StreamingFindUsagesPresenter presenter,
IFindAllReferencesWindow findReferencesWindow,
ImmutableArray<ITableColumnDefinition> customColumns,
IGlobalOptionService globalOptions,
bool includeContainingTypeAndMemberColumns,
bool includeKindColumn,
IThreadingContext threadingContext)
: base(presenter, findReferencesWindow, customColumns, globalOptions, includeContainingTypeAndMemberColumns, includeKindColumn, threadingContext)
{
}

// We should never be called in a context where we get references.
protected override ValueTask OnReferenceFoundWorkerAsync(SourceReferenceItem reference, CancellationToken cancellationToken)
Expand All @@ -60,10 +64,13 @@ private async Task CreateNoResultsFoundEntryIfNecessaryAsync()
var definitionBucket = GetOrCreateDefinitionBucket(CreateNoResultsDefinitionItem(message), expandedByDefault: true);
var entry = await SimpleMessageEntry.CreateAsync(definitionBucket, navigationBucket: null, message).ConfigureAwait(false);

var isPrimary = IsPrimary(definitionBucket.DefinitionItem);

lock (Gate)
{
EntriesWhenGroupingByDefinition.Add(entry);
EntriesWhenNotGroupingByDefinition.Add(entry);
Add(EntriesWhenGroupingByDefinition, entry, isPrimary);
Add(EntriesWhenNotGroupingByDefinition, entry, isPrimary);

CurrentVersionNumber++;
}

Expand Down Expand Up @@ -105,10 +112,12 @@ protected override async ValueTask OnDefinitionFoundWorkerAsync(DefinitionItem d

if (entries.Count > 0)
{
var isPrimary = IsPrimary(definition);

lock (Gate)
{
AddRange(EntriesWhenGroupingByDefinition, entries);
AddRange(EntriesWhenNotGroupingByDefinition, entries);
AddRange(EntriesWhenGroupingByDefinition, entries, isPrimary);
AddRange(EntriesWhenNotGroupingByDefinition, entries, isPrimary);
CurrentVersionNumber++;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ private StreamingFindUsagesPresenter(
public IClassificationFormatMap ClassificationFormatMap
=> _lazyClassificationFormatMap.Value;

private static bool IsPrimary(DefinitionItem definition)
=> definition.Properties.ContainsKey(DefinitionItem.Primary);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already had this data in the definitionItem. We were using it for other featuers. but hadn't updated FAR properly.


private static IEnumerable<ITableColumnDefinition> GetCustomColumns(IEnumerable<Lazy<ITableColumnDefinition, NameMetadata>> columns)
{
foreach (var column in columns)
Expand Down