Merge pull request #1830 from PowerShell/andschwa/untitled-tests

Add regression test for untitled scripts in Windows PowerShell

Author: SeeminglyScience

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs

@@ -105,10 +105,12 @@ public Task<ConfigurationDoneResponse> Handle(ConfigurationDoneArguments request
         private async Task LaunchScriptAsync(string scriptToLaunch)
         {
             PSCommand command;
-            if (ScriptFile.IsUntitledPath(scriptToLaunch))
+            // Script could an actual script, or a URI to a script file (or untitled document).
+            if (!System.Uri.IsWellFormedUriString(scriptToLaunch, System.UriKind.RelativeOrAbsolute)
+                || ScriptFile.IsUntitledPath(scriptToLaunch))
             {
-                ScriptFile untitledScript = _workspaceService.GetFile(scriptToLaunch);
-                if (BreakpointApiUtils.SupportsBreakpointApis(_runspaceContext.CurrentRunspace))
+                bool isScriptFile = _workspaceService.TryGetFile(scriptToLaunch, out ScriptFile untitledScript);
+                if (isScriptFile && BreakpointApiUtils.SupportsBreakpointApis(_runspaceContext.CurrentRunspace))
                 {
                     // Parse untitled files with their `Untitled:` URI as the filename which will
                     // cache the URI and contents within the PowerShell parser. By doing this, we
@@ -138,7 +140,11 @@ private async Task LaunchScriptAsync(string scriptToLaunch)
                     // Command breakpoints and `Wait-Debugger` will work. We must wrap the script
                     // with newlines so that any included comments don't break the command.
                     command = PSCommandHelpers.BuildDotSourceCommandWithArguments(
-                        string.Concat("{\n", untitledScript.Contents, "\n}"), _debugStateService.Arguments);
+                        string.Concat(
+                            "{" + System.Environment.NewLine,
+                            isScriptFile ? untitledScript.Contents : scriptToLaunch,
+                            System.Environment.NewLine + "}"),
+                            _debugStateService.Arguments);
                 }
             }
             else

src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs

@@ -427,13 +427,10 @@ public async Task<SymbolReference> GetDefinitionOfSymbolAsync(
             SymbolReference foundDefinition = null;
             foreach (ScriptFile scriptFile in referencedFiles)
             {
-                foundDefinition =
-                    AstOperations.FindDefinitionOfSymbol(
-                        scriptFile.ScriptAst,
-                        foundSymbol);
+                foundDefinition = AstOperations.FindDefinitionOfSymbol(scriptFile.ScriptAst, foundSymbol);
 
                 filesSearched.Add(scriptFile.FilePath);
-                if (foundDefinition != null)
+                if (foundDefinition is not null)
                 {
                     foundDefinition.FilePath = scriptFile.FilePath;
                     break;
@@ -453,7 +450,7 @@ public async Task<SymbolReference> GetDefinitionOfSymbolAsync(
 
             // if the definition the not found in referenced files
             // look for it in all the files in the workspace
-            if (foundDefinition == null)
+            if (foundDefinition is null)
             {
                 // Get a list of all powershell files in the workspace path
                 foreach (string file in _workspaceService.EnumeratePSFiles())
@@ -469,7 +466,7 @@ public async Task<SymbolReference> GetDefinitionOfSymbolAsync(
                             foundSymbol);
 
                     filesSearched.Add(file);
-                    if (foundDefinition != null)
+                    if (foundDefinition is not null)
                     {
                         foundDefinition.FilePath = file;
                         break;
@@ -480,7 +477,7 @@ public async Task<SymbolReference> GetDefinitionOfSymbolAsync(
             // if the definition is not found in a file in the workspace
             // look for it in the builtin commands but only if the symbol
             // we are looking at is possibly a Function.
-            if (foundDefinition == null
+            if (foundDefinition is null
                 && (foundSymbol.SymbolType == SymbolType.Function
                     || foundSymbol.SymbolType == SymbolType.Unknown))
             {

src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs

@@ -215,11 +215,8 @@ public static SymbolReference FindDefinitionOfSymbol(
             Ast scriptAst,
             SymbolReference symbolReference)
         {
-            FindDeclarationVisitor declarationVisitor =
-                new(
-                    symbolReference);
+            FindDeclarationVisitor declarationVisitor = new(symbolReference);
             scriptAst.Visit(declarationVisitor);
-
             return declarationVisitor.FoundDeclaration;
         }
 

src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs

@@ -195,18 +195,17 @@ internal static List<string> GetLinesInternal(string text)
         }
 
         /// <summary>
-        /// Deterines whether the supplied path indicates the file is an "untitled:Unitled-X"
+        /// Determines whether the supplied path indicates the file is an "untitled:Untitled-X"
         /// which has not been saved to file.
         /// </summary>
         /// <param name="path">The path to check.</param>
         /// <returns>True if the path is an untitled file, false otherwise.</returns>
         internal static bool IsUntitledPath(string path)
         {
             Validate.IsNotNull(nameof(path), path);
-            return !string.Equals(
-                DocumentUri.From(path).Scheme,
-                Uri.UriSchemeFile,
-                StringComparison.OrdinalIgnoreCase);
+            // This may not have been given a URI, so return false instead of throwing.
+            return Uri.IsWellFormedUriString(path, UriKind.RelativeOrAbsolute) &&
+                !string.Equals(DocumentUri.From(path).Scheme, Uri.UriSchemeFile, StringComparison.OrdinalIgnoreCase);
         }
 
         /// <summary>

src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs

@@ -2,17 +2,17 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
-using System.Linq;
 using System.IO;
+using System.Linq;
 using System.Security;
 using System.Text;
 using Microsoft.Extensions.FileSystemGlobbing;
 using Microsoft.Extensions.Logging;
-using Microsoft.PowerShell.EditorServices.Utility;
-using Microsoft.PowerShell.EditorServices.Services.Workspace;
 using Microsoft.PowerShell.EditorServices.Services.TextDocument;
-using System.Collections.Concurrent;
+using Microsoft.PowerShell.EditorServices.Services.Workspace;
+using Microsoft.PowerShell.EditorServices.Utility;
 using OmniSharp.Extensions.LanguageServer.Protocol;
 
 namespace Microsoft.PowerShell.EditorServices.Services
@@ -152,8 +152,19 @@ public ScriptFile GetFile(DocumentUri documentUri)
         /// </summary>
         /// <param name="filePath">The file path at which the script resides.</param>
         /// <param name="scriptFile">The out parameter that will contain the ScriptFile object.</param>
-        public bool TryGetFile(string filePath, out ScriptFile scriptFile) =>
-            TryGetFile(new Uri(filePath), out scriptFile);
+        public bool TryGetFile(string filePath, out ScriptFile scriptFile)
+        {
+            // This might not have been given a file path, in which case the Uri constructor barfs.
+            try
+            {
+                return TryGetFile(new Uri(filePath), out scriptFile);
+            }
+            catch (UriFormatException)
+            {
+                scriptFile = null;
+                return false;
+            }
+        }
 
         /// <summary>
         /// Tries to get an open file in the workspace. Returns true if it succeeds, false otherwise.
@@ -301,7 +312,7 @@ public ScriptFile[] ExpandScriptReferences(ScriptFile scriptFile)
             referencedScriptFiles.Add(scriptFile.Id, scriptFile);
             RecursivelyFindReferences(scriptFile, referencedScriptFiles);
 
-            // remove original file from referened file and add it as the first element of the
+            // remove original file from referenced file and add it as the first element of the
             // expanded referenced list to maintain order so the original file is always first in the list
             referencedScriptFiles.Remove(scriptFile.Id);
             expandedReferences.Add(scriptFile);

test/PowerShellEditorServices.Test.E2E/DebugAdapterClientExtensions.cs

@@ -12,13 +12,13 @@ namespace PowerShellEditorServices.Test.E2E
 {
     public static class DebugAdapterClientExtensions
     {
-        public static async Task LaunchScript(this DebugAdapterClient debugAdapterClient, string filePath, TaskCompletionSource<object> started)
+        public static async Task LaunchScript(this DebugAdapterClient debugAdapterClient, string script, TaskCompletionSource<object> started)
         {
             LaunchResponse launchResponse = await debugAdapterClient.Launch(
                 new PsesLaunchRequestArguments
                 {
                     NoDebug = false,
-                    Script = filePath,
+                    Script = script,
                     Cwd = "",
                     CreateTemporaryIntegratedConsole = false
                 }).ConfigureAwait(true);

test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs

@@ -127,7 +127,13 @@ private string GenerateScriptFromLoggingStatements(params string[] logStatements
                 throw new ArgumentNullException(nameof(logStatements), "Expected at least one argument.");
             }
 
-            // Have script create/overwrite file first with `>`.
+            // Clean up side effects from other test runs.
+            if (File.Exists(s_testOutputPath))
+            {
+                File.Delete(s_testOutputPath);
+            }
+
+            // Have script create file first with `>` (but don't rely on overwriting).
             StringBuilder builder = new StringBuilder().Append('\'').Append(logStatements[0]).Append("' > '").Append(s_testOutputPath).AppendLine("'");
             for (int i = 1; i < logStatements.Length; i++)
             {
@@ -177,7 +183,7 @@ public async Task CanLaunchScriptWithNoBreakpointsAsync()
         public async Task CanSetBreakpointsAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode,
+                PsesStdioProcess.RunningInConstrainedLanguageMode,
                 "You can't set breakpoints in ConstrainedLanguage mode.");
 
             string filePath = NewTestFile(GenerateScriptFromLoggingStatements(
@@ -254,7 +260,7 @@ public async Task CanSetBreakpointsAsync()
         public async Task CanStepPastSystemWindowsForms()
         {
             Skip.IfNot(PsesStdioProcess.IsWindowsPowerShell);
-            Skip.If(PsesStdioProcess.RunningInConstainedLanguageMode);
+            Skip.If(PsesStdioProcess.RunningInConstrainedLanguageMode);
 
             string filePath = NewTestFile(string.Join(Environment.NewLine, new[]
                 {
@@ -291,5 +297,32 @@ public async Task CanStepPastSystemWindowsForms()
             Assert.NotNull(form);
             Assert.Equal("System.Windows.Forms.Form, Text: ", form.Value);
         }
+
+        // This tests the edge-case where a raw script (or an untitled script) has the last line
+        // commented. Since in some cases (such as Windows PowerShell, or the script not having a
+        // backing ScriptFile) we just wrap the script with braces, we had a bug where the last
+        // brace would be after the comment. We had to ensure we wrapped with newlines instead.
+        [Trait("Category", "DAP")]
+        [Fact]
+        public async Task CanLaunchScriptWithCommentedLastLineAsync()
+        {
+            string script = GenerateScriptFromLoggingStatements("a log statement") + "# a comment at the end";
+            Assert.Contains(Environment.NewLine + "# a comment", script);
+            Assert.EndsWith("at the end", script);
+
+            // NOTE: This is horribly complicated, but the "script" parameter here is assigned to
+            // PsesLaunchRequestArguments.Script, which is then assigned to
+            // DebugStateService.ScriptToLaunch in that handler, and finally used by the
+            // ConfigurationDoneHandler in LaunchScriptAsync.
+            await PsesDebugAdapterClient.LaunchScript(script, Started).ConfigureAwait(false);
+
+            ConfigurationDoneResponse configDoneResponse = await PsesDebugAdapterClient.RequestConfigurationDone(new ConfigurationDoneArguments()).ConfigureAwait(false);
+            Assert.NotNull(configDoneResponse);
+
+            // At this point the script should be running so lets give it time
+            await Task.Delay(2000).ConfigureAwait(false);
+
+            Assert.Collection(GetLog(), (i) => Assert.Equal("a log statement", i));
+        }
     }
 }

test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs

@@ -155,7 +155,7 @@ function CanSendWorkspaceSymbolRequest {
         public async Task CanReceiveDiagnosticsFromFileOpenAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
+                PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
                 "Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");
 
             NewTestFile("$a = 4");
@@ -178,7 +178,7 @@ public async Task WontReceiveDiagnosticsFromFileOpenThatIsNotPowerShellAsync()
         public async Task CanReceiveDiagnosticsFromFileChangedAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
+                PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
                 "Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");
 
             string filePath = NewTestFile("$a = 4");
@@ -230,7 +230,7 @@ public async Task CanReceiveDiagnosticsFromFileChangedAsync()
         public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
+                PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
                 "Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");
 
             NewTestFile("gci | % { $_ }");
@@ -331,7 +331,7 @@ await PsesLanguageClient
         public async Task CanSendFormattingRequestAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
+                PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
                 "Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");
 
             string scriptPath = NewTestFile(@"
@@ -368,7 +368,7 @@ public async Task CanSendFormattingRequestAsync()
         public async Task CanSendRangeFormattingRequestAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
+                PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
                 "Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");
 
             string scriptPath = NewTestFile(@"
@@ -892,7 +892,7 @@ function CanSendReferencesCodeLensRequest {
         public async Task CanSendCodeActionRequestAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
+                PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
                 "Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");
 
             string filePath = NewTestFile("gci");
@@ -1090,7 +1090,7 @@ await PsesLanguageClient
         [SkippableFact]
         public async Task CanSendGetProjectTemplatesRequestAsync()
         {
-            Skip.If(PsesStdioProcess.RunningInConstainedLanguageMode, "Plaster doesn't work in ConstrainedLanguage mode.");
+            Skip.If(PsesStdioProcess.RunningInConstrainedLanguageMode, "Plaster doesn't work in ConstrainedLanguage mode.");
 
             GetProjectTemplatesResponse getProjectTemplatesResponse =
                 await PsesLanguageClient
@@ -1110,7 +1110,7 @@ await PsesLanguageClient
         public async Task CanSendGetCommentHelpRequestAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
+                PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell,
                 "Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load.");
 
             string scriptPath = NewTestFile(@"
@@ -1183,7 +1183,7 @@ await PsesLanguageClient
         public async Task CanSendExpandAliasRequestAsync()
         {
             Skip.If(
-                PsesStdioProcess.RunningInConstainedLanguageMode,
+                PsesStdioProcess.RunningInConstrainedLanguageMode,
                 "This feature currently doesn't support ConstrainedLanguage Mode.");
 
             ExpandAliasResult expandAliasResult =

test/PowerShellEditorServices.Test.E2E/Processes/PsesStdioProcess.cs

@@ -43,9 +43,10 @@ public class PsesStdioProcess : StdioServerProcess
 
         #region public static properties
 
+        // NOTE: Just hard-code this to "powershell" when testing with the code lens.
         public static string PwshExe { get; } = Environment.GetEnvironmentVariable("PWSH_EXE_NAME") ?? "pwsh";
         public static bool IsWindowsPowerShell { get; } = PwshExe.Contains("powershell");
-        public static bool RunningInConstainedLanguageMode { get; } =
+        public static bool RunningInConstrainedLanguageMode { get; } =
             Environment.GetEnvironmentVariable("__PSLockdownPolicy", EnvironmentVariableTarget.Machine) != null;
 
         #endregion

test/PowerShellEditorServices.Test/Language/SymbolsServiceTests.cs

@@ -133,7 +133,7 @@ public async Task FindsFunctionDefinition()
         [Fact]
         public async Task FindsFunctionDefinitionForAlias()
         {
-            // TODO: Eventually we should get the alises through the AST instead of relying on them
+            // TODO: Eventually we should get the aliases through the AST instead of relying on them
             // being defined in the runspace.
             await psesHost.ExecutePSCommandAsync(
                 new PSCommand().AddScript("Set-Alias -Name My-Alias -Value My-Function"),
@@ -184,6 +184,7 @@ public async Task FindsFunctionDefinitionInDotSourceReference()
         public async Task FindsDotSourcedFile()
         {
             SymbolReference definitionResult = await GetDefinition(FindsDotSourcedFileData.SourceDetails).ConfigureAwait(true);
+            Assert.NotNull(definitionResult);
             Assert.True(
                 definitionResult.FilePath.EndsWith(Path.Combine("References", "ReferenceFileE.ps1")),
                 "Unexpected reference file: " + definitionResult.FilePath);

test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs

@@ -651,6 +651,7 @@ public void DocumentUriReturnsCorrectStringForAbsolutePath()
         [InlineData("vscode-notebook-cell:/Users/me/Documents/test.ps1#0001", true)]
         [InlineData("https://microsoft.com", true)]
         [InlineData("Untitled:Untitled-1", true)]
+        [InlineData("'a log statement' > 'c:\\Users\\me\\Documents\\test.txt'\r\n", false)]
         public void IsUntitledFileIsCorrect(string path, bool expected) => Assert.Equal(expected, ScriptFile.IsUntitledPath(path));
     }
 }