From 7916a0cdbc8a87602dca688f6a02007c317f8155 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 15 Sep 2022 17:39:48 -0400 Subject: [PATCH] Overhaul workspace search for symbol references (#1917) Significantly reduce performance overhead of reference finding in large workspaces. * Dependent on PowerShell/vscode-powershell#4170 * Adds a reference cache to every ScriptFile * Workspace scan is performed only once on first request * An LSP file system watcher provides updates for files not changed via Did*TextDocument notifications * Adds a setting to only search "open documents" for references. This disables both the initial workspace scan and the file system watcher, relying only on Did*TextDocument notifications. As a stress test I opened up my profile directory (which has ~3k script files in total in the Modules directory) and created a file with a function definition on every line. I then tabbed to a different file, and then tabbed back to the new file. Before the changes, the references code lens took ~10 seconds to populate and my CPU spiked to ~50% usage. After the changes, they populated instantly and CPU spiked to ~2% usage. --- .../Server/PsesLanguageServer.cs | 1 + .../PowerShell/Utility/CommandHelpers.cs | 34 ++++ .../Services/Symbols/ReferenceTable.cs | 118 ++++++++++++ .../Services/Symbols/SymbolsService.cs | 172 ++++++++++++++---- .../Services/Symbols/Vistors/AstOperations.cs | 39 ++++ .../Handlers/DidChangeWatchedFilesHandler.cs | 117 ++++++++++++ .../Handlers/TextDocumentHandler.cs | 47 +++-- .../Services/TextDocument/ScriptFile.cs | 8 +- .../Workspace/LanguageServerSettings.cs | 2 + .../Services/Workspace/WorkspaceService.cs | 19 +- .../LanguageServerProtocolMessageTests.cs | 12 +- .../Language/SymbolsServiceTests.cs | 20 +- 12 files changed, 515 insertions(+), 74 deletions(-) create mode 100644 src/PowerShellEditorServices/Services/Symbols/ReferenceTable.cs create mode 100644 src/PowerShellEditorServices/Services/TextDocument/Handlers/DidChangeWatchedFilesHandler.cs diff --git a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs index 1f740cec6..9ebdac3fc 100644 --- a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs +++ b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs @@ -119,6 +119,7 @@ public async Task StartAsync() .WithHandler() .WithHandler() .WithHandler() + .WithHandler() // NOTE: The OnInitialize delegate gets run when we first receive the // _Initialize_ request: // https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize diff --git a/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs index 560a6524f..f5590a026 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Management.Automation; @@ -57,6 +58,39 @@ public record struct AliasMap( internal static readonly ConcurrentDictionary> s_cmdletToAliasCache = new(System.StringComparer.OrdinalIgnoreCase); internal static readonly ConcurrentDictionary s_aliasToCmdletCache = new(System.StringComparer.OrdinalIgnoreCase); + /// + /// Gets the actual command behind a fully module qualified command invocation, e.g. + /// Microsoft.PowerShell.Management\Get-ChildItem will return Get-ChildItem + /// + /// + /// The potentially module qualified command name at the site of invocation. + /// + /// + /// A reference that will contain the module name if the invocation is module qualified. + /// + /// The actual command name. + public static string StripModuleQualification(string invocationName, out ReadOnlyMemory moduleName) + { + int slashIndex = invocationName.LastIndexOfAny(new[] { '\\', '/' }); + if (slashIndex is -1) + { + moduleName = default; + return invocationName; + } + + // If '\' is the last character then it's probably not a module qualified command. + if (slashIndex == invocationName.Length - 1) + { + moduleName = default; + return invocationName; + } + + // Storing moduleName as ROMemory saves a string allocation in the common case where it + // is not needed. + moduleName = invocationName.AsMemory().Slice(0, slashIndex); + return invocationName.Substring(slashIndex + 1); + } + /// /// Gets the CommandInfo instance for a command with a particular name. /// diff --git a/src/PowerShellEditorServices/Services/Symbols/ReferenceTable.cs b/src/PowerShellEditorServices/Services/Symbols/ReferenceTable.cs new file mode 100644 index 000000000..2c3242c83 --- /dev/null +++ b/src/PowerShellEditorServices/Services/Symbols/ReferenceTable.cs @@ -0,0 +1,118 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#nullable enable + +using System; +using System.Collections.Concurrent; +using System.Management.Automation.Language; +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility; +using Microsoft.PowerShell.EditorServices.Services.Symbols; + +namespace Microsoft.PowerShell.EditorServices.Services; + +/// +/// Represents the symbols that are referenced and their locations within a single document. +/// +internal sealed class ReferenceTable +{ + private readonly ScriptFile _parent; + + private readonly ConcurrentDictionary> _symbolReferences = new(StringComparer.OrdinalIgnoreCase); + + private bool _isInited; + + public ReferenceTable(ScriptFile parent) => _parent = parent; + + /// + /// Clears the reference table causing it to rescan the source AST when queried. + /// + public void TagAsChanged() + { + _symbolReferences.Clear(); + _isInited = false; + } + + // Prefer checking if the dictionary has contents to determine if initialized. The field + // `_isInited` is to guard against rescanning files with no command references, but will + // generally be less reliable of a check. + private bool IsInitialized => !_symbolReferences.IsEmpty || _isInited; + + internal bool TryGetReferences(string command, out ConcurrentBag? references) + { + EnsureInitialized(); + return _symbolReferences.TryGetValue(command, out references); + } + + internal void EnsureInitialized() + { + if (IsInitialized) + { + return; + } + + _parent.ScriptAst.Visit(new ReferenceVisitor(this)); + } + + private void AddReference(string symbol, IScriptExtent extent) + { + _symbolReferences.AddOrUpdate( + symbol, + _ => new ConcurrentBag { extent }, + (_, existing) => + { + existing.Add(extent); + return existing; + }); + } + + private sealed class ReferenceVisitor : AstVisitor + { + private readonly ReferenceTable _references; + + public ReferenceVisitor(ReferenceTable references) => _references = references; + + public override AstVisitAction VisitCommand(CommandAst commandAst) + { + string? commandName = GetCommandName(commandAst); + if (string.IsNullOrEmpty(commandName)) + { + return AstVisitAction.Continue; + } + + _references.AddReference( + CommandHelpers.StripModuleQualification(commandName, out _), + commandAst.CommandElements[0].Extent); + + return AstVisitAction.Continue; + + static string? GetCommandName(CommandAst commandAst) + { + string commandName = commandAst.GetCommandName(); + if (!string.IsNullOrEmpty(commandName)) + { + return commandName; + } + + if (commandAst.CommandElements[0] is not ExpandableStringExpressionAst expandableStringExpressionAst) + { + return null; + } + + return AstOperations.TryGetInferredValue(expandableStringExpressionAst, out string value) ? value : null; + } + } + + public override AstVisitAction VisitVariableExpression(VariableExpressionAst variableExpressionAst) + { + // TODO: Consider tracking unscoped variable references only when they declared within + // the same function definition. + _references.AddReference( + $"${variableExpressionAst.VariablePath.UserPath}", + variableExpressionAst.Extent); + + return AstVisitAction.Continue; + } + } +} diff --git a/src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs b/src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs index 29f3ceea3..f052520f0 100644 --- a/src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs +++ b/src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs @@ -41,6 +41,7 @@ internal class SymbolsService private readonly ConcurrentDictionary _codeLensProviders; private readonly ConcurrentDictionary _documentSymbolProviders; + private readonly ConfigurationService _configurationService; #endregion #region Constructors @@ -65,6 +66,7 @@ public SymbolsService( _runspaceContext = runspaceContext; _executionService = executionService; _workspaceService = workspaceService; + _configurationService = configurationService; _codeLensProviders = new ConcurrentDictionary(); if (configurationService.CurrentSettings.EnableReferencesCodeLens) @@ -177,8 +179,15 @@ public async Task> FindReferencesOfSymbol( _executionService, cancellationToken).ConfigureAwait(false); - Dictionary> cmdletToAliases = aliases.CmdletToAliases; - Dictionary aliasToCmdlets = aliases.AliasToCmdlets; + string targetName = foundSymbol.SymbolName; + if (foundSymbol.SymbolType is SymbolType.Function) + { + targetName = CommandHelpers.StripModuleQualification(targetName, out _); + if (aliases.AliasToCmdlets.TryGetValue(foundSymbol.SymbolName, out string aliasDefinition)) + { + targetName = aliasDefinition; + } + } // We want to look for references first in referenced files, hence we use ordered dictionary // TODO: File system case-sensitivity is based on filesystem not OS, but OS is a much cheaper heuristic @@ -191,52 +200,63 @@ public async Task> FindReferencesOfSymbol( fileMap[scriptFile.FilePath] = scriptFile; } - foreach (string filePath in workspace.EnumeratePSFiles()) + await ScanWorkspacePSFiles(cancellationToken).ConfigureAwait(false); + + List symbolReferences = new(); + + // Using a nested method here to get a bit more readability and to avoid roslynator + // asserting we should use a giant nested ternary here. + static string[] GetIdentifiers(string symbolName, SymbolType symbolType, CommandHelpers.AliasMap aliases) { - if (!fileMap.Contains(filePath)) + if (symbolType is not SymbolType.Function) { - // This async method is pretty dense with synchronous code - // so it's helpful to add some yields. - await Task.Yield(); - cancellationToken.ThrowIfCancellationRequested(); - if (!workspace.TryGetFile(filePath, out ScriptFile scriptFile)) - { - // If we can't access the file for some reason, just ignore it - continue; - } + return new[] { symbolName }; + } - fileMap[filePath] = scriptFile; + if (!aliases.CmdletToAliases.TryGetValue(symbolName, out List foundAliasList)) + { + return new[] { symbolName }; } + + return foundAliasList.Prepend(symbolName) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); } - List symbolReferences = new(); - foreach (object fileName in fileMap.Keys) - { - ScriptFile file = (ScriptFile)fileMap[fileName]; + string[] allIdentifiers = GetIdentifiers(targetName, foundSymbol.SymbolType, aliases); - IEnumerable references = AstOperations.FindReferencesOfSymbol( - file.ScriptAst, - foundSymbol, - cmdletToAliases, - aliasToCmdlets); - - foreach (SymbolReference reference in references) + foreach (ScriptFile file in _workspaceService.GetOpenedFiles()) + { + foreach (string targetIdentifier in allIdentifiers) { - try + if (!file.References.TryGetReferences(targetIdentifier, out ConcurrentBag references)) { - reference.SourceLine = file.GetLine(reference.ScriptRegion.StartLineNumber); + continue; } - catch (ArgumentOutOfRangeException e) + + foreach (IScriptExtent extent in references.OrderBy(e => e.StartOffset)) { - reference.SourceLine = string.Empty; - _logger.LogException("Found reference is out of range in script file", e); + SymbolReference reference = new( + SymbolType.Function, + foundSymbol.SymbolName, + extent); + + try + { + reference.SourceLine = file.GetLine(reference.ScriptRegion.StartLineNumber); + } + catch (ArgumentOutOfRangeException e) + { + reference.SourceLine = string.Empty; + _logger.LogException("Found reference is out of range in script file", e); + } + reference.FilePath = file.FilePath; + symbolReferences.Add(reference); } - reference.FilePath = file.FilePath; - symbolReferences.Add(reference); - } - await Task.Yield(); - cancellationToken.ThrowIfCancellationRequested(); + await Task.Yield(); + cancellationToken.ThrowIfCancellationRequested(); + } } return symbolReferences; @@ -495,6 +515,59 @@ await CommandHelpers.GetCommandInfoAsync( return foundDefinition; } + private Task _workspaceScanCompleted; + + private async Task ScanWorkspacePSFiles(CancellationToken cancellationToken = default) + { + if (_configurationService.CurrentSettings.AnalyzeOpenDocumentsOnly) + { + return; + } + + Task scanTask = _workspaceScanCompleted; + // It's not impossible for two scans to start at once but it should be exceedingly + // unlikely, and shouldn't break anything if it happens to. So we can save some + // lock time by accepting that possibility. + if (scanTask is null) + { + scanTask = Task.Run( + () => + { + foreach (string file in _workspaceService.EnumeratePSFiles()) + { + if (_workspaceService.TryGetFile(file, out ScriptFile scriptFile)) + { + scriptFile.References.EnsureInitialized(); + } + } + }, + CancellationToken.None); + + // Ignore the analyzer yelling that we're not awaiting this task, we'll get there. +#pragma warning disable CS4014 + Interlocked.CompareExchange(ref _workspaceScanCompleted, scanTask, null); +#pragma warning restore CS4014 + } + + // In the simple case where the task is already completed or the token we're given cannot + // be cancelled, do a simple await. + if (scanTask.IsCompleted || !cancellationToken.CanBeCanceled) + { + await scanTask.ConfigureAwait(false); + return; + } + + // If it's not yet done and we can be cancelled, create a new task to represent the + // cancellation. That way we can exit a request that relies on the scan without + // having to actually stop the work (and then request it again in a few seconds). + // + // TODO: There's a new API in net6 that lets you await a task with a cancellation token. + // we should #if that in if feasible. + TaskCompletionSource cancelled = new(); + cancellationToken.Register(() => cancelled.TrySetCanceled()); + await Task.WhenAny(scanTask, cancelled.Task).ConfigureAwait(false); + } + /// /// Gets a path from a dot-source symbol. /// @@ -673,6 +746,35 @@ public static FunctionDefinitionAst GetFunctionDefinitionAtLine( internal void OnConfigurationUpdated(object _, LanguageServerSettings e) { + if (e.AnalyzeOpenDocumentsOnly) + { + Task scanInProgress = _workspaceScanCompleted; + if (scanInProgress is not null) + { + // Wait until after the scan completes to close unopened files. + _ = scanInProgress.ContinueWith(_ => CloseUnopenedFiles(), TaskScheduler.Default); + } + else + { + CloseUnopenedFiles(); + } + + _workspaceScanCompleted = null; + + void CloseUnopenedFiles() + { + foreach (ScriptFile scriptFile in _workspaceService.GetOpenedFiles()) + { + if (scriptFile.IsOpen) + { + continue; + } + + _workspaceService.CloseFile(scriptFile); + } + } + } + if (e.EnableReferencesCodeLens) { if (_codeLensProviders.ContainsKey(ReferencesCodeLensProvider.Id)) diff --git a/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs b/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs index 841408ad5..e14b2051a 100644 --- a/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs +++ b/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs @@ -24,6 +24,7 @@ namespace Microsoft.PowerShell.EditorServices.Services.Symbols internal static class AstOperations { private static readonly Func s_clonePositionWithNewOffset; + static AstOperations() { Type internalScriptPositionType = typeof(PSObject).GetTypeInfo().Assembly @@ -311,5 +312,43 @@ public static string[] FindDotSourcedIncludes(Ast scriptAst, string psScriptRoot return dotSourcedVisitor.DotSourcedFiles.ToArray(); } + + internal static bool TryGetInferredValue(ExpandableStringExpressionAst expandableStringExpressionAst, out string value) + { + // Currently we only support inferring the value of `$PSScriptRoot`. We could potentially + // expand this to parts of `$MyInvocation` and some basic constant folding. + if (string.IsNullOrEmpty(expandableStringExpressionAst.Extent.File)) + { + value = null; + return false; + } + + string psScriptRoot = System.IO.Path.GetDirectoryName(expandableStringExpressionAst.Extent.File); + if (string.IsNullOrEmpty(psScriptRoot)) + { + value = null; + return false; + } + + string path = expandableStringExpressionAst.Value; + foreach (ExpressionAst nestedExpression in expandableStringExpressionAst.NestedExpressions) + { + // If the string contains the variable $PSScriptRoot, we replace it with the corresponding value. + if (!(nestedExpression is VariableExpressionAst variableAst + && variableAst.VariablePath.UserPath.Equals("PSScriptRoot", StringComparison.OrdinalIgnoreCase))) + { + value = null; + return false; + } + + // TODO: This should use offsets from the extent rather than a blind replace. In + // practice it won't hurt anything because $ is not valid in paths, but if we expand + // this functionality, this will be problematic. + path = path.Replace(variableAst.ToString(), psScriptRoot); + } + + value = path; + return true; + } } } diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/DidChangeWatchedFilesHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/DidChangeWatchedFilesHandler.cs new file mode 100644 index 000000000..edbe9b0ad --- /dev/null +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/DidChangeWatchedFilesHandler.cs @@ -0,0 +1,117 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#nullable enable + +using System.Threading; +using System.Threading.Tasks; +using MediatR; +using Microsoft.Extensions.FileSystemGlobbing; +using Microsoft.PowerShell.EditorServices.Services; +using Microsoft.PowerShell.EditorServices.Services.Configuration; +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; +using OmniSharp.Extensions.LanguageServer.Protocol.Workspace; + +// Using an alias since this is conflicting with System.IO.FileSystemWatcher and ends up really finicky. +using OmniSharpFileSystemWatcher = OmniSharp.Extensions.LanguageServer.Protocol.Models.FileSystemWatcher; + +namespace Microsoft.PowerShell.EditorServices.Handlers; + +/// +/// Receives file change notifications from the client for any file in the workspace, including those +/// that are not considered opened by the client. This handler is used to allow us to scan the +/// workspace only once when the language server starts. +/// +internal class DidChangeWatchedFilesHandler : IDidChangeWatchedFilesHandler +{ + private readonly WorkspaceService _workspaceService; + + private readonly ConfigurationService _configurationService; + + public DidChangeWatchedFilesHandler( + WorkspaceService workspaceService, + ConfigurationService configurationService) + { + _workspaceService = workspaceService; + _configurationService = configurationService; + } + + public DidChangeWatchedFilesRegistrationOptions GetRegistrationOptions( + DidChangeWatchedFilesCapability capability, + ClientCapabilities clientCapabilities) + => new() + { + Watchers = new[] + { + new OmniSharpFileSystemWatcher() + { + GlobPattern = "**/*.{ps1,psm1}", + Kind = WatchKind.Create | WatchKind.Delete | WatchKind.Change, + }, + }, + }; + + public Task Handle(DidChangeWatchedFilesParams request, CancellationToken cancellationToken) + { + LanguageServerSettings currentSettings = _configurationService.CurrentSettings; + if (currentSettings.AnalyzeOpenDocumentsOnly) + { + return Task.FromResult(Unit.Value); + } + + // Honor `search.exclude` settings in the watcher. + Matcher matcher = new(); + matcher.AddExcludePatterns(_workspaceService.ExcludeFilesGlob); + foreach (FileEvent change in request.Changes) + { + if (matcher.Match(change.Uri.GetFileSystemPath()).HasMatches) + { + continue; + } + + if (!_workspaceService.TryGetFile(change.Uri, out ScriptFile scriptFile)) + { + continue; + } + + if (change.Type is FileChangeType.Created) + { + // We've already triggered adding the file to `OpenedFiles` via `TryGetFile`. + continue; + } + + if (change.Type is FileChangeType.Deleted) + { + _workspaceService.CloseFile(scriptFile); + continue; + } + + if (change.Type is FileChangeType.Changed) + { + // If the file is opened by the editor (rather than by us in the background), let + // DidChangeTextDocument handle changes. + if (scriptFile.IsOpen) + { + continue; + } + + string fileContents; + try + { + fileContents = WorkspaceService.ReadFileContents(change.Uri); + } + catch + { + continue; + } + + scriptFile.SetFileContents(fileContents); + scriptFile.References.TagAsChanged(); + } + } + + return Task.FromResult(Unit.Value); + } +} diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs index 0f18d27cb..d9ece3db5 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs @@ -26,6 +26,8 @@ internal class PsesTextDocumentHandler : TextDocumentSyncHandlerBase private readonly WorkspaceService _workspaceService; private readonly RemoteFileManagerService _remoteFileManagerService; + private bool _isFileWatcherSupported; + public static TextDocumentSyncKind Change => TextDocumentSyncKind.Incremental; public PsesTextDocumentHandler( @@ -59,30 +61,34 @@ public override Task Handle(DidChangeTextDocumentParams notification, Canc return Unit.Task; } - protected override TextDocumentSyncRegistrationOptions CreateRegistrationOptions(SynchronizationCapability capability, ClientCapabilities clientCapabilities) => new() + protected override TextDocumentSyncRegistrationOptions CreateRegistrationOptions(SynchronizationCapability capability, ClientCapabilities clientCapabilities) { - DocumentSelector = LspUtils.PowerShellDocumentSelector, - Change = Change, - Save = new SaveOptions { IncludeText = true } - }; + _isFileWatcherSupported = clientCapabilities.Workspace.DidChangeWatchedFiles.IsSupported; + return new TextDocumentSyncRegistrationOptions() + { + DocumentSelector = LspUtils.PowerShellDocumentSelector, + Change = Change, + Save = new SaveOptions { IncludeText = true } + }; + } public override Task Handle(DidOpenTextDocumentParams notification, CancellationToken token) { + // We use a fake Uri because we only want to test the LanguageId here and not if the + // file ends in ps*1. + TextDocumentAttributes attributes = new(s_fakeUri, notification.TextDocument.LanguageId); + if (!LspUtils.PowerShellDocumentSelector.IsMatch(attributes)) + { + return Unit.Task; + } + ScriptFile openedFile = _workspaceService.GetFileBuffer( notification.TextDocument.Uri, notification.TextDocument.Text); - if (LspUtils.PowerShellDocumentSelector.IsMatch(new TextDocumentAttributes( - // We use a fake Uri because we only want to test the LanguageId here and not if the - // file ends in ps*1. - s_fakeUri, - notification.TextDocument.LanguageId))) - { - // Kick off script diagnostics if we got a PowerShell file without blocking the response - // TODO: Get all recently edited files in the workspace - _analysisService.StartScriptDiagnostics(new ScriptFile[] { openedFile }); - } + openedFile.IsOpen = true; + _analysisService.StartScriptDiagnostics(new ScriptFile[] { openedFile }); _logger.LogTrace("Finished opening document."); return Unit.Task; @@ -95,7 +101,16 @@ public override Task Handle(DidCloseTextDocumentParams notification, Cance if (fileToClose != null) { - _workspaceService.CloseFile(fileToClose); + fileToClose.IsOpen = false; + + // If the file watcher is supported, only close in-memory files when this + // notification is triggered. This lets us keep workspace files open so we can scan + // for references. When a file is deleted, the file watcher will close the file. + if (!_isFileWatcherSupported || fileToClose.IsInMemory) + { + _workspaceService.CloseFile(fileToClose); + } + _analysisService.ClearMarkers(fileToClose); } diff --git a/src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs b/src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs index 449b78a13..c3e5ac303 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs @@ -119,6 +119,10 @@ public string[] ReferencedFiles private set; } + internal ReferenceTable References { get; } + + internal bool IsOpen { get; set; } + #endregion #region Constructors @@ -149,6 +153,7 @@ internal ScriptFile( // SetFileContents() calls ParseFileContents() which initializes the rest of the properties. SetFileContents(textReader.ReadToEnd()); + References = new ReferenceTable(this); } /// @@ -383,6 +388,7 @@ public void ApplyChange(FileChange fileChange) // Parse the script again to be up-to-date ParseFileContents(); + References.TagAsChanged(); } /// @@ -519,7 +525,7 @@ public BufferRange GetRangeBetweenOffsets(int startOffset, int endOffset) #region Private Methods - private void SetFileContents(string fileContents) + internal void SetFileContents(string fileContents) { // Split the file contents into lines and trim // any carriage returns from the strings. diff --git a/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs b/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs index 8f16ed43a..833b6a881 100644 --- a/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs +++ b/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs @@ -23,6 +23,7 @@ internal class LanguageServerSettings public PesterSettings Pester { get; set; } public string Cwd { get; set; } public bool EnableReferencesCodeLens { get; set; } = true; + public bool AnalyzeOpenDocumentsOnly { get; set; } public LanguageServerSettings() { @@ -48,6 +49,7 @@ public void Update( Pester.Update(settings.Pester, logger); Cwd = settings.Cwd; EnableReferencesCodeLens = settings.EnableReferencesCodeLens; + AnalyzeOpenDocumentsOnly = settings.AnalyzeOpenDocumentsOnly; } } } diff --git a/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs b/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs index b4e6a8b30..ee76bfd99 100644 --- a/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs +++ b/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs @@ -127,8 +127,7 @@ public ScriptFile GetFile(DocumentUri documentUri) { // This method allows FileNotFoundException to bubble up // if the file isn't found. - using (FileStream fileStream = new(documentUri.GetFileSystemPath(), FileMode.Open, FileAccess.Read)) - using (StreamReader streamReader = new(fileStream, Encoding.UTF8)) + using (StreamReader streamReader = OpenStreamReader(documentUri)) { scriptFile = new ScriptFile( @@ -451,6 +450,22 @@ private void RecursivelyFindReferences( } } + internal static StreamReader OpenStreamReader(DocumentUri uri) + { + FileStream fileStream = new(uri.GetFileSystemPath(), FileMode.Open, FileAccess.Read); + // Default to UTF8 no BOM if a BOM is not present. Note that `Encoding.UTF8` is *with* + // BOM, so we call the ctor here to get the BOM-less version. + // + // TODO: Honor workspace encoding settings for the fallback. + return new StreamReader(fileStream, new UTF8Encoding(), detectEncodingFromByteOrderMarks: true); + } + + internal static string ReadFileContents(DocumentUri uri) + { + using StreamReader reader = OpenStreamReader(uri); + return reader.ReadToEnd(); + } + internal static bool IsPathInMemory(string filePath) { bool isInMemory = false; diff --git a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs index bcfd49de9..df1a86912 100644 --- a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs @@ -478,17 +478,9 @@ function CanSendReferencesRequest { .Returning(CancellationToken.None).ConfigureAwait(true); Assert.Collection(locations, - location1 => + location => { - Range range = location1.Range; - Assert.Equal(1, range.Start.Line); - Assert.Equal(9, range.Start.Character); - Assert.Equal(1, range.End.Line); - Assert.Equal(33, range.End.Character); - }, - location2 => - { - Range range = location2.Range; + Range range = location.Range; Assert.Equal(5, range.Start.Line); Assert.Equal(0, range.Start.Character); Assert.Equal(5, range.End.Line); diff --git a/test/PowerShellEditorServices.Test/Language/SymbolsServiceTests.cs b/test/PowerShellEditorServices.Test/Language/SymbolsServiceTests.cs index 34f1720ce..e7938c287 100644 --- a/test/PowerShellEditorServices.Test/Language/SymbolsServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Language/SymbolsServiceTests.cs @@ -152,9 +152,9 @@ await psesHost.ExecutePSCommandAsync( public async Task FindsReferencesOnFunction() { List referencesResult = await GetReferences(FindsReferencesOnFunctionData.SourceDetails).ConfigureAwait(true); - Assert.Equal(3, referencesResult.Count); - Assert.Equal(1, referencesResult[0].ScriptRegion.StartLineNumber); - Assert.Equal(10, referencesResult[0].ScriptRegion.StartColumnNumber); + Assert.Equal(2, referencesResult.Count); + Assert.Equal(3, referencesResult[0].ScriptRegion.StartLineNumber); + Assert.Equal(2, referencesResult[0].ScriptRegion.StartColumnNumber); } [Fact] @@ -166,9 +166,9 @@ await psesHost.ExecutePSCommandAsync( CancellationToken.None).ConfigureAwait(true); List referencesResult = await GetReferences(FindsReferencesOnFunctionData.SourceDetails).ConfigureAwait(true); - Assert.Equal(4, referencesResult.Count); - Assert.Equal(1, referencesResult[0].ScriptRegion.StartLineNumber); - Assert.Equal(10, referencesResult[0].ScriptRegion.StartColumnNumber); + Assert.Equal(3, referencesResult.Count); + Assert.Equal(3, referencesResult[0].ScriptRegion.StartLineNumber); + Assert.Equal(2, referencesResult[0].ScriptRegion.StartColumnNumber); } [Fact] @@ -246,8 +246,8 @@ public async Task FindsReferencesOnCommandWithAlias() { List referencesResult = await GetReferences(FindsReferencesOnBuiltInCommandWithAliasData.SourceDetails).ConfigureAwait(true); Assert.Equal(4, referencesResult.Count); - Assert.Equal("gci", referencesResult[1].SymbolName); - Assert.Equal("dir", referencesResult[2].SymbolName); + Assert.Equal("Get-ChildItem", referencesResult[1].SymbolName); + Assert.Equal("Get-ChildItem", referencesResult[2].SymbolName); Assert.Equal("Get-ChildItem", referencesResult[referencesResult.Count - 1].SymbolName); } @@ -255,14 +255,14 @@ public async Task FindsReferencesOnCommandWithAlias() public async Task FindsReferencesOnFileWithReferencesFileB() { List referencesResult = await GetReferences(FindsReferencesOnFunctionMultiFileDotSourceFileB.SourceDetails).ConfigureAwait(true); - Assert.Equal(4, referencesResult.Count); + Assert.Equal(3, referencesResult.Count); } [Fact] public async Task FindsReferencesOnFileWithReferencesFileC() { List referencesResult = await GetReferences(FindsReferencesOnFunctionMultiFileDotSourceFileC.SourceDetails).ConfigureAwait(true); - Assert.Equal(4, referencesResult.Count); + Assert.Equal(3, referencesResult.Count); } [Fact]