From 830c40dc3e6f2eca9566a6faac39347b74e4561c Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Mon, 31 Dec 2018 19:25:09 -0700 Subject: [PATCH 1/2] Fix FileNotFoundEx crash when Fold happens on untitled: scheme doc --- .../Server/LanguageServer.cs | 20 ++++++++++++++----- .../Workspace/Workspace.cs | 6 ++++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs index 81fc92341..53aa67ff9 100644 --- a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs +++ b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs @@ -1376,15 +1376,24 @@ protected Task HandleEvaluateRequest( #region Event Handlers - private FoldingRange[] Fold( - string documentUri) + private FoldingRange[] Fold(string documentUri) { // TODO Should be using dynamic registrations if (!this.currentSettings.CodeFolding.Enable) { return null; } + + // Avoid crash when using untitled: scheme or any other scheme where the document doesn't + // have a backing file. https://github.com/PowerShell/vscode-powershell/issues/1676 + // Perhaps a better option would be to parse the contents of the document as a string + // as opposed to reading a file but the senario of "no backing file" probably doesn't + // warrant the extra effort. + ScriptFile scriptFile = editorSession.Workspace.GetFile(documentUri); + if (scriptFile == null) { return null; } + var result = new List(); - foreach (FoldingReference fold in TokenOperations.FoldableRegions( - editorSession.Workspace.GetFile(documentUri).ScriptTokens, - this.currentSettings.CodeFolding.ShowLastLine)) + FoldingReference[] foldableRegions = + TokenOperations.FoldableRegions(scriptFile.ScriptTokens, this.currentSettings.CodeFolding.ShowLastLine); + + foreach (FoldingReference fold in foldableRegions) { result.Add(new FoldingRange { EndCharacter = fold.EndCharacter, @@ -1394,6 +1403,7 @@ private FoldingRange[] Fold( StartLine = fold.StartLine }); } + return result.ToArray(); } diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index 69847ee4d..a94083959 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -109,9 +109,11 @@ public ScriptFile GetFile(string filePath) string resolvedFilePath = this.ResolveFilePath(filePath); string keyName = resolvedFilePath.ToLower(); - // Make sure the file isn't already loaded into the workspace + // Make sure the file isn't already loaded into the workspace and that the filePath isn't an "in-memory" path. + // There have been crashes caused by this method being called with an "untitled:" document uri. See: + // https://github.com/PowerShell/vscode-powershell/issues/1676 ScriptFile scriptFile = null; - if (!this.workspaceFiles.TryGetValue(keyName, out scriptFile)) + if (!this.workspaceFiles.TryGetValue(keyName, out scriptFile) && !IsPathInMemory(filePath)) { // This method allows FileNotFoundException to bubble up // if the file isn't found. From ade6c728bd0e21446b53ced89feb633d31438d0b Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Wed, 2 Jan 2019 17:27:31 -0700 Subject: [PATCH 2/2] Address PR feedback, use TryGetFile() instead of modifying GetFile --- .../Server/LanguageServer.cs | 4 ++-- .../Workspace/Workspace.cs | 15 ++++++--------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs index 53aa67ff9..a13f8ee11 100644 --- a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs +++ b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs @@ -1386,8 +1386,8 @@ private FoldingRange[] Fold(string documentUri) // Perhaps a better option would be to parse the contents of the document as a string // as opposed to reading a file but the senario of "no backing file" probably doesn't // warrant the extra effort. - ScriptFile scriptFile = editorSession.Workspace.GetFile(documentUri); - if (scriptFile == null) { return null; } + ScriptFile scriptFile; + if (!editorSession.Workspace.TryGetFile(documentUri, out scriptFile)) { return null; } var result = new List(); FoldingReference[] foldableRegions = diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index a94083959..d74bf4955 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -91,8 +91,9 @@ public ScriptFile CreateScriptFileFromFileBuffer(string filePath, string initial } /// - /// Gets an open file in the workspace. If the file isn't open but - /// exists on the filesystem, load and return it. + /// Gets an open file in the workspace. If the file isn't open but exists on the filesystem, load and return it. + /// IMPORTANT: Not all documents have a backing file e.g. untitled: scheme documents. Consider using + /// instead. /// /// The file path at which the script resides. /// @@ -109,11 +110,9 @@ public ScriptFile GetFile(string filePath) string resolvedFilePath = this.ResolveFilePath(filePath); string keyName = resolvedFilePath.ToLower(); - // Make sure the file isn't already loaded into the workspace and that the filePath isn't an "in-memory" path. - // There have been crashes caused by this method being called with an "untitled:" document uri. See: - // https://github.com/PowerShell/vscode-powershell/issues/1676 + // Make sure the file isn't already loaded into the workspace ScriptFile scriptFile = null; - if (!this.workspaceFiles.TryGetValue(keyName, out scriptFile) && !IsPathInMemory(filePath)) + if (!this.workspaceFiles.TryGetValue(keyName, out scriptFile)) { // This method allows FileNotFoundException to bubble up // if the file isn't found. @@ -156,9 +155,7 @@ e is DirectoryNotFoundException || e is PathTooLongException || e is UnauthorizedAccessException) { - this.logger.WriteException( - $"Failed to set breakpoint on file: {filePath}", - e); + this.logger.WriteHandledException($"Failed to get file for {nameof(filePath)}: '{filePath}'", e); scriptFile = null; return false; }