Skip to content

Add SortDocumentSymbols to make the outline hierarchical (again) #2084

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Add SortDocumentSymbols to make the outline hierarchical (again) #2084

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27b5325
Add `SortDocumentSymbols` to make the outline hierarchical (again)
andyleejordan Sep 27, 2023
8de86ff
Improve algorithm to avoid excess allocations
andyleejordan Oct 13, 2023
d68be7d
Optimize algorithm by sorting in-place
andyleejordan Oct 19, 2023
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
148 changes: 131 additions & 17 deletions src/PowerShellEditorServices/Services/TextDocument/Handlers/DocumentSymbolHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System.Collections.Generic;
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -40,15 +41,118 @@ public PsesDocumentSymbolHandler(ILoggerFactory factory, WorkspaceService worksp
DocumentSelector = LspUtils.PowerShellDocumentSelector
};

// AKA the outline feature
// Modifies a flat list of symbols into a hierarchical list.
private static Task SortHierarchicalSymbols(List<HierarchicalSymbol> symbols, CancellationToken cancellationToken)
{
// Sort by the start of the symbol definition (they're probably sorted but we need to be
// certain otherwise this algorithm won't work). We only need to sort the list once, and
// since the implementation is recursive, it's easiest to use the stack to track that
// this is the first call.
symbols.Sort((x1, x2) => x1.Range.Start.CompareTo(x2.Range.Start));
return SortHierarchicalSymbolsImpl(symbols, cancellationToken);
}

private static async Task SortHierarchicalSymbolsImpl(List<HierarchicalSymbol> symbols, CancellationToken cancellationToken)
{
for (int i = 0; i < symbols.Count; i++)
{
// This async method is pretty dense with synchronous code
// so it's helpful to add some yields.
await Task.Yield();
if (cancellationToken.IsCancellationRequested)
{
return;
}

HierarchicalSymbol symbol = symbols[i];

// Base case where we haven't found any parents yet (the first symbol must be a
// parent by definition).
if (i == 0)
{
continue;
}
// If the symbol starts after end of last symbol parsed then it's a new parent.
else if (symbol.Range.Start > symbols[i - 1].Range.End)
{
continue;
}
// Otherwise it's a child, we just need to figure out whose child it is and move it there (which also means removing it from the current list).
else
{
for (int j = 0; j <= i; j++)
{
// While we should only check up to j < i, we iterate up to j <= i so that
// we can check this assertion that we didn't exhaust the parents.
Debug.Assert(j != i, "We didn't find the child's parent!");

HierarchicalSymbol parent = symbols[j];
// If the symbol starts after the parent starts and ends before the parent
// ends then its a child.
if (symbol.Range.Start > parent.Range.Start && symbol.Range.End < parent.Range.End)
{
// Add it to the parent's list.
parent.Children.Add(symbol);
// Remove it from this "parents" list (because it's a child) and adjust
// our loop counter because it's been removed.
symbols.RemoveAt(i);
i--;
break;
}
}
}
}

// Now recursively sort the children into nested buckets of children too.
foreach (HierarchicalSymbol parent in symbols)
{
// Since this modifies in place we just recurse, no re-assignment or clearing from
// parent.Children necessary.
await SortHierarchicalSymbols(parent.Children, cancellationToken).ConfigureAwait(false);
}
}

// This struct and the mapping function below exist to allow us to skip a *bunch* of
// unnecessary allocations when sorting the symbols since DocumentSymbol (which this is
// pretty much a mirror of) is an immutable record...but we need to constantly modify the
// list of children when sorting.
private struct HierarchicalSymbol
{
public SymbolKind Kind;
public Range Range;
public Range SelectionRange;
public string Name;
public List<HierarchicalSymbol> Children;
}

// Recursively turn our HierarchicalSymbol struct into OmniSharp's DocumentSymbol record.
private static List<DocumentSymbol> GetDocumentSymbolsFromHierarchicalSymbols(IEnumerable<HierarchicalSymbol> hierarchicalSymbols)
{
List<DocumentSymbol> documentSymbols = new();
foreach (HierarchicalSymbol symbol in hierarchicalSymbols)
{
documentSymbols.Add(new DocumentSymbol
{
Kind = symbol.Kind,
Range = symbol.Range,
SelectionRange = symbol.SelectionRange,
Name = symbol.Name,
Children = GetDocumentSymbolsFromHierarchicalSymbols(symbol.Children)
});
}
return documentSymbols;
}

// AKA the outline feature!
public override async Task<SymbolInformationOrDocumentSymbolContainer> Handle(DocumentSymbolParams request, CancellationToken cancellationToken)
{
_logger.LogDebug($"Handling document symbols for {request.TextDocument.Uri}");

ScriptFile scriptFile = _workspaceService.GetFile(request.TextDocument.Uri);
List<SymbolInformationOrDocumentSymbol> symbols = new();

foreach (SymbolReference r in ProvideDocumentSymbols(scriptFile))
List<HierarchicalSymbol> hierarchicalSymbols = new();

foreach (SymbolReference symbolReference in ProvideDocumentSymbols(scriptFile))
{
// This async method is pretty dense with synchronous code
// so it's helpful to add some yields.
Expand All @@ -62,27 +166,37 @@ public override async Task<SymbolInformationOrDocumentSymbolContainer> Handle(Do
//
// TODO: We should also include function invocations that are part of DSLs (like
// Invoke-Build etc.).
if (!r.IsDeclaration || r.Type is SymbolType.Parameter)
if (!symbolReference.IsDeclaration || symbolReference.Type is SymbolType.Parameter)
{
continue;
}

// TODO: This now needs the Children property filled out to support hierarchical
// symbols, and we don't have the information nor algorithm to do that currently.
// OmniSharp was previously doing this for us based on the range, perhaps we can
// find that logic and reuse it.
symbols.Add(new SymbolInformationOrDocumentSymbol(new DocumentSymbol
hierarchicalSymbols.Add(new HierarchicalSymbol
{
Kind = SymbolTypeUtils.GetSymbolKind(r.Type),
Range = r.ScriptRegion.ToRange(),
SelectionRange = r.NameRegion.ToRange(),
Name = r.Name
}));
Kind = SymbolTypeUtils.GetSymbolKind(symbolReference.Type),
Range = symbolReference.ScriptRegion.ToRange(),
SelectionRange = symbolReference.NameRegion.ToRange(),
Name = symbolReference.Name,
Children = new List<HierarchicalSymbol>()
});
}

return symbols.Count == 0
? s_emptySymbolInformationOrDocumentSymbolContainer
: new SymbolInformationOrDocumentSymbolContainer(symbols);
// Short-circuit if we have no symbols.
if (hierarchicalSymbols.Count == 0)
{
return s_emptySymbolInformationOrDocumentSymbolContainer;
}

// Otherwise slowly sort them into a hierarchy (this modifies the list).
await SortHierarchicalSymbols(hierarchicalSymbols, cancellationToken).ConfigureAwait(false);

// And finally convert them to the silly SymbolInformationOrDocumentSymbol wrapper.
List<SymbolInformationOrDocumentSymbol> container = new();
foreach (DocumentSymbol symbol in GetDocumentSymbolsFromHierarchicalSymbols(hierarchicalSymbols))
{
container.Add(new SymbolInformationOrDocumentSymbol(symbol));
}
return container;
}

private IEnumerable<SymbolReference> ProvideDocumentSymbols(ScriptFile scriptFile)
Expand Down